From 7bd7d3b92b94144b1b008562f9cc5990a7bfb3fc Mon Sep 17 00:00:00 2001 From: Anupam Juniwal Date: Wed, 6 Sep 2023 12:20:28 +0530 Subject: [PATCH 01/12] Initial changes for proof backed deserialization attack detection --- .../deserialisation/build.gradle | 16 ++ .../csec/validator/scanner/DeltaClass.java | 22 ++ .../src/main/java/java/io/SecurityHelper.java | 9 + .../java/io/Serializable_Instrumentation.java | 100 +++++++++ .../instrumentator/dispatcher/Dispatcher.java | 1 + .../security/schema/AbstractOperation.java | 28 ++- .../agent/security/schema/AgentMetaData.java | 11 + .../security/schema/DeserializationInfo.java | 207 ++++++++++++++++++ .../security/schema/SecurityMetaData.java | 29 +++ .../schema/VulnerabilityCaseType.java | 4 +- .../operation/DeserialisationOperation.java | 46 ++++ settings.gradle | 2 +- 12 files changed, 470 insertions(+), 5 deletions(-) create mode 100644 instrumentation-security/deserialisation/build.gradle create mode 100644 instrumentation-security/deserialisation/src/main/java/com/newrelic/csec/validator/scanner/DeltaClass.java create mode 100644 instrumentation-security/deserialisation/src/main/java/java/io/SecurityHelper.java create mode 100644 instrumentation-security/deserialisation/src/main/java/java/io/Serializable_Instrumentation.java create mode 100644 newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/DeserializationInfo.java create mode 100644 newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/operation/DeserialisationOperation.java diff --git a/instrumentation-security/deserialisation/build.gradle b/instrumentation-security/deserialisation/build.gradle new file mode 100644 index 000000000..40fb3a3e9 --- /dev/null +++ b/instrumentation-security/deserialisation/build.gradle @@ -0,0 +1,16 @@ +dependencies { + implementation(project(":newrelic-security-api")) + implementation("com.newrelic.agent.java:newrelic-api:${nrAPIVersion}") + implementation("com.newrelic.agent.java:newrelic-weaver-api:${nrAPIVersion}") +} + +// This instrumentation module should not use the bootstrap classpath + + +jar { + manifest { attributes 'Implementation-Title': 'com.newrelic.instrumentation.security.deserialisation' } +} + +verifyInstrumentation { + verifyClasspath = false // We don't want to verify classpath since these are JDK classes +} diff --git a/instrumentation-security/deserialisation/src/main/java/com/newrelic/csec/validator/scanner/DeltaClass.java b/instrumentation-security/deserialisation/src/main/java/com/newrelic/csec/validator/scanner/DeltaClass.java new file mode 100644 index 000000000..b578fde1e --- /dev/null +++ b/instrumentation-security/deserialisation/src/main/java/com/newrelic/csec/validator/scanner/DeltaClass.java @@ -0,0 +1,22 @@ +package com.newrelic.csec.validator.scanner; + +import java.io.Serializable; + +public class DeltaClass implements Serializable { + + private static final long serialVersionUID = 5220788560470736671L; + + public String field0; + + public DeltaClass(String field0) { + this.field0 = field0; + } + + public String getField0() { + return field0; + } + + public void setField0(String field0) { + this.field0 = field0; + } +} diff --git a/instrumentation-security/deserialisation/src/main/java/java/io/SecurityHelper.java b/instrumentation-security/deserialisation/src/main/java/java/io/SecurityHelper.java new file mode 100644 index 000000000..daed02acb --- /dev/null +++ b/instrumentation-security/deserialisation/src/main/java/java/io/SecurityHelper.java @@ -0,0 +1,9 @@ +package java.io; + +public class SecurityHelper { + public static final String METHOD_NAME_READ_OBJECT = "readObject"; + public static final String NULL_STRING = "null"; + + public static final String NR_SEC_CUSTOM_ATTRIB_NAME = "UNSAFE-DESERIALISATION-LOCK-JAVA-IO-"; + +} \ No newline at end of file diff --git a/instrumentation-security/deserialisation/src/main/java/java/io/Serializable_Instrumentation.java b/instrumentation-security/deserialisation/src/main/java/java/io/Serializable_Instrumentation.java new file mode 100644 index 000000000..c160664f3 --- /dev/null +++ b/instrumentation-security/deserialisation/src/main/java/java/io/Serializable_Instrumentation.java @@ -0,0 +1,100 @@ +package java.io; + +import com.newrelic.api.agent.security.NewRelicSecurity; +import com.newrelic.api.agent.security.instrumentation.helpers.GenericHelper; +import com.newrelic.api.agent.security.schema.DeserializationInfo; +import com.newrelic.api.agent.security.schema.operation.DeserialisationOperation; +import com.newrelic.api.agent.weaver.MatchType; +import com.newrelic.api.agent.weaver.Weave; +import com.newrelic.api.agent.weaver.Weaver; + + +@Weave(type = MatchType.BaseClass, originalName = "java.io.ObjectStreamClass") +public abstract class Serializable_Instrumentation { + + void invokeReadObject(Object obj, ObjectInputStream in) + throws ClassNotFoundException, IOException, + UnsupportedOperationException { + if (NewRelicSecurity.isHookProcessingActive() && + !NewRelicSecurity.getAgent().getSecurityMetaData().getRequest().isEmpty()) { + try { + + NewRelicSecurity.getAgent().getSecurityMetaData().addToDeserializingObjectStack( + new DeserializationInfo(obj.getClass().getName(), obj) + ); + } catch (Exception e){ + e.printStackTrace(); + } + System.out.println("IN invokeReadObject"); + } + try { + System.out.println("CALLING original"); + Weaver.callOriginal(); + System.out.println("Will create DeserialisationOperation"); + DeserialisationOperation operation = new DeserialisationOperation( + this.getClass().getName(), + SecurityHelper.METHOD_NAME_READ_OBJECT + ); + System.out.println("Created DeserialisationOperation"); + NewRelicSecurity.getAgent().registerOperation(operation); + System.out.println("Registered DeserialisationOperation"); + + } finally { + if (NewRelicSecurity.isHookProcessingActive() && + !NewRelicSecurity.getAgent().getSecurityMetaData().getRequest().isEmpty()) { + NewRelicSecurity.getAgent().getSecurityMetaData().popFromDeserializingObjectStack(); + } + } +// registerExitOperation(isFileLockAcquired, operation); + } + + void invokeReadObjectNoData(Object obj) + throws IOException, UnsupportedOperationException{ + boolean isLockAcquired = acquireLockIfPossible(); + if (isLockAcquired && NewRelicSecurity.isHookProcessingActive() && + !NewRelicSecurity.getAgent().getSecurityMetaData().getRequest().isEmpty()) { + try { + + NewRelicSecurity.getAgent().getSecurityMetaData().addToDeserializingObjectStack( + new DeserializationInfo(obj.getClass().getName(), obj) + ); + } catch (Exception e){ + e.printStackTrace(); + } + } + try { + Weaver.callOriginal(); + + DeserialisationOperation operation = new DeserialisationOperation( + this.getClass().getName(), + SecurityHelper.METHOD_NAME_READ_OBJECT + ); + NewRelicSecurity.getAgent().registerOperation(operation); + + } finally { + if (isLockAcquired && NewRelicSecurity.isHookProcessingActive() && + !NewRelicSecurity.getAgent().getSecurityMetaData().getRequest().isEmpty()) { + NewRelicSecurity.getAgent().getSecurityMetaData().popFromDeserializingObjectStack(); + } + if (isLockAcquired){ + releaseLock(); + } + } +// registerExitOperation(isFileLockAcquired, operation); + } + + private void releaseLock() { + try { + GenericHelper.releaseLock(SecurityHelper.NR_SEC_CUSTOM_ATTRIB_NAME, this.hashCode()); + } catch (Throwable ignored) { + } + } + + private boolean acquireLockIfPossible() { + try { + return GenericHelper.acquireLockIfPossible(SecurityHelper.NR_SEC_CUSTOM_ATTRIB_NAME, this.hashCode()); + } catch (Throwable ignored) { + } + return false; + } +} \ No newline at end of file diff --git a/newrelic-security-agent/src/main/java/com/newrelic/agent/security/instrumentator/dispatcher/Dispatcher.java b/newrelic-security-agent/src/main/java/com/newrelic/agent/security/instrumentator/dispatcher/Dispatcher.java index b1fc5202b..fd7e0b327 100644 --- a/newrelic-security-agent/src/main/java/com/newrelic/agent/security/instrumentator/dispatcher/Dispatcher.java +++ b/newrelic-security-agent/src/main/java/com/newrelic/agent/security/instrumentator/dispatcher/Dispatcher.java @@ -577,6 +577,7 @@ private JavaAgentEventBean setGenericProperties(AbstractOperation objectBean, Ja eventBean.setUserAPIInfo(operation.getUserClassEntity().getUserClassElement().getLineNumber(), operation.getUserClassEntity().getUserClassElement().getClassName(), operation.getUserClassEntity().getUserClassElement().getMethodName()); + eventBean.getMetaData().setDeserializationInfo(operation.getDeserializationInfo()); return eventBean; } diff --git a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/AbstractOperation.java b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/AbstractOperation.java index c7e1e3cbb..b0d3fe4c9 100644 --- a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/AbstractOperation.java +++ b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/AbstractOperation.java @@ -1,5 +1,7 @@ package com.newrelic.api.agent.security.schema; +import com.newrelic.api.agent.security.NewRelicSecurity; + public abstract class AbstractOperation { public static final String EMPTY = ""; @@ -25,6 +27,8 @@ public abstract class AbstractOperation { private boolean isLowSeverityHook; + private DeserializationInfo deserializationInfo; + public AbstractOperation() { this.className = EMPTY; this.sourceMethod = EMPTY; @@ -36,9 +40,19 @@ public AbstractOperation() { } public AbstractOperation(String className, String methodName){ - this.className = className; - this.methodName = methodName; - this.blockingEndTime = 0L; + this.className = className; + this.methodName = methodName; + this.blockingEndTime = 0L; + try { + SecurityMetaData meta = NewRelicSecurity.getAgent() + .getSecurityMetaData(); + if (meta != null && meta.peekDeserializingObjectStack() != null) { + this.deserializationInfo = meta.peekDeserializingObjectStack(); + this.deserializationInfo.computeAttributeFlatMap(); + } + }catch (Exception e){ + e.printStackTrace(); + } } public String getClassName() { @@ -135,4 +149,12 @@ public boolean isLowSeverityHook() { public void setLowSeverityHook(boolean lowSeverityHook) { this.isLowSeverityHook = lowSeverityHook; } + + public DeserializationInfo getDeserializationInfo() { + return deserializationInfo; + } + + public void setDeserializationInfo(DeserializationInfo deserializationInfo) { + this.deserializationInfo = deserializationInfo; + } } diff --git a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/AgentMetaData.java b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/AgentMetaData.java index 110dc6fbe..2e31a0d59 100644 --- a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/AgentMetaData.java +++ b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/AgentMetaData.java @@ -38,6 +38,8 @@ public class AgentMetaData { @JsonIgnore private Set ips; + private DeserializationInfo deserializationInfo = new DeserializationInfo(); + public AgentMetaData() { this.rciMethodsCalls = new HashSet<>(); this.ips = new HashSet<>(); @@ -57,6 +59,7 @@ public AgentMetaData(AgentMetaData agentMetaData) { this.userDataTranslationMap = new HashMap<>(agentMetaData.userDataTranslationMap); this.userLevelServiceMethodEncountered = agentMetaData.userLevelServiceMethodEncountered; this.reflectedMetaData = agentMetaData.reflectedMetaData; + this.deserializationInfo = new DeserializationInfo(agentMetaData.deserializationInfo); } public boolean isTriggerViaRCI() { @@ -151,4 +154,12 @@ public boolean isUserLevelServiceMethodEncountered(String framework) { public void setUserLevelServiceMethodEncountered(boolean userLevelServiceMethodEncountered) { this.userLevelServiceMethodEncountered = userLevelServiceMethodEncountered; } + + public DeserializationInfo getDeserializationInfo() { + return deserializationInfo; + } + + public void setDeserializationInfo(DeserializationInfo deserializationInfo) { + this.deserializationInfo = new DeserializationInfo(deserializationInfo); + } } diff --git a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/DeserializationInfo.java b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/DeserializationInfo.java new file mode 100644 index 000000000..b6f8e4fff --- /dev/null +++ b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/DeserializationInfo.java @@ -0,0 +1,207 @@ +package com.newrelic.api.agent.security.schema; + +import java.io.Serializable; +import java.lang.reflect.Field; +import java.util.*; + +public class DeserializationInfo implements Serializable { + final static int MAX_DEPTH_POPULATION = 10; + final static Set PRIMITIVE_WRAPPERS = new HashSet() {{ + add(Boolean.class); + add(Character.class); + add(Byte.class); + add(Short.class); + add(Integer.class); + add(Long.class); + add(Float.class); + add(Double.class); + add(Void.class); + add(boolean.class); + add(char.class); + add(byte.class); + add(short.class); + add(int.class); + add(long.class); + add(float.class); + add(double.class); + add(void.class); + }}; + + private String type; + private boolean leaf; + private Map value; + private String leafValue; + private Object instance; + + public DeserializationInfo(String type, Object instance) { + this.type = type; + this.instance = instance; + this.leaf = false; + } + + public DeserializationInfo(String type, Object instance, Map value) { + this.type = type; + this.instance = instance; + this.leaf = false; + this.value = value; + } + + public DeserializationInfo(String type, Object instance, String value) { + this.type = type; + this.instance = instance; + this.leaf = true; + this.leafValue = value; + } + + public DeserializationInfo(DeserializationInfo instance) { + this.type = instance.type; + this.leaf = instance.leaf; + if (instance.leaf){ + this.leafValue = instance.leafValue; + } else { + this.value = new HashMap<>(); + this.value.putAll(instance.value); + } + } + public DeserializationInfo() { + this.type = ""; + this.leaf = false; + this.value = new HashMap<>(); + this.leafValue = ""; + } + + + public Map computeAttributeFlatMap() { + if (this.value == null || this.leafValue == null || this.leafValue.isEmpty()) { + try { + this.value = computeKeyValueMappingOnObject(this.instance, 0); + } catch (IllegalAccessException e) { + throw new RuntimeException(e); + } + } + return this.value; + } + + private Map computeKeyValueMappingOnObject(Object obj, int depth) throws IllegalAccessException { + Map valueMap = new HashMap<>(); + if (depth > MAX_DEPTH_POPULATION){ + return new HashMap<>(); + } + try { + Field[] fields = obj.getClass().getFields(); + for (Field field : fields) { + boolean accessibility = field.isAccessible(); + field.setAccessible(true); + Object val = field.get(obj); + populateForField(val, field.getName(), valueMap, depth++); + field.setAccessible(accessibility); + } + List fieldList = getAllPrivateFields(obj.getClass()); + for (Field field : fieldList) { + boolean accessibility = field.isAccessible(); + field.setAccessible(true); + Object val = field.get(obj); + populateForField(val, field.getName(), valueMap, depth++); + field.setAccessible(accessibility); + } + } catch (Exception e) { + e.printStackTrace(); + } + return valueMap; + } + + private void populateForField(Object obj, String name, Map valueMap, int depth) throws IllegalAccessException { + if (depth > MAX_DEPTH_POPULATION || obj == null || name == null || valueMap == null){ + return; + } + boolean primitiveObj = obj.getClass().isPrimitive() || PRIMITIVE_WRAPPERS.contains(obj.getClass()); + if (primitiveObj) { + return; + } + + if (obj.getClass() == String.class) { + DeserializationInfo entry = new DeserializationInfo(obj.getClass().getName(), obj, (String) obj); + valueMap.put(name, entry); + } else if (Collection.class.isAssignableFrom(obj.getClass())) { + Object col[] = ((Collection) obj).toArray(); + Map collectionMap = new HashMap<>(); + for (int elementIndex=0; elementIndex getAllPrivateFields(Class clz){ + List fields = new ArrayList<>(); + fields.addAll(Arrays.asList(clz.getDeclaredFields())); + Class superClass = clz.getSuperclass(); + if (superClass != null && (PRIMITIVE_WRAPPERS.contains(superClass) || superClass == Object.class)){ + fields.addAll(getAllPrivateFields(superClass)); + } + return fields; + } + + private Map getFlatMap(Map identifierMap){ + Map flatMap = new HashMap<>(); + for (Map.Entry entry : identifierMap.entrySet()) { + if (entry.getValue().getKey() == String.class.getName()) { + flatMap.put(entry.getKey(), (String) entry.getValue().getValue()); + } else if (Map.class.isAssignableFrom(entry.getValue().getValue().getClass())){ + Map valMap = (Map) entry.getValue().getValue(); + Map childFlatMap = getFlatMap(valMap); + for (Map.Entry childEntry : childFlatMap.entrySet()){ + String key = String.format("%s.%s", entry.getKey(), childEntry.getKey()); + flatMap.put(key, childEntry.getValue()); + } + } + } + return flatMap; + } + + public String getType() { + return type; + } + + public void setType(String type) { + this.type = type; + } + + public boolean isLeaf() { + return leaf; + } + + public void setLeaf(boolean leaf) { + this.leaf = leaf; + } + + public Map getValue() { + return value; + } + + public void setValue(Map value) { + this.value = value; + } + + public String getLeafValue() { + return leafValue; + } + + public void setLeafValue(String leafValue) { + this.leafValue = leafValue; + } + + public Object getInstance() { + return instance; + } + + public void setInstance(Object instance) { + this.instance = instance; + } +} \ No newline at end of file diff --git a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/SecurityMetaData.java b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/SecurityMetaData.java index 93950bb7e..27742dcff 100644 --- a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/SecurityMetaData.java +++ b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/SecurityMetaData.java @@ -1,6 +1,7 @@ package com.newrelic.api.agent.security.schema; +import com.newrelic.api.agent.security.schema.operation.DeserialisationOperation; import com.newrelic.api.agent.security.schema.operation.FileIntegrityOperation; import java.util.HashMap; @@ -111,4 +112,32 @@ public void removeCustomAttribute(String key) { this.customData.remove(key); } + public Map getDeserializingObjectStack() { + if (getCustomAttribute("deserializingObjectStack", Map.class) == null){ + addCustomAttribute("deserializingObjectStack", new HashMap()); + } + return getCustomAttribute("deserializingObjectStack", Map.class); + } + + public void addToDeserializingObjectStack(DeserializationInfo dinfo) { + Map deserializingObjectStack = getDeserializingObjectStack(); + int nextIndex = deserializingObjectStack.size(); + deserializingObjectStack.put(Integer.toString(nextIndex), dinfo); + } + + public void popFromDeserializingObjectStack() { + Map deserializingObjectStack = getDeserializingObjectStack(); + int nextIndex = deserializingObjectStack.size(); + if (nextIndex > 0) { + deserializingObjectStack.remove(Integer.toString(nextIndex-1)); + } + } + + public DeserializationInfo peekDeserializingObjectStack() { + Map deserializingObjectStack = getDeserializingObjectStack(); + if (deserializingObjectStack == null || deserializingObjectStack.isEmpty()) return null; + + int nextIndex = deserializingObjectStack.size(); + return deserializingObjectStack.get(Integer.toString(nextIndex-1)); + } } diff --git a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/VulnerabilityCaseType.java b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/VulnerabilityCaseType.java index f51be2edc..7bde5fdbc 100644 --- a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/VulnerabilityCaseType.java +++ b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/VulnerabilityCaseType.java @@ -68,7 +68,9 @@ public enum VulnerabilityCaseType { /** JavaScript Injection */ JAVASCRIPT_INJECTION("JAVASCRIPT_INJECTION"), /** JavaScript Injection */ - XQUERY_INJECTION("XQUERY_INJECTION"); + XQUERY_INJECTION("XQUERY_INJECTION"), + /** Unsafe Deserialization */ + UNSAFE_DESERIALIZATION("UNSAFE_DESERIALIZATION"); /** case type. */ diff --git a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/operation/DeserialisationOperation.java b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/operation/DeserialisationOperation.java new file mode 100644 index 000000000..0ded6560f --- /dev/null +++ b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/operation/DeserialisationOperation.java @@ -0,0 +1,46 @@ +package com.newrelic.api.agent.security.schema.operation; + +import com.newrelic.api.agent.security.NewRelicSecurity; +import com.newrelic.api.agent.security.schema.AbstractOperation; +import com.newrelic.api.agent.security.schema.DeserializationInfo; +import com.newrelic.api.agent.security.schema.StringUtils; +import com.newrelic.api.agent.security.schema.VulnerabilityCaseType; + +import java.util.Map; + +public class DeserialisationOperation extends AbstractOperation { + + private String entityName; + private Map params; + + + public DeserialisationOperation(String className, String methodName) { + super(className, methodName); + this.entityName = NewRelicSecurity.getAgent().getSecurityMetaData() + .peekDeserializingObjectStack().getType(); + this.params = NewRelicSecurity.getAgent().getSecurityMetaData() + .peekDeserializingObjectStack().computeAttributeFlatMap(); + this.setCaseType(VulnerabilityCaseType.UNSAFE_DESERIALIZATION); + } + + @Override + public boolean isEmpty() { + return this.params==null || this.params.isEmpty() || StringUtils.isEmpty(this.entityName); + } + + public String getEntityName() { + return entityName; + } + + public void setEntityName(String entityName) { + this.entityName = entityName; + } + + public Map getParams() { + return params; + } + + public void setParams(Map params) { + this.params = params; + } +} diff --git a/settings.gradle b/settings.gradle index d97187553..7775bc294 100644 --- a/settings.gradle +++ b/settings.gradle @@ -41,6 +41,7 @@ include 'instrumentation:java-lang' include 'instrumentation:java-io-stream' include 'instrumentation:java-io-inputstream-jdk8' include 'instrumentation:java-io-inputstream-jdk9' +include 'instrumentation:deserialisation' include 'instrumentation:file-operation' include 'instrumentation:servlet-2.4' include 'instrumentation:servlet-5.0' @@ -134,4 +135,3 @@ include 'instrumentation:jdbc-inet-merlia' include 'instrumentation:jdbc-inet-oranxo' include 'instrumentation:jdbc-sybase-6' include 'instrumentation:low-priority-instrumentation' - From cac032e27d850980fa21ed2d5148b86c4a462976 Mon Sep 17 00:00:00 2001 From: Anupam Juniwal Date: Wed, 13 Sep 2023 18:15:32 +0530 Subject: [PATCH 02/12] This contains code cleanup and multiple fixes as required to gather more information regarding deserialization attacks --- .../java/io/Serializable_Instrumentation.java | 55 +++---------------- .../instrumentator/dispatcher/Dispatcher.java | 9 +++ .../security/schema/AbstractOperation.java | 15 ++++- .../security/schema/DeserializationInfo.java | 24 +++++++- .../operation/DeserialisationOperation.java | 11 ++-- 5 files changed, 57 insertions(+), 57 deletions(-) diff --git a/instrumentation-security/deserialisation/src/main/java/java/io/Serializable_Instrumentation.java b/instrumentation-security/deserialisation/src/main/java/java/io/Serializable_Instrumentation.java index c160664f3..3a19fc309 100644 --- a/instrumentation-security/deserialisation/src/main/java/java/io/Serializable_Instrumentation.java +++ b/instrumentation-security/deserialisation/src/main/java/java/io/Serializable_Instrumentation.java @@ -9,76 +9,35 @@ import com.newrelic.api.agent.weaver.Weaver; -@Weave(type = MatchType.BaseClass, originalName = "java.io.ObjectStreamClass") +@Weave(type = MatchType.ExactClass, originalName = "java.io.ObjectInputStream") public abstract class Serializable_Instrumentation { - void invokeReadObject(Object obj, ObjectInputStream in) - throws ClassNotFoundException, IOException, - UnsupportedOperationException { - if (NewRelicSecurity.isHookProcessingActive() && - !NewRelicSecurity.getAgent().getSecurityMetaData().getRequest().isEmpty()) { - try { + private void readSerialData(Object obj, ObjectStreamClass desc) + throws IOException { - NewRelicSecurity.getAgent().getSecurityMetaData().addToDeserializingObjectStack( - new DeserializationInfo(obj.getClass().getName(), obj) - ); - } catch (Exception e){ - e.printStackTrace(); - } - System.out.println("IN invokeReadObject"); - } try { - System.out.println("CALLING original"); - Weaver.callOriginal(); - System.out.println("Will create DeserialisationOperation"); - DeserialisationOperation operation = new DeserialisationOperation( - this.getClass().getName(), - SecurityHelper.METHOD_NAME_READ_OBJECT - ); - System.out.println("Created DeserialisationOperation"); - NewRelicSecurity.getAgent().registerOperation(operation); - System.out.println("Registered DeserialisationOperation"); - - } finally { if (NewRelicSecurity.isHookProcessingActive() && !NewRelicSecurity.getAgent().getSecurityMetaData().getRequest().isEmpty()) { - NewRelicSecurity.getAgent().getSecurityMetaData().popFromDeserializingObjectStack(); - } - } -// registerExitOperation(isFileLockAcquired, operation); - } - - void invokeReadObjectNoData(Object obj) - throws IOException, UnsupportedOperationException{ - boolean isLockAcquired = acquireLockIfPossible(); - if (isLockAcquired && NewRelicSecurity.isHookProcessingActive() && - !NewRelicSecurity.getAgent().getSecurityMetaData().getRequest().isEmpty()) { - try { - NewRelicSecurity.getAgent().getSecurityMetaData().addToDeserializingObjectStack( new DeserializationInfo(obj.getClass().getName(), obj) ); - } catch (Exception e){ - e.printStackTrace(); } - } - try { Weaver.callOriginal(); DeserialisationOperation operation = new DeserialisationOperation( this.getClass().getName(), SecurityHelper.METHOD_NAME_READ_OBJECT ); + NewRelicSecurity.getAgent().registerOperation(operation); + } catch(Exception e){ + } finally { - if (isLockAcquired && NewRelicSecurity.isHookProcessingActive() && + if (NewRelicSecurity.isHookProcessingActive() && !NewRelicSecurity.getAgent().getSecurityMetaData().getRequest().isEmpty()) { NewRelicSecurity.getAgent().getSecurityMetaData().popFromDeserializingObjectStack(); } - if (isLockAcquired){ - releaseLock(); - } } // registerExitOperation(isFileLockAcquired, operation); } diff --git a/newrelic-security-agent/src/main/java/com/newrelic/agent/security/instrumentator/dispatcher/Dispatcher.java b/newrelic-security-agent/src/main/java/com/newrelic/agent/security/instrumentator/dispatcher/Dispatcher.java index 52a9b6046..a30dc0c18 100644 --- a/newrelic-security-agent/src/main/java/com/newrelic/agent/security/instrumentator/dispatcher/Dispatcher.java +++ b/newrelic-security-agent/src/main/java/com/newrelic/agent/security/instrumentator/dispatcher/Dispatcher.java @@ -191,6 +191,8 @@ public Object call() throws Exception { XQueryOperation xQueryOperationalBean = (XQueryOperation) operation; eventBean = prepareXQueryInjectionEvent(eventBean, xQueryOperationalBean); break; + case UNSAFE_DESERIALIZATION: + prepareDeserializationEvent(eventBean, (DeserialisationOperation) operation); default: } @@ -498,6 +500,13 @@ private static JavaAgentEventBean prepareSSRFEvent(JavaAgentEventBean eventBean, return eventBean; } + private static JavaAgentEventBean prepareDeserializationEvent(JavaAgentEventBean eventBean, + DeserialisationOperation deserialisationOperation) { + JSONArray params = new JSONArray(); + eventBean.setParameters(params); + return eventBean; + } + private boolean allowedExtensionFileIO(JSONArray params, String sourceString, String url) { if (JAVA_IO_FILE_INPUTSTREAM_OPEN.equals(sourceString)) { for (int i = 0; i < params.size(); i++) { diff --git a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/AbstractOperation.java b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/AbstractOperation.java index b0d3fe4c9..510d29719 100644 --- a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/AbstractOperation.java +++ b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/AbstractOperation.java @@ -2,6 +2,8 @@ import com.newrelic.api.agent.security.NewRelicSecurity; +import java.util.Map; + public abstract class AbstractOperation { public static final String EMPTY = ""; @@ -47,8 +49,17 @@ public AbstractOperation(String className, String methodName){ SecurityMetaData meta = NewRelicSecurity.getAgent() .getSecurityMetaData(); if (meta != null && meta.peekDeserializingObjectStack() != null) { - this.deserializationInfo = meta.peekDeserializingObjectStack(); - this.deserializationInfo.computeAttributeFlatMap(); + Map stack = meta.getDeserializingObjectStack(); + this.deserializationInfo = stack.get("0"); + DeserializationInfo nextElement = this.deserializationInfo; + for (DeserializationInfo element : stack.values()){ + element.computeAttributeFlatMap(); + if (element == this.deserializationInfo) { + continue; + } + nextElement.setNext(element); + nextElement = element; + } } }catch (Exception e){ e.printStackTrace(); diff --git a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/DeserializationInfo.java b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/DeserializationInfo.java index b6f8e4fff..c367d3b4a 100644 --- a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/DeserializationInfo.java +++ b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/DeserializationInfo.java @@ -32,6 +32,7 @@ public class DeserializationInfo implements Serializable { private Map value; private String leafValue; private Object instance; + private DeserializationInfo next; public DeserializationInfo(String type, Object instance) { this.type = type; @@ -56,13 +57,17 @@ public DeserializationInfo(String type, Object instance, String value) { public DeserializationInfo(DeserializationInfo instance) { this.type = instance.type; this.leaf = instance.leaf; - if (instance.leaf){ + if (instance.leaf) { this.leafValue = instance.leafValue; } else { this.value = new HashMap<>(); this.value.putAll(instance.value); } +// if (instance.next != null) { +// this.next = new DeserializationInfo(instance.next); +// } } + public DeserializationInfo() { this.type = ""; this.leaf = false; @@ -122,8 +127,13 @@ private void populateForField(Object obj, String name, Map collectionMap = new HashMap<>(); for (int elementIndex=0; elementIndex Date: Tue, 26 Sep 2023 15:16:40 +0530 Subject: [PATCH 03/12] Code cleanup --- .../security/schema/AbstractOperation.java | 24 ++++--------- .../security/schema/DeserializationInfo.java | 35 ++----------------- .../operation/DeserialisationOperation.java | 2 +- 3 files changed, 9 insertions(+), 52 deletions(-) diff --git a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/AbstractOperation.java b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/AbstractOperation.java index 510d29719..d056e802f 100644 --- a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/AbstractOperation.java +++ b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/AbstractOperation.java @@ -45,25 +45,13 @@ public AbstractOperation(String className, String methodName){ this.className = className; this.methodName = methodName; this.blockingEndTime = 0L; - try { - SecurityMetaData meta = NewRelicSecurity.getAgent() - .getSecurityMetaData(); - if (meta != null && meta.peekDeserializingObjectStack() != null) { - Map stack = meta.getDeserializingObjectStack(); - this.deserializationInfo = stack.get("0"); - DeserializationInfo nextElement = this.deserializationInfo; - for (DeserializationInfo element : stack.values()){ - element.computeAttributeFlatMap(); - if (element == this.deserializationInfo) { - continue; - } - nextElement.setNext(element); - nextElement = element; - } + if (NewRelicSecurity.getAgent() != null && + NewRelicSecurity.getAgent().getSecurityMetaData() != null && + NewRelicSecurity.getAgent().getSecurityMetaData().peekDeserializingObjectStack() != null) { + this.deserializationInfo = NewRelicSecurity.getAgent().getSecurityMetaData() + .peekDeserializingObjectStack(); + this.deserializationInfo.computeObjectMap(); } - }catch (Exception e){ - e.printStackTrace(); - } } public String getClassName() { diff --git a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/DeserializationInfo.java b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/DeserializationInfo.java index c367d3b4a..3084701a7 100644 --- a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/DeserializationInfo.java +++ b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/DeserializationInfo.java @@ -32,7 +32,6 @@ public class DeserializationInfo implements Serializable { private Map value; private String leafValue; private Object instance; - private DeserializationInfo next; public DeserializationInfo(String type, Object instance) { this.type = type; @@ -63,9 +62,6 @@ public DeserializationInfo(DeserializationInfo instance) { this.value = new HashMap<>(); this.value.putAll(instance.value); } -// if (instance.next != null) { -// this.next = new DeserializationInfo(instance.next); -// } } public DeserializationInfo() { @@ -76,7 +72,7 @@ public DeserializationInfo() { } - public Map computeAttributeFlatMap() { + public Map computeObjectMap() { if (this.value == null || this.leafValue == null || this.leafValue.isEmpty()) { try { this.value = computeKeyValueMappingOnObject(this.instance, 0); @@ -109,9 +105,7 @@ private Map computeKeyValueMappingOnObject(Object o populateForField(val, field.getName(), valueMap, depth++); field.setAccessible(accessibility); } - } catch (Exception e) { - e.printStackTrace(); - } + } catch (Exception e) {} return valueMap; } @@ -158,23 +152,6 @@ private List getAllPrivateFields(Class clz){ return fields; } - private Map getFlatMap(Map identifierMap){ - Map flatMap = new HashMap<>(); - for (Map.Entry entry : identifierMap.entrySet()) { - if (entry.getValue().getKey() == String.class.getName()) { - flatMap.put(entry.getKey(), (String) entry.getValue().getValue()); - } else if (Map.class.isAssignableFrom(entry.getValue().getValue().getClass())){ - Map valMap = (Map) entry.getValue().getValue(); - Map childFlatMap = getFlatMap(valMap); - for (Map.Entry childEntry : childFlatMap.entrySet()){ - String key = String.format("%s.%s", entry.getKey(), childEntry.getKey()); - flatMap.put(key, childEntry.getValue()); - } - } - } - return flatMap; - } - public String getType() { return type; } @@ -214,12 +191,4 @@ public Object getInstance() { public void setInstance(Object instance) { this.instance = instance; } - - public DeserializationInfo getNext() { - return next; - } - - public void setNext(DeserializationInfo next) { - this.next = next; - } } \ No newline at end of file diff --git a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/operation/DeserialisationOperation.java b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/operation/DeserialisationOperation.java index cd317c31b..6ae0f8061 100644 --- a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/operation/DeserialisationOperation.java +++ b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/operation/DeserialisationOperation.java @@ -21,7 +21,7 @@ public DeserialisationOperation(String className, String methodName) { this.entityName = NewRelicSecurity.getAgent().getSecurityMetaData() .peekDeserializingObjectStack().getType(); this.params = NewRelicSecurity.getAgent().getSecurityMetaData() - .peekDeserializingObjectStack().computeAttributeFlatMap(); + .peekDeserializingObjectStack().computeObjectMap(); } this.setCaseType(VulnerabilityCaseType.UNSAFE_DESERIALIZATION); } From c17d614506298a90d130497b82150cdeb036988e Mon Sep 17 00:00:00 2001 From: Anupam Juniwal Date: Wed, 18 Oct 2023 12:07:09 +0530 Subject: [PATCH 04/12] This contains changes for deserializationInfo to be managed with one node only instead of a stack. This also contains changes to include information based on object being deserialized in a parent child, but in an adhoc format, this would be helpful for cases where events are triggered due to deserialization being done. --- .../java/io/Serializable_Instrumentation.java | 32 ++++++++++-------- .../security/schema/AbstractOperation.java | 6 ++-- .../security/schema/DeserializationInfo.java | 19 +++++++++-- .../security/schema/SecurityMetaData.java | 33 ++++++------------- .../operation/DeserialisationOperation.java | 6 ++-- 5 files changed, 50 insertions(+), 46 deletions(-) diff --git a/instrumentation-security/deserialisation/src/main/java/java/io/Serializable_Instrumentation.java b/instrumentation-security/deserialisation/src/main/java/java/io/Serializable_Instrumentation.java index 3a19fc309..5470ceda5 100644 --- a/instrumentation-security/deserialisation/src/main/java/java/io/Serializable_Instrumentation.java +++ b/instrumentation-security/deserialisation/src/main/java/java/io/Serializable_Instrumentation.java @@ -14,31 +14,35 @@ public abstract class Serializable_Instrumentation { private void readSerialData(Object obj, ObjectStreamClass desc) throws IOException { - + DeserializationInfo dInfo = new DeserializationInfo(obj.getClass().getName(), obj); try { if (NewRelicSecurity.isHookProcessingActive() && - !NewRelicSecurity.getAgent().getSecurityMetaData().getRequest().isEmpty()) { - NewRelicSecurity.getAgent().getSecurityMetaData().addToDeserializingObjectStack( - new DeserializationInfo(obj.getClass().getName(), obj) - ); + !NewRelicSecurity.getAgent().getSecurityMetaData().getRequest().isEmpty()) { + dInfo = new DeserializationInfo(obj.getClass().getName(), obj); + NewRelicSecurity.getAgent().getSecurityMetaData().addToDeserializationRoot(dInfo); } - Weaver.callOriginal(); - DeserialisationOperation operation = new DeserialisationOperation( - this.getClass().getName(), - SecurityHelper.METHOD_NAME_READ_OBJECT - ); + Weaver.callOriginal(); - NewRelicSecurity.getAgent().registerOperation(operation); + if (NewRelicSecurity.getAgent().getSecurityMetaData().peekDeserializationRoot() == dInfo) { + DeserialisationOperation operation = new DeserialisationOperation( + this.getClass().getName(), + SecurityHelper.METHOD_NAME_READ_OBJECT + ); - } catch(Exception e){ + NewRelicSecurity.getAgent().registerOperation(operation); + } + } catch (Exception e) { } finally { if (NewRelicSecurity.isHookProcessingActive() && - !NewRelicSecurity.getAgent().getSecurityMetaData().getRequest().isEmpty()) { - NewRelicSecurity.getAgent().getSecurityMetaData().popFromDeserializingObjectStack(); + !NewRelicSecurity.getAgent().getSecurityMetaData().getRequest().isEmpty() && + NewRelicSecurity.getAgent().getSecurityMetaData().peekDeserializationRoot() != null && + NewRelicSecurity.getAgent().getSecurityMetaData().peekDeserializationRoot() == dInfo) { + NewRelicSecurity.getAgent().getSecurityMetaData().resetDeserializationRoot(); } } + // registerExitOperation(isFileLockAcquired, operation); } diff --git a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/AbstractOperation.java b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/AbstractOperation.java index d056e802f..c32f7d63e 100644 --- a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/AbstractOperation.java +++ b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/AbstractOperation.java @@ -2,8 +2,6 @@ import com.newrelic.api.agent.security.NewRelicSecurity; -import java.util.Map; - public abstract class AbstractOperation { public static final String EMPTY = ""; @@ -47,9 +45,9 @@ public AbstractOperation(String className, String methodName){ this.blockingEndTime = 0L; if (NewRelicSecurity.getAgent() != null && NewRelicSecurity.getAgent().getSecurityMetaData() != null && - NewRelicSecurity.getAgent().getSecurityMetaData().peekDeserializingObjectStack() != null) { + NewRelicSecurity.getAgent().getSecurityMetaData().peekDeserializationRoot() != null) { this.deserializationInfo = NewRelicSecurity.getAgent().getSecurityMetaData() - .peekDeserializingObjectStack(); + .peekDeserializationRoot(); this.deserializationInfo.computeObjectMap(); } } diff --git a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/DeserializationInfo.java b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/DeserializationInfo.java index 3084701a7..ea329b0e2 100644 --- a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/DeserializationInfo.java +++ b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/DeserializationInfo.java @@ -31,6 +31,7 @@ public class DeserializationInfo implements Serializable { private boolean leaf; private Map value; private String leafValue; + private List unlinkedChildren = new ArrayList<>(); private Object instance; public DeserializationInfo(String type, Object instance) { @@ -58,9 +59,15 @@ public DeserializationInfo(DeserializationInfo instance) { this.leaf = instance.leaf; if (instance.leaf) { this.leafValue = instance.leafValue; - } else { + } else if (instance.value != null){ this.value = new HashMap<>(); - this.value.putAll(instance.value); + for(Map.Entry entry: instance.value.entrySet()){ + this.value.put(entry.getKey(), new DeserializationInfo(entry.getValue())); + } + } + for(DeserializationInfo value: instance.unlinkedChildren){ + value.computeObjectMap(); + this.unlinkedChildren.add(new DeserializationInfo(value)); } } @@ -191,4 +198,12 @@ public Object getInstance() { public void setInstance(Object instance) { this.instance = instance; } + + public List getUnlinkedChildren() { + return unlinkedChildren; + } + + public void setUnlinkedChildren(List unlinkedChildren) { + this.unlinkedChildren = unlinkedChildren; + } } \ No newline at end of file diff --git a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/SecurityMetaData.java b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/SecurityMetaData.java index 27742dcff..3597544fa 100644 --- a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/SecurityMetaData.java +++ b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/SecurityMetaData.java @@ -1,7 +1,6 @@ package com.newrelic.api.agent.security.schema; -import com.newrelic.api.agent.security.schema.operation.DeserialisationOperation; import com.newrelic.api.agent.security.schema.operation.FileIntegrityOperation; import java.util.HashMap; @@ -112,32 +111,20 @@ public void removeCustomAttribute(String key) { this.customData.remove(key); } - public Map getDeserializingObjectStack() { - if (getCustomAttribute("deserializingObjectStack", Map.class) == null){ - addCustomAttribute("deserializingObjectStack", new HashMap()); + public void addToDeserializationRoot(DeserializationInfo dinfo) { + if (getCustomAttribute("deserializationRoot", DeserializationInfo.class) == null){ + addCustomAttribute("deserializationRoot", dinfo); + } else { + DeserializationInfo root = getCustomAttribute("deserializationRoot", DeserializationInfo.class); + root.getUnlinkedChildren().add(dinfo); } - return getCustomAttribute("deserializingObjectStack", Map.class); } - public void addToDeserializingObjectStack(DeserializationInfo dinfo) { - Map deserializingObjectStack = getDeserializingObjectStack(); - int nextIndex = deserializingObjectStack.size(); - deserializingObjectStack.put(Integer.toString(nextIndex), dinfo); + public void resetDeserializationRoot() { + removeCustomAttribute("deserializationRoot"); } - public void popFromDeserializingObjectStack() { - Map deserializingObjectStack = getDeserializingObjectStack(); - int nextIndex = deserializingObjectStack.size(); - if (nextIndex > 0) { - deserializingObjectStack.remove(Integer.toString(nextIndex-1)); - } - } - - public DeserializationInfo peekDeserializingObjectStack() { - Map deserializingObjectStack = getDeserializingObjectStack(); - if (deserializingObjectStack == null || deserializingObjectStack.isEmpty()) return null; - - int nextIndex = deserializingObjectStack.size(); - return deserializingObjectStack.get(Integer.toString(nextIndex-1)); + public DeserializationInfo peekDeserializationRoot() { + return getCustomAttribute("deserializationRoot", DeserializationInfo.class); } } diff --git a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/operation/DeserialisationOperation.java b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/operation/DeserialisationOperation.java index 6ae0f8061..da35c5529 100644 --- a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/operation/DeserialisationOperation.java +++ b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/operation/DeserialisationOperation.java @@ -17,11 +17,11 @@ public class DeserialisationOperation extends AbstractOperation { public DeserialisationOperation(String className, String methodName) { super(className, methodName); if (NewRelicSecurity.getAgent().getSecurityMetaData()!= null && - NewRelicSecurity.getAgent().getSecurityMetaData().peekDeserializingObjectStack()!=null) { + NewRelicSecurity.getAgent().getSecurityMetaData().peekDeserializationRoot()!=null) { this.entityName = NewRelicSecurity.getAgent().getSecurityMetaData() - .peekDeserializingObjectStack().getType(); + .peekDeserializationRoot().getType(); this.params = NewRelicSecurity.getAgent().getSecurityMetaData() - .peekDeserializingObjectStack().computeObjectMap(); + .peekDeserializationRoot().computeObjectMap(); } this.setCaseType(VulnerabilityCaseType.UNSAFE_DESERIALIZATION); } From 58fa09625e41c7a5d07e75cbecd7ee0e8e680c13 Mon Sep 17 00:00:00 2001 From: Anupam Juniwal Date: Mon, 30 Oct 2023 14:05:50 +0530 Subject: [PATCH 05/12] minor fix for NPE handling --- .../api/agent/security/schema/DeserializationInfo.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/DeserializationInfo.java b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/DeserializationInfo.java index ea329b0e2..0f65906da 100644 --- a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/DeserializationInfo.java +++ b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/DeserializationInfo.java @@ -55,6 +55,9 @@ public DeserializationInfo(String type, Object instance, String value) { } public DeserializationInfo(DeserializationInfo instance) { + if (instance == null) { + return; + } this.type = instance.type; this.leaf = instance.leaf; if (instance.leaf) { From 4cd631fd17fcd23db8d2aaa6e1c05d8e3018f8f9 Mon Sep 17 00:00:00 2001 From: Anupam Juniwal Date: Mon, 27 Nov 2023 11:02:17 +0530 Subject: [PATCH 06/12] Refactoring for java deserialization hook --- .../java/io/Serializable_Instrumentation.java | 60 ++++++++----------- 1 file changed, 25 insertions(+), 35 deletions(-) diff --git a/instrumentation-security/deserialisation/src/main/java/java/io/Serializable_Instrumentation.java b/instrumentation-security/deserialisation/src/main/java/java/io/Serializable_Instrumentation.java index 5470ceda5..e5f5a7f48 100644 --- a/instrumentation-security/deserialisation/src/main/java/java/io/Serializable_Instrumentation.java +++ b/instrumentation-security/deserialisation/src/main/java/java/io/Serializable_Instrumentation.java @@ -1,7 +1,6 @@ package java.io; import com.newrelic.api.agent.security.NewRelicSecurity; -import com.newrelic.api.agent.security.instrumentation.helpers.GenericHelper; import com.newrelic.api.agent.security.schema.DeserializationInfo; import com.newrelic.api.agent.security.schema.operation.DeserialisationOperation; import com.newrelic.api.agent.weaver.MatchType; @@ -14,50 +13,41 @@ public abstract class Serializable_Instrumentation { private void readSerialData(Object obj, ObjectStreamClass desc) throws IOException { - DeserializationInfo dInfo = new DeserializationInfo(obj.getClass().getName(), obj); + DeserializationInfo dInfo = preProcessSecurityHook(obj); try { - if (NewRelicSecurity.isHookProcessingActive() && - !NewRelicSecurity.getAgent().getSecurityMetaData().getRequest().isEmpty()) { - dInfo = new DeserializationInfo(obj.getClass().getName(), obj); - NewRelicSecurity.getAgent().getSecurityMetaData().addToDeserializationRoot(dInfo); - } - Weaver.callOriginal(); - - if (NewRelicSecurity.getAgent().getSecurityMetaData().peekDeserializationRoot() == dInfo) { - DeserialisationOperation operation = new DeserialisationOperation( - this.getClass().getName(), - SecurityHelper.METHOD_NAME_READ_OBJECT - ); - - NewRelicSecurity.getAgent().registerOperation(operation); - } - - } catch (Exception e) { + postProcessSecurityHook(dInfo); } finally { - if (NewRelicSecurity.isHookProcessingActive() && - !NewRelicSecurity.getAgent().getSecurityMetaData().getRequest().isEmpty() && - NewRelicSecurity.getAgent().getSecurityMetaData().peekDeserializationRoot() != null && - NewRelicSecurity.getAgent().getSecurityMetaData().peekDeserializationRoot() == dInfo) { - NewRelicSecurity.getAgent().getSecurityMetaData().resetDeserializationRoot(); - } + finalProcessSecurityHook(dInfo); } + } -// registerExitOperation(isFileLockAcquired, operation); + private DeserializationInfo preProcessSecurityHook(Object obj) { + if (NewRelicSecurity.isHookProcessingActive() && + !NewRelicSecurity.getAgent().getSecurityMetaData().getRequest().isEmpty()) { + DeserializationInfo dInfo = new DeserializationInfo(obj.getClass().getName(), obj); + NewRelicSecurity.getAgent().getSecurityMetaData().addToDeserializationRoot(dInfo); + return dInfo; + } + return null; } - private void releaseLock() { - try { - GenericHelper.releaseLock(SecurityHelper.NR_SEC_CUSTOM_ATTRIB_NAME, this.hashCode()); - } catch (Throwable ignored) { + private void postProcessSecurityHook(DeserializationInfo dInfo) { + if (dInfo != null && NewRelicSecurity.getAgent().getSecurityMetaData().peekDeserializationRoot() == dInfo) { + DeserialisationOperation operation = new DeserialisationOperation( + this.getClass().getName(), + SecurityHelper.METHOD_NAME_READ_OBJECT + ); + NewRelicSecurity.getAgent().registerOperation(operation); } } - private boolean acquireLockIfPossible() { - try { - return GenericHelper.acquireLockIfPossible(SecurityHelper.NR_SEC_CUSTOM_ATTRIB_NAME, this.hashCode()); - } catch (Throwable ignored) { + private void finalProcessSecurityHook(DeserializationInfo dInfo) { + if (dInfo != null && NewRelicSecurity.isHookProcessingActive() && + !NewRelicSecurity.getAgent().getSecurityMetaData().getRequest().isEmpty() && + NewRelicSecurity.getAgent().getSecurityMetaData().peekDeserializationRoot() != null && + NewRelicSecurity.getAgent().getSecurityMetaData().peekDeserializationRoot() == dInfo) { + NewRelicSecurity.getAgent().getSecurityMetaData().resetDeserializationRoot(); } - return false; } } \ No newline at end of file From ee8d3f3862f889bb508775551f949049f4fb1f92 Mon Sep 17 00:00:00 2001 From: ajuniwal Date: Mon, 13 Jan 2025 10:16:39 +0530 Subject: [PATCH 07/12] Added todo note for object deserialiation --- .../newrelic/api/agent/security/schema/DeserializationInfo.java | 1 + 1 file changed, 1 insertion(+) diff --git a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/DeserializationInfo.java b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/DeserializationInfo.java index 0f65906da..c34ecd02f 100644 --- a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/DeserializationInfo.java +++ b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/DeserializationInfo.java @@ -98,6 +98,7 @@ private Map computeKeyValueMappingOnObject(Object o if (depth > MAX_DEPTH_POPULATION){ return new HashMap<>(); } + // TODO: Update this to ObjectMapper.readObject to parse complete deseriaized object and return json str. try { Field[] fields = obj.getClass().getFields(); for (Field field : fields) { From 12559be40be31bfa1525c8c90073aa249be3d623 Mon Sep 17 00:00:00 2001 From: lovesh-ap Date: Wed, 15 Jan 2025 10:18:37 +0530 Subject: [PATCH 08/12] Deserialization POC without reflection --- .../instrumentator/dispatcher/Dispatcher.java | 5 +++++ .../security/schema/AbstractOperation.java | 2 +- .../security/schema/DeserializationInfo.java | 11 +++++++---- .../operation/DeserialisationOperation.java | 17 ++++++++++++++--- 4 files changed, 27 insertions(+), 8 deletions(-) diff --git a/newrelic-security-agent/src/main/java/com/newrelic/agent/security/instrumentator/dispatcher/Dispatcher.java b/newrelic-security-agent/src/main/java/com/newrelic/agent/security/instrumentator/dispatcher/Dispatcher.java index 290996f13..f0275b75e 100644 --- a/newrelic-security-agent/src/main/java/com/newrelic/agent/security/instrumentator/dispatcher/Dispatcher.java +++ b/newrelic-security-agent/src/main/java/com/newrelic/agent/security/instrumentator/dispatcher/Dispatcher.java @@ -657,7 +657,12 @@ private static JavaAgentEventBean prepareSSRFEvent(JavaAgentEventBean eventBean, private static JavaAgentEventBean prepareDeserializationEvent(JavaAgentEventBean eventBean, DeserialisationOperation deserialisationOperation) { + DeserializationInfo rootDeserializationInfo = deserialisationOperation.getRootDeserializationInfo(); JSONArray params = new JSONArray(); + if(rootDeserializationInfo != null) { + eventBean.getMetaData().setDeserializationInfo(rootDeserializationInfo); + params.add(GsonUtil.toJson(rootDeserializationInfo.getInstance())); + } eventBean.setParameters(params); return eventBean; } diff --git a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/AbstractOperation.java b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/AbstractOperation.java index c32f7d63e..abf87b4ba 100644 --- a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/AbstractOperation.java +++ b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/AbstractOperation.java @@ -48,7 +48,7 @@ public AbstractOperation(String className, String methodName){ NewRelicSecurity.getAgent().getSecurityMetaData().peekDeserializationRoot() != null) { this.deserializationInfo = NewRelicSecurity.getAgent().getSecurityMetaData() .peekDeserializationRoot(); - this.deserializationInfo.computeObjectMap(); +// this.deserializationInfo.computeObjectMap(); } } diff --git a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/DeserializationInfo.java b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/DeserializationInfo.java index c34ecd02f..c70583d45 100644 --- a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/DeserializationInfo.java +++ b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/DeserializationInfo.java @@ -1,5 +1,7 @@ package com.newrelic.api.agent.security.schema; +import com.newrelic.api.agent.security.NewRelicSecurity; + import java.io.Serializable; import java.lang.reflect.Field; import java.util.*; @@ -68,10 +70,10 @@ public DeserializationInfo(DeserializationInfo instance) { this.value.put(entry.getKey(), new DeserializationInfo(entry.getValue())); } } - for(DeserializationInfo value: instance.unlinkedChildren){ - value.computeObjectMap(); - this.unlinkedChildren.add(new DeserializationInfo(value)); - } +// for(DeserializationInfo value: instance.unlinkedChildren){ +// value.computeObjectMap(); +// this.unlinkedChildren.add(new DeserializationInfo(value)); +// } } public DeserializationInfo() { @@ -98,6 +100,7 @@ private Map computeKeyValueMappingOnObject(Object o if (depth > MAX_DEPTH_POPULATION){ return new HashMap<>(); } + // TODO: Update this to ObjectMapper.readObject to parse complete deseriaized object and return json str. try { Field[] fields = obj.getClass().getFields(); diff --git a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/operation/DeserialisationOperation.java b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/operation/DeserialisationOperation.java index da35c5529..901c48475 100644 --- a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/operation/DeserialisationOperation.java +++ b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/operation/DeserialisationOperation.java @@ -12,6 +12,7 @@ public class DeserialisationOperation extends AbstractOperation { private String entityName; private Map params; + private DeserializationInfo rootDeserializationInfo; public DeserialisationOperation(String className, String methodName) { @@ -20,15 +21,17 @@ public DeserialisationOperation(String className, String methodName) { NewRelicSecurity.getAgent().getSecurityMetaData().peekDeserializationRoot()!=null) { this.entityName = NewRelicSecurity.getAgent().getSecurityMetaData() .peekDeserializationRoot().getType(); - this.params = NewRelicSecurity.getAgent().getSecurityMetaData() - .peekDeserializationRoot().computeObjectMap(); +// this.params = NewRelicSecurity.getAgent().getSecurityMetaData() +// .peekDeserializationRoot().computeObjectMap(); + this.rootDeserializationInfo = NewRelicSecurity.getAgent().getSecurityMetaData() + .peekDeserializationRoot(); } this.setCaseType(VulnerabilityCaseType.UNSAFE_DESERIALIZATION); } @Override public boolean isEmpty() { - return this.params==null || this.params.isEmpty() || StringUtils.isEmpty(this.entityName); + return this.rootDeserializationInfo==null || StringUtils.isEmpty(this.entityName); } public String getEntityName() { @@ -46,4 +49,12 @@ public Map getParams() { public void setParams(Map params) { this.params = params; } + + public DeserializationInfo getRootDeserializationInfo() { + return rootDeserializationInfo; + } + + public void setRootDeserializationInfo(DeserializationInfo rootDeserializationInfo) { + this.rootDeserializationInfo = rootDeserializationInfo; + } } From d72a9acfcf52ab19d60e5c4f16dcb51d3284d45e Mon Sep 17 00:00:00 2001 From: lovesh-ap Date: Fri, 7 Feb 2025 15:30:08 +0530 Subject: [PATCH 09/12] change parameter schema for Deserialization event --- .../security/instrumentator/dispatcher/Dispatcher.java | 6 ++++-- .../newrelic/api/agent/security/schema/AgentMetaData.java | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/newrelic-security-agent/src/main/java/com/newrelic/agent/security/instrumentator/dispatcher/Dispatcher.java b/newrelic-security-agent/src/main/java/com/newrelic/agent/security/instrumentator/dispatcher/Dispatcher.java index f0275b75e..28bf09a76 100644 --- a/newrelic-security-agent/src/main/java/com/newrelic/agent/security/instrumentator/dispatcher/Dispatcher.java +++ b/newrelic-security-agent/src/main/java/com/newrelic/agent/security/instrumentator/dispatcher/Dispatcher.java @@ -660,8 +660,10 @@ private static JavaAgentEventBean prepareDeserializationEvent(JavaAgentEventBean DeserializationInfo rootDeserializationInfo = deserialisationOperation.getRootDeserializationInfo(); JSONArray params = new JSONArray(); if(rootDeserializationInfo != null) { - eventBean.getMetaData().setDeserializationInfo(rootDeserializationInfo); - params.add(GsonUtil.toJson(rootDeserializationInfo.getInstance())); + JSONObject deserializationInfo = new JSONObject(); + deserializationInfo.put("type", rootDeserializationInfo.getType()); + deserializationInfo.put("value", GsonUtil.toJson(rootDeserializationInfo.getInstance())); + params.add(deserializationInfo); } eventBean.setParameters(params); return eventBean; diff --git a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/AgentMetaData.java b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/AgentMetaData.java index f53fa2524..1d087f33a 100644 --- a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/AgentMetaData.java +++ b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/AgentMetaData.java @@ -49,6 +49,7 @@ public class AgentMetaData { private AppServerInfo appServerInfo; + @JsonIgnore private DeserializationInfo deserializationInfo = new DeserializationInfo(); public AgentMetaData() { From bfb2acd5c1c96b065ab5419e03b7ff6c26e75dfc Mon Sep 17 00:00:00 2001 From: idawda Date: Tue, 11 Feb 2025 14:32:40 +0530 Subject: [PATCH 10/12] Bump json version to 1.2.11 --- gradle.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle.properties b/gradle.properties index 220b28d15..4bd015446 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,6 +1,6 @@ # The agent version. agentVersion=1.6.0 -jsonVersion=1.2.9 +jsonVersion=1.2.11 # Updated exposed NR APM API version. nrAPIVersion=8.12.0 From fd3d5729042fe5136dab6a09ce89df97ccc4d8d1 Mon Sep 17 00:00:00 2001 From: lovesh-ap Date: Mon, 24 Feb 2025 23:29:37 +0530 Subject: [PATCH 11/12] Prodcutisation of DeserializationInfo --- ...=> ObjectInputStream_Instrumentation.java} | 41 +++-- .../instrumentator/dispatcher/Dispatcher.java | 1 - .../agent/security/schema/AgentMetaData.java | 11 -- .../security/schema/DeserializationInfo.java | 170 +----------------- 4 files changed, 30 insertions(+), 193 deletions(-) rename instrumentation-security/deserialisation/src/main/java/java/io/{Serializable_Instrumentation.java => ObjectInputStream_Instrumentation.java} (53%) diff --git a/instrumentation-security/deserialisation/src/main/java/java/io/Serializable_Instrumentation.java b/instrumentation-security/deserialisation/src/main/java/java/io/ObjectInputStream_Instrumentation.java similarity index 53% rename from instrumentation-security/deserialisation/src/main/java/java/io/Serializable_Instrumentation.java rename to instrumentation-security/deserialisation/src/main/java/java/io/ObjectInputStream_Instrumentation.java index e5f5a7f48..e4037bd38 100644 --- a/instrumentation-security/deserialisation/src/main/java/java/io/Serializable_Instrumentation.java +++ b/instrumentation-security/deserialisation/src/main/java/java/io/ObjectInputStream_Instrumentation.java @@ -1,7 +1,10 @@ package java.io; import com.newrelic.api.agent.security.NewRelicSecurity; +import com.newrelic.api.agent.security.instrumentation.helpers.GenericHelper; +import com.newrelic.api.agent.security.schema.AbstractOperation; import com.newrelic.api.agent.security.schema.DeserializationInfo; +import com.newrelic.api.agent.security.schema.VulnerabilityCaseType; import com.newrelic.api.agent.security.schema.operation.DeserialisationOperation; import com.newrelic.api.agent.weaver.MatchType; import com.newrelic.api.agent.weaver.Weave; @@ -9,45 +12,55 @@ @Weave(type = MatchType.ExactClass, originalName = "java.io.ObjectInputStream") -public abstract class Serializable_Instrumentation { +public abstract class ObjectInputStream_Instrumentation { private void readSerialData(Object obj, ObjectStreamClass desc) throws IOException { - DeserializationInfo dInfo = preProcessSecurityHook(obj); + boolean isLockAcquired = acquireLockIfPossible(); + AbstractOperation operation = null; + DeserializationInfo dInfo = null; + if(isLockAcquired) { + dInfo = preProcessSecurityHook(obj); + } try { Weaver.callOriginal(); - postProcessSecurityHook(dInfo); + operation = postProcessSecurityHook(dInfo); } finally { - finalProcessSecurityHook(dInfo); + if(isLockAcquired) { + finalProcessSecurityHook(dInfo); + GenericHelper.releaseLock(SecurityHelper.NR_SEC_CUSTOM_ATTRIB_NAME); + } } + //TODO add register exit operation if required } private DeserializationInfo preProcessSecurityHook(Object obj) { - if (NewRelicSecurity.isHookProcessingActive() && - !NewRelicSecurity.getAgent().getSecurityMetaData().getRequest().isEmpty()) { - DeserializationInfo dInfo = new DeserializationInfo(obj.getClass().getName(), obj); - NewRelicSecurity.getAgent().getSecurityMetaData().addToDeserializationRoot(dInfo); - return dInfo; - } - return null; + DeserializationInfo dInfo = new DeserializationInfo(obj.getClass().getName(), obj); + NewRelicSecurity.getAgent().getSecurityMetaData().addToDeserializationRoot(dInfo); + return dInfo; } - private void postProcessSecurityHook(DeserializationInfo dInfo) { + private DeserialisationOperation postProcessSecurityHook(DeserializationInfo dInfo) { if (dInfo != null && NewRelicSecurity.getAgent().getSecurityMetaData().peekDeserializationRoot() == dInfo) { DeserialisationOperation operation = new DeserialisationOperation( this.getClass().getName(), SecurityHelper.METHOD_NAME_READ_OBJECT ); NewRelicSecurity.getAgent().registerOperation(operation); + return operation; } + return null; } private void finalProcessSecurityHook(DeserializationInfo dInfo) { - if (dInfo != null && NewRelicSecurity.isHookProcessingActive() && - !NewRelicSecurity.getAgent().getSecurityMetaData().getRequest().isEmpty() && + if (dInfo != null && NewRelicSecurity.getAgent().getSecurityMetaData().peekDeserializationRoot() != null && NewRelicSecurity.getAgent().getSecurityMetaData().peekDeserializationRoot() == dInfo) { NewRelicSecurity.getAgent().getSecurityMetaData().resetDeserializationRoot(); } } + + private boolean acquireLockIfPossible() { + return GenericHelper.acquireLockIfPossible(VulnerabilityCaseType.UNSAFE_DESERIALIZATION, SecurityHelper.NR_SEC_CUSTOM_ATTRIB_NAME); + } } \ No newline at end of file diff --git a/newrelic-security-agent/src/main/java/com/newrelic/agent/security/instrumentator/dispatcher/Dispatcher.java b/newrelic-security-agent/src/main/java/com/newrelic/agent/security/instrumentator/dispatcher/Dispatcher.java index 28bf09a76..a9bc3cf9f 100644 --- a/newrelic-security-agent/src/main/java/com/newrelic/agent/security/instrumentator/dispatcher/Dispatcher.java +++ b/newrelic-security-agent/src/main/java/com/newrelic/agent/security/instrumentator/dispatcher/Dispatcher.java @@ -764,7 +764,6 @@ private JavaAgentEventBean setGenericProperties(AbstractOperation objectBean, Ja eventBean.setUserAPIInfo(operation.getUserClassEntity().getUserClassElement().getLineNumber(), operation.getUserClassEntity().getUserClassElement().getClassName(), operation.getUserClassEntity().getUserClassElement().getMethodName()); - eventBean.getMetaData().setDeserializationInfo(operation.getDeserializationInfo()); eventBean.getLinkingMetadata().put(NR_APM_TRACE_ID, securityMetaData.getCustomAttribute(NR_APM_TRACE_ID, String.class)); eventBean.getLinkingMetadata().put(NR_APM_SPAN_ID, securityMetaData.getCustomAttribute(NR_APM_SPAN_ID, String.class)); return eventBean; diff --git a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/AgentMetaData.java b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/AgentMetaData.java index 1d087f33a..b05381bcd 100644 --- a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/AgentMetaData.java +++ b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/AgentMetaData.java @@ -49,9 +49,6 @@ public class AgentMetaData { private AppServerInfo appServerInfo; - @JsonIgnore - private DeserializationInfo deserializationInfo = new DeserializationInfo(); - public AgentMetaData() { this.rciMethodsCalls = new HashSet<>(); this.ips = new HashSet<>(); @@ -82,7 +79,6 @@ public AgentMetaData(AgentMetaData agentMetaData) { this.fromJumpRequiredInStackTrace = agentMetaData.getFromJumpRequiredInStackTrace(); this.framework = agentMetaData.framework; this.skipScanParameters = agentMetaData.skipScanParameters; - this.deserializationInfo = new DeserializationInfo(agentMetaData.deserializationInfo); } public boolean isTriggerViaRCI() { @@ -234,11 +230,4 @@ public void setSkipScanParameters(SkipScanParameters skipScanParameters) { this.skipScanParameters = skipScanParameters; } - public DeserializationInfo getDeserializationInfo() { - return deserializationInfo; - } - - public void setDeserializationInfo(DeserializationInfo deserializationInfo) { - this.deserializationInfo = new DeserializationInfo(deserializationInfo); - } } diff --git a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/DeserializationInfo.java b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/DeserializationInfo.java index c70583d45..b23acc615 100644 --- a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/DeserializationInfo.java +++ b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/DeserializationInfo.java @@ -1,59 +1,18 @@ package com.newrelic.api.agent.security.schema; -import com.newrelic.api.agent.security.NewRelicSecurity; +import java.util.ArrayList; +import java.util.List; -import java.io.Serializable; -import java.lang.reflect.Field; -import java.util.*; -public class DeserializationInfo implements Serializable { - final static int MAX_DEPTH_POPULATION = 10; - final static Set PRIMITIVE_WRAPPERS = new HashSet() {{ - add(Boolean.class); - add(Character.class); - add(Byte.class); - add(Short.class); - add(Integer.class); - add(Long.class); - add(Float.class); - add(Double.class); - add(Void.class); - add(boolean.class); - add(char.class); - add(byte.class); - add(short.class); - add(int.class); - add(long.class); - add(float.class); - add(double.class); - add(void.class); - }}; +public class DeserializationInfo { private String type; - private boolean leaf; - private Map value; - private String leafValue; private List unlinkedChildren = new ArrayList<>(); private Object instance; public DeserializationInfo(String type, Object instance) { this.type = type; this.instance = instance; - this.leaf = false; - } - - public DeserializationInfo(String type, Object instance, Map value) { - this.type = type; - this.instance = instance; - this.leaf = false; - this.value = value; - } - - public DeserializationInfo(String type, Object instance, String value) { - this.type = type; - this.instance = instance; - this.leaf = true; - this.leafValue = value; } public DeserializationInfo(DeserializationInfo instance) { @@ -61,111 +20,12 @@ public DeserializationInfo(DeserializationInfo instance) { return; } this.type = instance.type; - this.leaf = instance.leaf; - if (instance.leaf) { - this.leafValue = instance.leafValue; - } else if (instance.value != null){ - this.value = new HashMap<>(); - for(Map.Entry entry: instance.value.entrySet()){ - this.value.put(entry.getKey(), new DeserializationInfo(entry.getValue())); - } - } // for(DeserializationInfo value: instance.unlinkedChildren){ // value.computeObjectMap(); // this.unlinkedChildren.add(new DeserializationInfo(value)); // } } - public DeserializationInfo() { - this.type = ""; - this.leaf = false; - this.value = new HashMap<>(); - this.leafValue = ""; - } - - - public Map computeObjectMap() { - if (this.value == null || this.leafValue == null || this.leafValue.isEmpty()) { - try { - this.value = computeKeyValueMappingOnObject(this.instance, 0); - } catch (IllegalAccessException e) { - throw new RuntimeException(e); - } - } - return this.value; - } - - private Map computeKeyValueMappingOnObject(Object obj, int depth) throws IllegalAccessException { - Map valueMap = new HashMap<>(); - if (depth > MAX_DEPTH_POPULATION){ - return new HashMap<>(); - } - - // TODO: Update this to ObjectMapper.readObject to parse complete deseriaized object and return json str. - try { - Field[] fields = obj.getClass().getFields(); - for (Field field : fields) { - boolean accessibility = field.isAccessible(); - field.setAccessible(true); - Object val = field.get(obj); - populateForField(val, field.getName(), valueMap, depth++); - field.setAccessible(accessibility); - } - List fieldList = getAllPrivateFields(obj.getClass()); - for (Field field : fieldList) { - boolean accessibility = field.isAccessible(); - field.setAccessible(true); - Object val = field.get(obj); - populateForField(val, field.getName(), valueMap, depth++); - field.setAccessible(accessibility); - } - } catch (Exception e) {} - return valueMap; - } - - private void populateForField(Object obj, String name, Map valueMap, int depth) throws IllegalAccessException { - if (depth > MAX_DEPTH_POPULATION || obj == null || name == null || valueMap == null){ - return; - } - boolean primitiveObj = obj.getClass().isPrimitive() || PRIMITIVE_WRAPPERS.contains(obj.getClass()); - if (primitiveObj) { - return; - } - - if (obj.getClass() == String.class) { - DeserializationInfo entry = new DeserializationInfo(obj.getClass().getName(), obj, (String) obj); - valueMap.put(name, entry); - } else if (Collection.class.isAssignableFrom(obj.getClass()) || obj.getClass().isArray()) { - Object col[]; - if (obj.getClass().isArray()) { - col = (Object []) obj; - } else { - col = ((Collection) obj).toArray(); - } - Map collectionMap = new HashMap<>(); - for (int elementIndex=0; elementIndex getAllPrivateFields(Class clz){ - List fields = new ArrayList<>(); - fields.addAll(Arrays.asList(clz.getDeclaredFields())); - Class superClass = clz.getSuperclass(); - if (superClass != null && (PRIMITIVE_WRAPPERS.contains(superClass) || superClass == Object.class)){ - fields.addAll(getAllPrivateFields(superClass)); - } - return fields; - } - public String getType() { return type; } @@ -174,30 +34,6 @@ public void setType(String type) { this.type = type; } - public boolean isLeaf() { - return leaf; - } - - public void setLeaf(boolean leaf) { - this.leaf = leaf; - } - - public Map getValue() { - return value; - } - - public void setValue(Map value) { - this.value = value; - } - - public String getLeafValue() { - return leafValue; - } - - public void setLeafValue(String leafValue) { - this.leafValue = leafValue; - } - public Object getInstance() { return instance; } From 4577300fa3152d85fcc6d55785ae47533ef9357d Mon Sep 17 00:00:00 2001 From: lovesh-ap Date: Wed, 26 Feb 2025 10:45:43 +0530 Subject: [PATCH 12/12] Add UNSAFE_DESERIALIZATION as a new category --- .../instrumentation/helpers/GenericHelper.java | 3 +++ .../schema/policy/IastDetectionCategory.java | 13 +++++++++++++ 2 files changed, 16 insertions(+) diff --git a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/instrumentation/helpers/GenericHelper.java b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/instrumentation/helpers/GenericHelper.java index 6bbaca47c..6cc61f8f6 100644 --- a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/instrumentation/helpers/GenericHelper.java +++ b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/instrumentation/helpers/GenericHelper.java @@ -104,6 +104,9 @@ private static boolean isLockAcquirePossible(VulnerabilityCaseType caseType) { case HASH: enabled = NewRelicSecurity.getAgent().getIastDetectionCategory().getInsecureSettingsEnabled(); break; + case UNSAFE_DESERIALIZATION: + enabled = NewRelicSecurity.getAgent().getIastDetectionCategory().getUnsafeDeserializationEnabled(); + break; default: break; } diff --git a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/policy/IastDetectionCategory.java b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/policy/IastDetectionCategory.java index 25d0439d4..a8e23c963 100644 --- a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/policy/IastDetectionCategory.java +++ b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/policy/IastDetectionCategory.java @@ -16,6 +16,7 @@ public class IastDetectionCategory { Boolean javascriptInjectionEnabled = false; Boolean xpathInjectionEnabled = false; Boolean ssrfEnabled = false; + Boolean unsafeDeserializationEnabled = false; @JsonIgnore private String disabledCategoriesCSV; @@ -157,6 +158,10 @@ public void generateDisabledCategoriesCSV() { disabledCategoriesCSVBuilder.append(VulnerabilityCaseType.HTTP_REQUEST); disabledCategoriesCSVBuilder.append(STR_COMMA); } + if (unsafeDeserializationEnabled) { + disabledCategoriesCSVBuilder.append(VulnerabilityCaseType.UNSAFE_DESERIALIZATION); + disabledCategoriesCSVBuilder.append(STR_COMMA); + } if (disabledCategoriesCSVBuilder.length() > 0) { disabledCategoriesCSVBuilder.deleteCharAt(disabledCategoriesCSVBuilder.length() - 1); } @@ -166,4 +171,12 @@ public void generateDisabledCategoriesCSV() { public String getDisabledCategoriesCSV() { return disabledCategoriesCSV; } + + public Boolean getUnsafeDeserializationEnabled() { + return unsafeDeserializationEnabled; + } + + public void setUnsafeDeserializationEnabled(Boolean unsafeDeserializationEnabled) { + this.unsafeDeserializationEnabled = unsafeDeserializationEnabled; + } }