diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java index 9baf349f8b9..ec5ab19ca09 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java @@ -529,7 +529,7 @@ public void execute() { installDatadogTracer(initTelemetry, scoClass, sco); maybeStartAppSec(scoClass, sco); - maybeStartIast(scoClass, sco); + maybeStartIast(instrumentation, scoClass, sco); maybeStartCiVisibility(instrumentation, scoClass, sco); maybeStartLogsIntake(scoClass, sco); // start debugger before remote config to subscribe to it before starting to poll @@ -820,14 +820,14 @@ private static boolean isSupportedAppSecArch() { return true; } - private static void maybeStartIast(Class scoClass, Object o) { + private static void maybeStartIast(Instrumentation instrumentation, Class scoClass, Object o) { if (iastEnabled || !iastFullyDisabled) { StaticEventLogger.begin("IAST"); try { SubscriptionService ss = AgentTracer.get().getSubscriptionService(RequestContextSlot.IAST); - startIast(ss, scoClass, o); + startIast(instrumentation, ss, scoClass, o); } catch (Exception e) { log.error("Error starting IAST subsystem", e); } @@ -836,12 +836,13 @@ private static void maybeStartIast(Class scoClass, Object o) { } } - private static void startIast(SubscriptionService ss, Class scoClass, Object sco) { + private static void startIast( + Instrumentation instrumentation, SubscriptionService ss, Class scoClass, Object sco) { try { final Class appSecSysClass = AGENT_CLASSLOADER.loadClass("com.datadog.iast.IastSystem"); final Method iastInstallerMethod = - appSecSysClass.getMethod("start", SubscriptionService.class); - iastInstallerMethod.invoke(null, ss); + appSecSysClass.getMethod("start", Instrumentation.class, SubscriptionService.class); + iastInstallerMethod.invoke(null, instrumentation, ss); } catch (final Throwable e) { log.warn("Not starting IAST subsystem", e); } diff --git a/dd-java-agent/agent-iast/build.gradle b/dd-java-agent/agent-iast/build.gradle index 659a568d947..01dd53138ba 100644 --- a/dd-java-agent/agent-iast/build.gradle +++ b/dd-java-agent/agent-iast/build.gradle @@ -50,6 +50,7 @@ dependencies { implementation project(':internal-api') implementation project(':internal-api:internal-api-9') implementation libs.moshi + implementation libs.bundles.asm testFixturesApi project(':dd-java-agent:testing') testFixturesApi project(':utils:test-utils') diff --git a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/IastSystem.java b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/IastSystem.java index 806fb36b2e8..118be0e231b 100644 --- a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/IastSystem.java +++ b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/IastSystem.java @@ -8,6 +8,7 @@ import com.datadog.iast.propagation.CodecModuleImpl; import com.datadog.iast.propagation.PropagationModuleImpl; import com.datadog.iast.propagation.StringModuleImpl; +import com.datadog.iast.securitycontrol.IastSecurityControlTransformer; import com.datadog.iast.sink.ApplicationModuleImpl; import com.datadog.iast.sink.CommandInjectionModuleImpl; import com.datadog.iast.sink.HardcodedSecretModuleImpl; @@ -47,12 +48,17 @@ import datadog.trace.api.iast.IastContext; import datadog.trace.api.iast.IastModule; import datadog.trace.api.iast.InstrumentationBridge; +import datadog.trace.api.iast.securitycontrol.SecurityControl; +import datadog.trace.api.iast.securitycontrol.SecurityControlFormatter; import datadog.trace.api.iast.telemetry.IastMetricCollector; import datadog.trace.api.iast.telemetry.Verbosity; import datadog.trace.util.AgentTaskScheduler; import datadog.trace.util.stacktrace.StackWalkerFactory; +import java.lang.instrument.Instrumentation; import java.lang.reflect.Constructor; import java.lang.reflect.UndeclaredThrowableException; +import java.util.List; +import java.util.Map; import java.util.function.BiFunction; import java.util.function.Supplier; import java.util.stream.Stream; @@ -67,11 +73,21 @@ public class IastSystem { public static Verbosity VERBOSITY = Verbosity.OFF; public static void start(final SubscriptionService ss) { - start(ss, null); + start(null, ss, null); + } + + public static void start(final SubscriptionService ss, OverheadController overheadController) { + start(null, ss, overheadController); + } + + public static void start(final Instrumentation instrumentation, final SubscriptionService ss) { + start(instrumentation, ss, null); } public static void start( - final SubscriptionService ss, @Nullable OverheadController overheadController) { + @Nullable final Instrumentation instrumentation, + final SubscriptionService ss, + @Nullable OverheadController overheadController) { final Config config = Config.get(); final ProductActivation iast = config.getIastActivation(); final ProductActivation appSec = config.getAppSecActivation(); @@ -106,9 +122,23 @@ public static void start( registerRequestEndedCallback(ss, addTelemetry, dependencies); registerHeadersCallback(ss); registerGrpcServerRequestMessageCallback(ss); + maybeApplySecurityControls(instrumentation); LOGGER.debug("IAST started"); } + private static void maybeApplySecurityControls(@Nullable Instrumentation instrumentation) { + if (Config.get().getIastSecurityControlsConfiguration() == null || instrumentation == null) { + return; + } + Map> securityControls = + SecurityControlFormatter.format(Config.get().getIastSecurityControlsConfiguration()); + if (securityControls == null) { + LOGGER.warn("No security controls to apply"); + return; + } + instrumentation.addTransformer(new IastSecurityControlTransformer(securityControls), true); + } + private static IastContext.Provider contextProvider( final ProductActivation iast, final boolean global) { if (iast != FULLY_ENABLED) { diff --git a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/propagation/PropagationModuleImpl.java b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/propagation/PropagationModuleImpl.java index e0d1668d6fd..1503fd12909 100644 --- a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/propagation/PropagationModuleImpl.java +++ b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/propagation/PropagationModuleImpl.java @@ -652,6 +652,22 @@ public Taintable.Source findSource( return highestPrioritySource(to, target); } + @Override + public void markIfTainted(@Nullable Object target, int mark) { + if (target == null) { + return; + } + final IastContext ctx = IastContext.Provider.get(); + if (ctx == null) { + return; + } + TaintedObjects taintedObjects = ctx.getTaintedObjects(); + TaintedObject taintedObject = taintedObjects.get(target); + if (taintedObject != null) { + taintedObject.setRanges(markRanges(taintedObject.getRanges(), mark)); + } + } + @Override public boolean isTainted(@Nullable final Object target) { if (target == null) { diff --git a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/securitycontrol/AbstractMethodAdapter.java b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/securitycontrol/AbstractMethodAdapter.java new file mode 100644 index 00000000000..0cd8663b7e5 --- /dev/null +++ b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/securitycontrol/AbstractMethodAdapter.java @@ -0,0 +1,45 @@ +package com.datadog.iast.securitycontrol; + +import datadog.trace.api.iast.securitycontrol.SecurityControl; +import org.objectweb.asm.MethodVisitor; +import org.objectweb.asm.Opcodes; +import org.objectweb.asm.Type; + +public class AbstractMethodAdapter extends MethodVisitor { + + private static final String HELPER = + "datadog/trace/api/iast/securitycontrol/SecurityControlHelper"; + private static final String METHOD = "setSecureMarks"; + private static final String DESCRIPTOR = "(Ljava/lang/Object;I)V"; + protected final MethodVisitor mv; + protected final SecurityControl securityControl; + protected final int accessFlags; + protected final Type method; + + public AbstractMethodAdapter( + final MethodVisitor mv, + final SecurityControl securityControl, + final int accessFlags, + final Type method) { + super(Opcodes.ASM9, mv); + this.mv = mv; + this.securityControl = securityControl; + this.accessFlags = accessFlags; + this.method = method; + } + + protected boolean isStatic() { + return (accessFlags & Opcodes.ACC_STATIC) > 0; + } + + /** + * This method loads the current secure marks defined in the control and then calls the helper + * method + */ + protected void loadMarksAndCallHelper() { + // Load the marks from securityControl onto the stack + mv.visitLdcInsn(securityControl.getMarks()); + // Insert the call to setSecureMarks with the return value and marks as parameters + mv.visitMethodInsn(Opcodes.INVOKESTATIC, HELPER, METHOD, DESCRIPTOR, false); + } +} diff --git a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/securitycontrol/IastSecurityControlTransformer.java b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/securitycontrol/IastSecurityControlTransformer.java new file mode 100644 index 00000000000..b15e7abeeb6 --- /dev/null +++ b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/securitycontrol/IastSecurityControlTransformer.java @@ -0,0 +1,51 @@ +package com.datadog.iast.securitycontrol; + +import static org.objectweb.asm.ClassReader.SKIP_DEBUG; +import static org.objectweb.asm.ClassReader.SKIP_FRAMES; + +import datadog.trace.api.iast.securitycontrol.SecurityControl; +import java.lang.instrument.ClassFileTransformer; +import java.util.List; +import java.util.Map; +import javax.annotation.Nullable; +import org.objectweb.asm.ClassReader; +import org.objectweb.asm.ClassVisitor; +import org.objectweb.asm.ClassWriter; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class IastSecurityControlTransformer implements ClassFileTransformer { + + private static final Logger LOGGER = + LoggerFactory.getLogger(IastSecurityControlTransformer.class); + + private final Map> securityControls; + + public IastSecurityControlTransformer(Map> securityControls) { + this.securityControls = securityControls; + } + + @Override + @Nullable + public byte[] transform( + ClassLoader loader, + String className, + Class classBeingRedefined, + java.security.ProtectionDomain protectionDomain, + byte[] classfileBuffer) { + List match = securityControls.get(className); + if (match == null || match.isEmpty()) { + return null; // Do not transform classes that do not have a security control + } + try { + ClassReader cr = new ClassReader(classfileBuffer); + ClassWriter cw = new ClassWriter(cr, ClassWriter.COMPUTE_FRAMES); + ClassVisitor cv = new SecurityControlMethodClassVisitor(cw, match); + cr.accept(cv, SKIP_DEBUG | SKIP_FRAMES); + return cw.toByteArray(); + } catch (Throwable e) { + LOGGER.warn("Failed to transform class: {}", className, e); + return null; + } + } +} diff --git a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/securitycontrol/InputValidatorMethodAdapter.java b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/securitycontrol/InputValidatorMethodAdapter.java new file mode 100644 index 00000000000..def0bde8ba6 --- /dev/null +++ b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/securitycontrol/InputValidatorMethodAdapter.java @@ -0,0 +1,63 @@ +package com.datadog.iast.securitycontrol; + +import static com.datadog.iast.securitycontrol.SecurityControlMethodClassVisitor.LOGGER; +import static com.datadog.iast.securitycontrol.SecurityControlMethodClassVisitor.isPrimitive; + +import datadog.trace.api.iast.securitycontrol.SecurityControl; +import org.objectweb.asm.MethodVisitor; +import org.objectweb.asm.Opcodes; +import org.objectweb.asm.Type; + +public class InputValidatorMethodAdapter extends AbstractMethodAdapter { + + private final boolean isStatic; + + public InputValidatorMethodAdapter( + final MethodVisitor mv, + final SecurityControl securityControl, + final int accessFlags, + final Type method) { + super(mv, securityControl, accessFlags, method); + isStatic = isStatic(); + } + + @Override + public void visitCode() { + super.visitCode(); + processInputValidator(); + } + + private void processInputValidator() { + boolean allParameters = securityControl.getParametersToMark() == null; + Type[] types = method.getArgumentTypes(); + int parametersCount = 0; + for (int i = 0; i < types.length; i++) { + Type type = types[i]; + boolean isPrimitive = isPrimitive(type); + if (allParameters) { + if (!isPrimitive) { + callInputValidation(parametersCount); + } + } else if (securityControl.getParametersToMark().get(i)) { + if (isPrimitive) { + LOGGER.warn( + "Input validators should not be used on primitive types. Parameter {} with type {} .Security control: {}", + i, + type.getClassName(), + securityControl); + } else { + callInputValidation(parametersCount); + } + } + parametersCount += type.getSize(); + } + } + + private void callInputValidation(int i) { + // Load the parameter onto the stack + mv.visitVarInsn( + Opcodes.ALOAD, + isStatic ? i : i + 1); // instance methods have this as first element in the stack + loadMarksAndCallHelper(); + } +} diff --git a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/securitycontrol/SanitizerMethodAdapter.java b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/securitycontrol/SanitizerMethodAdapter.java new file mode 100644 index 00000000000..26502759f42 --- /dev/null +++ b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/securitycontrol/SanitizerMethodAdapter.java @@ -0,0 +1,31 @@ +package com.datadog.iast.securitycontrol; + +import datadog.trace.api.iast.securitycontrol.SecurityControl; +import org.objectweb.asm.MethodVisitor; +import org.objectweb.asm.Opcodes; +import org.objectweb.asm.Type; + +public class SanitizerMethodAdapter extends AbstractMethodAdapter { + + public SanitizerMethodAdapter( + final MethodVisitor mv, + final SecurityControl securityControl, + final int accessFlags, + final Type method) { + super(mv, securityControl, accessFlags, method); + } + + @Override + public void visitInsn(int opcode) { + if (opcode == Opcodes.ARETURN) { + processSanitizer(); + } + super.visitInsn(opcode); + } + + private void processSanitizer() { + // Duplicate the return value on the stack + mv.visitInsn(Opcodes.DUP); + loadMarksAndCallHelper(); + } +} diff --git a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/securitycontrol/SecurityControlMethodClassVisitor.java b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/securitycontrol/SecurityControlMethodClassVisitor.java new file mode 100644 index 00000000000..0a1c825fd55 --- /dev/null +++ b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/securitycontrol/SecurityControlMethodClassVisitor.java @@ -0,0 +1,111 @@ +package com.datadog.iast.securitycontrol; + +import static org.objectweb.asm.Opcodes.ASM8; + +import datadog.trace.api.iast.securitycontrol.SecurityControl; +import datadog.trace.api.iast.securitycontrol.SecurityControlType; +import java.util.List; +import javax.annotation.Nullable; +import org.objectweb.asm.ClassVisitor; +import org.objectweb.asm.ClassWriter; +import org.objectweb.asm.MethodVisitor; +import org.objectweb.asm.Type; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class SecurityControlMethodClassVisitor extends ClassVisitor { + + static final Logger LOGGER = LoggerFactory.getLogger(SecurityControlMethodClassVisitor.class); + + private final List securityControls; + + public SecurityControlMethodClassVisitor( + final ClassWriter cw, final List securityControls) { + super(ASM8, cw); + this.securityControls = securityControls; + } + + @Override + @Nullable + public MethodVisitor visitMethod( + int access, String name, String desc, String signature, String[] exceptions) { + MethodVisitor mv = super.visitMethod(access, name, desc, signature, exceptions); + if (mv == null) { + return null; + } + SecurityControl match = null; + for (SecurityControl securityControl : securityControls) { + if (shouldBeAdapted(securityControl, name, desc)) { + match = securityControl; + break; + } + } + if (match != null) { + final Type method = Type.getMethodType(desc); + if (match.getType() == SecurityControlType.SANITIZER) { + mv = adaptSanitizer(mv, match, access, method); + } else { + mv = adaptInputValidator(mv, match, access, method); + } + } + return mv; + } + + public void visitEnd() { + cv.visitEnd(); + } + + private boolean shouldBeAdapted(SecurityControl securityControl, String name, String desc) { + + if (!securityControl.getMethod().equals(name)) { + return false; + } + + if (securityControl.getParameterTypes() == null) { + return true; + } + + Type[] types = Type.getArgumentTypes(desc); + if (types.length != securityControl.getParameterTypes().size()) { + return false; + } + + for (int i = 0; i < types.length; i++) { + if (!types[i].getClassName().equals(securityControl.getParameterTypes().get(i))) { + return false; + } + } + + return true; + } + + private MethodVisitor adaptSanitizer( + final MethodVisitor mv, + final SecurityControl control, + final int accessFlags, + final Type method) { + if (isPrimitive(method.getReturnType())) { + // no need to check primitives as we are not tainting them + LOGGER.warn( + "Sanitizers should not be used on primitive return types. Return type {}. Security control: {}", + method.getReturnType().getClassName(), + control); + return mv; + } + return new SanitizerMethodAdapter(mv, control, accessFlags, method); + } + + private MethodVisitor adaptInputValidator( + final MethodVisitor mv, + final SecurityControl control, + final int accessFlags, + final Type method) { + return new InputValidatorMethodAdapter(mv, control, accessFlags, method); + } + + public static boolean isPrimitive(final Type type) { + // Check if is a primitive type + int sort = type.getSort(); + return sort >= Type.BOOLEAN && sort <= Type.DOUBLE; + } +} diff --git a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/IastSystemTest.groovy b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/IastSystemTest.groovy index 71914a424a2..38014536f16 100644 --- a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/IastSystemTest.groovy +++ b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/IastSystemTest.groovy @@ -136,7 +136,7 @@ class IastSystemTest extends DDSpecification { InstrumentationBridge.clearIastModules() when: - Agent.maybeStartIast(null, null) + Agent.maybeStartIast(null, null, null) then: InstrumentationBridge.iastModules.each { diff --git a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/securitycontrol/IastSecurityControlTransformerForkedTest.groovy b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/securitycontrol/IastSecurityControlTransformerForkedTest.groovy new file mode 100644 index 00000000000..d87583eabd4 --- /dev/null +++ b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/securitycontrol/IastSecurityControlTransformerForkedTest.groovy @@ -0,0 +1,131 @@ +package com.datadog.iast.securitycontrol + +import datadog.trace.api.iast.InstrumentationBridge +import datadog.trace.api.iast.VulnerabilityMarks +import datadog.trace.api.iast.propagation.PropagationModule +import datadog.trace.api.iast.securitycontrol.SecurityControl +import datadog.trace.api.iast.securitycontrol.SecurityControlFormatter +import datadog.trace.test.util.DDSpecification +import foo.bar.securitycontrol.SecurityControlStaticTestSuite +import foo.bar.securitycontrol.SecurityControlTestSuite +import net.bytebuddy.agent.ByteBuddyAgent + +import java.lang.instrument.Instrumentation + +class IastSecurityControlTransformerForkedTest extends DDSpecification{ + + //static methods + private static final String STATIC_SANITIZER = 'SANITIZER:XSS:foo.bar.securitycontrol.SecurityControlStaticTestSuite:sanitize' + private static final String STATIC_SANITIZE_VALIDATE_OBJECT = 'SANITIZER:XSS:foo.bar.securitycontrol.SecurityControlStaticTestSuite:sanitizeObject' + private static final String STATIC_SANITIZE_INPUTS= 'SANITIZER:XSS:foo.bar.securitycontrol.SecurityControlStaticTestSuite:sanitizeInputs' + private static final String STATIC_SANITIZE_MANY_INPUTS= 'SANITIZER:XSS:foo.bar.securitycontrol.SecurityControlStaticTestSuite:sanitizeManyInputs' + private static final String STATIC_SANITIZE_INT = 'SANITIZER:XSS:foo.bar.securitycontrol.SecurityControlStaticTestSuite:sanitizeInt' + private static final String STATIC_SANITIZE_LONG = 'SANITIZER:XSS:foo.bar.securitycontrol.SecurityControlStaticTestSuite:sanitizeLong' + private static final String STATIC_INPUT_VALIDATOR_VALIDATE_ALL = 'INPUT_VALIDATOR:XSS:foo.bar.securitycontrol.SecurityControlStaticTestSuite:validateAll' + private static final String STATIC_INPUT_VALIDATOR_VALIDATE_OVERLOADED = 'INPUT_VALIDATOR:XSS:foo.bar.securitycontrol.SecurityControlStaticTestSuite:validate:java.lang.Object,java.lang.String,java.lang.String:1,2' + private static final String STATIC_INPUT_VALIDATOR_VALIDATE_RETURNING_INT = 'INPUT_VALIDATOR:XSS:foo.bar.securitycontrol.SecurityControlStaticTestSuite:validateReturningInt' + private static final String STATIC_INPUT_VALIDATOR_VALIDATE_OBJECT = 'INPUT_VALIDATOR:XSS:foo.bar.securitycontrol.SecurityControlStaticTestSuite:validateObject' + private static final String STATIC_INPUT_VALIDATOR_VALIDATE_LONG = 'INPUT_VALIDATOR:XSS:foo.bar.securitycontrol.SecurityControlStaticTestSuite:validateLong' + private static final String STATIC_INPUT_VALIDATOR_VALIDATE_SELECTED_LONG = 'INPUT_VALIDATOR:XSS:foo.bar.securitycontrol.SecurityControlStaticTestSuite:validateSelectedLong:0' + + //not static methods + private static final String SANITIZER = 'SANITIZER:XSS:foo.bar.securitycontrol.SecurityControlTestSuite:sanitize' + private static final String SANITIZE_VALIDATE_OBJECT = 'SANITIZER:XSS:foo.bar.securitycontrol.SecurityControlTestSuite:sanitizeObject' + private static final String SANITIZE_INPUTS= 'SANITIZER:XSS:foo.bar.securitycontrol.SecurityControlTestSuite:sanitizeInputs' + private static final String SANITIZE_MANY_INPUTS= 'SANITIZER:XSS:foo.bar.securitycontrol.SecurityControlTestSuite:sanitizeManyInputs' + private static final String SANITIZE_INT = 'SANITIZER:XSS:foo.bar.securitycontrol.SecurityControlTestSuite:sanitizeInt' + private static final String SANITIZE_LONG = 'SANITIZER:XSS:foo.bar.securitycontrol.SecurityControlTestSuite:sanitizeLong' + private static final String INPUT_VALIDATOR_VALIDATE_ALL = 'INPUT_VALIDATOR:XSS:foo.bar.securitycontrol.SecurityControlTestSuite:validateAll' + private static final String INPUT_VALIDATOR_VALIDATE_OVERLOADED = 'INPUT_VALIDATOR:XSS:foo.bar.securitycontrol.SecurityControlTestSuite:validate:java.lang.Object,java.lang.String,java.lang.String:1,2' + private static final String INPUT_VALIDATOR_VALIDATE_RETURNING_INT = 'INPUT_VALIDATOR:XSS:foo.bar.securitycontrol.SecurityControlTestSuite:validateReturningInt' + private static final String INPUT_VALIDATOR_VALIDATE_OBJECT = 'INPUT_VALIDATOR:XSS:foo.bar.securitycontrol.SecurityControlTestSuite:validateObject' + private static final String INPUT_VALIDATOR_VALIDATE_LONG = 'INPUT_VALIDATOR:XSS:foo.bar.securitycontrol.SecurityControlTestSuite:validateLong' + private static final String INPUT_VALIDATOR_VALIDATE_SELECTED_LONG = 'INPUT_VALIDATOR:XSS:foo.bar.securitycontrol.SecurityControlTestSuite:validateSelectedLong:0' + + + + def setupSpec() { + final staticConfig = "${STATIC_SANITIZER};${STATIC_SANITIZE_VALIDATE_OBJECT};${STATIC_SANITIZE_INPUTS};${STATIC_SANITIZE_MANY_INPUTS};${STATIC_SANITIZE_INT};${STATIC_SANITIZE_LONG};${STATIC_INPUT_VALIDATOR_VALIDATE_ALL};${STATIC_INPUT_VALIDATOR_VALIDATE_OVERLOADED};${STATIC_INPUT_VALIDATOR_VALIDATE_RETURNING_INT};${STATIC_INPUT_VALIDATOR_VALIDATE_OBJECT};${STATIC_INPUT_VALIDATOR_VALIDATE_LONG};${STATIC_INPUT_VALIDATOR_VALIDATE_SELECTED_LONG}" + final config = "${SANITIZER};${SANITIZE_VALIDATE_OBJECT};${SANITIZE_INPUTS};${SANITIZE_MANY_INPUTS};${SANITIZE_INT};${SANITIZE_LONG};${INPUT_VALIDATOR_VALIDATE_ALL};${INPUT_VALIDATOR_VALIDATE_OVERLOADED};${INPUT_VALIDATOR_VALIDATE_RETURNING_INT};${INPUT_VALIDATOR_VALIDATE_OBJECT};${INPUT_VALIDATOR_VALIDATE_LONG};${INPUT_VALIDATOR_VALIDATE_SELECTED_LONG}" + final fullConfig = "${staticConfig};${config}" + Instrumentation instrumentation = ByteBuddyAgent.install() + Map> securityControls = + SecurityControlFormatter.format(fullConfig) + assert securityControls != null + instrumentation.addTransformer(new IastSecurityControlTransformer(securityControls), true) + } + + + void 'test sanitize'(){ + given: + final iastModule = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(iastModule) + final marks = (VulnerabilityMarks.XSS_MARK | VulnerabilityMarks.CUSTOM_SECURITY_CONTROL_MARK) + + when: + SecurityControlStaticTestSuite.&"$method".call(*args) + + then: + expected * iastModule.markIfTainted( toSanitize, marks) + 0 * _ + + when: + final suite = new SecurityControlTestSuite() + suite.&"$method".call(*args) + + then: + expected * iastModule.markIfTainted( toSanitize, marks) + 0 * _ + + where: + method | args | toSanitize | expected + 'sanitize' | ['test'] | 'Sanitized' | 1 + 'sanitizeObject' | ['test'] | 'Sanitized' | 1 + 'sanitizeInputs' | ['test1', new Object(), 27i] | 'Sanitized' | 1 + 'sanitizeManyInputs' | ['test', 'test2', 'test3', 'test4', 'test5', 'test6', 'test7', 'test8', 'test9', 'test10'] | 'Sanitized' | 1 + 'sanitizeInt' | [1i] | args | 0 + 'sanitizeLong' | [1L] | args | 0 + } + + void 'test validate'(){ + given: + final iastModule = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(iastModule) + final marks = (VulnerabilityMarks.XSS_MARK | VulnerabilityMarks.CUSTOM_SECURITY_CONTROL_MARK) + + when: + SecurityControlStaticTestSuite.&"$method".call(*args) + + then: + for (final validate : toValidate){ + expected * iastModule.markIfTainted(validate, marks) + } + 0 * _ + + when: + final suite = new SecurityControlTestSuite() + suite.&"$method".call(*args) + + then: + for (final validate : toValidate){ + expected * iastModule.markIfTainted(validate, marks) + } + 0 * _ + + where: + method | args | toValidate | expected + 'validateAll' | ['test'] | args | 1 + 'validateAll' | ['test1', 'test2'] | args | 1 + 'validateAll' | [1L, 'test2'] | [args[1]] | 1 + 'validateAll' | ['test', 'test2', 'test3', 'test4', 'test5', 'test6', 'test7', 'test8', 'test9', 'test10'] | args | 1 + 'validate' | ['test'] | args | 0 + 'validate' | [new Object(), 'test1', 'test2'] | [args[1], args[2]] | 1 + 'validateReturningInt' | ['test'] | args | 1 + 'validateObject' | [new Object()] | args | 1 + 'validateLong' | [1L, 'test2'] | [args[1]] | 1 + 'validateLong' | ['test2', 2L] | [args[0]] | 1 + 'validateLong' | [1L, 'test2', 2L] | [args[1]] | 1 + 'validateSelectedLong' | [1L] | args | 0 + 'validateSelectedLong' | [1L, 2L] | [args[0]] | 0 + } +} diff --git a/dd-java-agent/agent-iast/src/test/java/foo/bar/securitycontrol/SecurityControlStaticTestSuite.java b/dd-java-agent/agent-iast/src/test/java/foo/bar/securitycontrol/SecurityControlStaticTestSuite.java new file mode 100644 index 00000000000..d0169c06773 --- /dev/null +++ b/dd-java-agent/agent-iast/src/test/java/foo/bar/securitycontrol/SecurityControlStaticTestSuite.java @@ -0,0 +1,100 @@ +package foo.bar.securitycontrol; + +public class SecurityControlStaticTestSuite { + + public static boolean validateAll(long input, String input2) { + return true; // dummy implementation + } + + public static boolean validateAll(String input) { + return true; // dummy implementation + } + + public static boolean validateAll(String input, String input2) { + return true; // dummy implementation + } + + public static boolean validateAll( + String input, + String input2, + String input3, + String input4, + String input5, + String input6, + String input7, + String input8, + String input9, + String input10) { + return true; // dummy implementation + } + + public static boolean validateLong(long input, String input2) { + return true; // dummy implementation + } + + public static boolean validateLong(String input, long input2) { + return true; // dummy implementation + } + + public static boolean validateLong(long intput1, String input2, long input3) { + return true; // dummy implementation + } + + public static boolean validateSelectedLong(long intput1) { + return true; // dummy implementation + } + + public static boolean validateSelectedLong(long input1, long intput2) { + return true; // dummy implementation + } + + public static boolean validate(String input) { + return true; // dummy implementation + } + + public static boolean validate(Object o, String input, String input2) { + return true; // dummy implementation + } + + public static int validateReturningInt(String input) { + return 1; // dummy implementation + } + + public static int validateObject(Object input) { + return 1; // dummy implementation + } + + public static String sanitize(String input) { + return "Sanitized"; + } + + public static Object sanitizeObject(String input) { + return "Sanitized"; + } + + public static String sanitizeInputs(String input, Object input2, int input3) { + return "Sanitized"; + } + + public static String sanitizeManyInputs( + String input, + String input2, + String input3, + String input4, + String input5, + String input6, + String input7, + String input8, + String input9, + String input10) { + return "Sanitized"; + } + + public static int sanitizeInt(int input) { + return input; + } + + public static long sanitizeLong(long input) { + return input; + } +} diff --git a/dd-java-agent/agent-iast/src/test/java/foo/bar/securitycontrol/SecurityControlTestSuite.java b/dd-java-agent/agent-iast/src/test/java/foo/bar/securitycontrol/SecurityControlTestSuite.java new file mode 100644 index 00000000000..99fae518950 --- /dev/null +++ b/dd-java-agent/agent-iast/src/test/java/foo/bar/securitycontrol/SecurityControlTestSuite.java @@ -0,0 +1,100 @@ +package foo.bar.securitycontrol; + +public class SecurityControlTestSuite { + + public boolean validateAll(long input, String input2) { + return true; // dummy implementation + } + + public boolean validateAll(String input) { + return true; // dummy implementation + } + + public boolean validateAll(String input, String input2) { + return true; // dummy implementation + } + + public boolean validateAll( + String input, + String input2, + String input3, + String input4, + String input5, + String input6, + String input7, + String input8, + String input9, + String input10) { + return true; // dummy implementation + } + + public boolean validateLong(long input, String input2) { + return true; // dummy implementation + } + + public boolean validateLong(String input, long input2) { + return true; // dummy implementation + } + + public boolean validateLong(long intput1, String input2, long input3) { + return true; // dummy implementation + } + + public boolean validateSelectedLong(long intput1) { + return true; // dummy implementation + } + + public boolean validateSelectedLong(long input1, long intput2) { + return true; // dummy implementation + } + + public boolean validate(String input) { + return true; // dummy implementation + } + + public boolean validate(Object o, String input, String input2) { + return true; // dummy implementation + } + + public int validateReturningInt(String input) { + return 1; // dummy implementation + } + + public int validateObject(Object input) { + return 1; // dummy implementation + } + + public String sanitize(String input) { + return "Sanitized"; + } + + public Object sanitizeObject(String input) { + return "Sanitized"; + } + + public String sanitizeInputs(String input, Object input2, int input3) { + return "Sanitized"; + } + + public String sanitizeManyInputs( + String input, + String input2, + String input3, + String input4, + String input5, + String input6, + String input7, + String input8, + String input9, + String input10) { + return "Sanitized"; + } + + public int sanitizeInt(int input) { + return input; + } + + public long sanitizeLong(long input) { + return input; + } +} diff --git a/dd-smoke-tests/iast-util/src/main/java/datadog/smoketest/springboot/controller/XssController.java b/dd-smoke-tests/iast-util/src/main/java/datadog/smoketest/springboot/controller/XssController.java index 81f708acb55..32cad411098 100644 --- a/dd-smoke-tests/iast-util/src/main/java/datadog/smoketest/springboot/controller/XssController.java +++ b/dd-smoke-tests/iast-util/src/main/java/datadog/smoketest/springboot/controller/XssController.java @@ -1,5 +1,7 @@ package datadog.smoketest.springboot.controller; +import ddtest.securitycontrols.InputValidator; +import ddtest.securitycontrols.Sanitizer; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.io.IOException; import java.util.Locale; @@ -190,4 +192,52 @@ public void format4(final HttpServletRequest request, final HttpServletResponse public String responseBody(final HttpServletRequest request, final HttpServletResponse response) { return request.getParameter("string"); } + + @GetMapping("/sanitize") + @SuppressFBWarnings + public void sanitize(final HttpServletRequest request, final HttpServletResponse response) { + try { + response.getWriter().write(Sanitizer.sanitize(request.getParameter("string"))); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @GetMapping("/validateAll") + @SuppressFBWarnings + public void validateAll(final HttpServletRequest request, final HttpServletResponse response) { + try { + String s = request.getParameter("string"); + InputValidator.validateAll(s); + response.getWriter().write(s); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @GetMapping("/validateAll2") + @SuppressFBWarnings + public void validate2(final HttpServletRequest request, final HttpServletResponse response) { + try { + String string1 = request.getParameter("string"); + String string2 = request.getParameter("string2"); + InputValidator.validateAll(string1, string2); + response.getWriter().write(string1 + string2); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @GetMapping("/validate") + @SuppressFBWarnings + public void validate(final HttpServletRequest request, final HttpServletResponse response) { + try { + String string1 = request.getParameter("string"); + String string2 = request.getParameter("string2"); + InputValidator.validate(null, string1, string2); + response.getWriter().write(string1 + string2); + } catch (IOException e) { + throw new RuntimeException(e); + } + } } diff --git a/dd-smoke-tests/iast-util/src/main/java/ddtest/securitycontrols/InputValidator.java b/dd-smoke-tests/iast-util/src/main/java/ddtest/securitycontrols/InputValidator.java new file mode 100644 index 00000000000..2786b00be9c --- /dev/null +++ b/dd-smoke-tests/iast-util/src/main/java/ddtest/securitycontrols/InputValidator.java @@ -0,0 +1,20 @@ +package ddtest.securitycontrols; + +public class InputValidator { + + public static boolean validateAll(String input) { + return true; // dummy implementation + } + + public static boolean validateAll(String input, String input2) { + return true; // dummy implementation + } + + public static boolean validate(String input) { + return true; // dummy implementation + } + + public static boolean validate(Object o, String input, String input2) { + return true; // dummy implementation + } +} diff --git a/dd-smoke-tests/iast-util/src/main/java/ddtest/securitycontrols/Sanitizer.java b/dd-smoke-tests/iast-util/src/main/java/ddtest/securitycontrols/Sanitizer.java new file mode 100644 index 00000000000..3fc4dcf9937 --- /dev/null +++ b/dd-smoke-tests/iast-util/src/main/java/ddtest/securitycontrols/Sanitizer.java @@ -0,0 +1,8 @@ +package ddtest.securitycontrols; + +public class Sanitizer { + + public static String sanitize(String input) { + return "Sanitized: " + input; + } +} diff --git a/dd-smoke-tests/iast-util/src/testFixtures/groovy/datadog/smoketest/AbstractIastSpringBootTest.groovy b/dd-smoke-tests/iast-util/src/testFixtures/groovy/datadog/smoketest/AbstractIastSpringBootTest.groovy index 65e46c6c444..806d8e2bdf9 100644 --- a/dd-smoke-tests/iast-util/src/testFixtures/groovy/datadog/smoketest/AbstractIastSpringBootTest.groovy +++ b/dd-smoke-tests/iast-util/src/testFixtures/groovy/datadog/smoketest/AbstractIastSpringBootTest.groovy @@ -10,6 +10,7 @@ import okhttp3.Response import static datadog.trace.api.config.IastConfig.IAST_DEBUG_ENABLED import static datadog.trace.api.config.IastConfig.IAST_DETECTION_MODE import static datadog.trace.api.config.IastConfig.IAST_ENABLED +import static datadog.trace.api.config.IastConfig.IAST_SECURITY_CONTROLS_CONFIGURATION abstract class AbstractIastSpringBootTest extends AbstractIastServerSmokeTest { @@ -36,6 +37,7 @@ abstract class AbstractIastSpringBootTest extends AbstractIastServerSmokeTest { withSystemProperty(IAST_ENABLED, true), withSystemProperty(IAST_DETECTION_MODE, 'FULL'), withSystemProperty(IAST_DEBUG_ENABLED, true), + withSystemProperty(IAST_SECURITY_CONTROLS_CONFIGURATION, "SANITIZER:XSS:ddtest.securitycontrols.Sanitizer:sanitize;INPUT_VALIDATOR:XSS:ddtest.securitycontrols.InputValidator:validateAll;INPUT_VALIDATOR:XSS:ddtest.securitycontrols.InputValidator:validate:java.lang.Object,java.lang.String,java.lang.String:1,2"), ] } @@ -1199,5 +1201,24 @@ abstract class AbstractIastSpringBootTest extends AbstractIastServerSmokeTest { then: response.body().string().contains("Test") } + void 'security controls avoid vulnerabilities'() { + setup: + final url = "http://localhost:${httpPort}/xss/${method}?string=test&string2=test2" + final request = new Request.Builder().url(url).get().build() + + when: + client.newCall(request).execute() + + then: + noVulnerability { vul -> vul.type == 'XSS' && vul.location.method == method } + + where: + method | _ + 'sanitize' | _ + 'validateAll' | _ + 'validateAll2' | _ + 'validate' | _ + } + } diff --git a/dd-trace-api/src/main/java/datadog/trace/api/config/IastConfig.java b/dd-trace-api/src/main/java/datadog/trace/api/config/IastConfig.java index 73f0b93a4c9..42dd7e91c4d 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/config/IastConfig.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/config/IastConfig.java @@ -17,7 +17,6 @@ public final class IastConfig { public static final String IAST_REDACTION_NAME_PATTERN = "iast.redaction.name.pattern"; public static final String IAST_REDACTION_VALUE_PATTERN = "iast.redaction.value.pattern"; public static final String IAST_STACKTRACE_LEAK_SUPPRESS = "iast.stacktrace-leak.suppress"; - public static final String IAST_HARDCODED_SECRET_ENABLED = "iast.hardcoded-secret.enabled"; public static final String IAST_MAX_RANGE_COUNT = "iast.max-range-count"; public static final String IAST_TRUNCATION_MAX_VALUE_LENGTH = "iast.truncation.max.value.length"; @@ -27,8 +26,10 @@ public final class IastConfig { public static final String IAST_SOURCE_MAPPING_MAX_SIZE = "iast.source-mapping.max-size"; public static final String IAST_EXPERIMENTAL_PROPAGATION_ENABLED = "iast.experimental.propagation.enabled"; - public static final String IAST_STACK_TRACE_ENABLED = "iast.stacktrace.enabled"; + public static final String IAST_SECURITY_CONTROLS_ENABLED = "iast.security-controls.enabled"; + public static final String IAST_SECURITY_CONTROLS_CONFIGURATION = + "iast.security-controls.configuration"; private IastConfig() {} } diff --git a/internal-api/build.gradle b/internal-api/build.gradle index f62b6ec81a3..fb4d388fc97 100644 --- a/internal-api/build.gradle +++ b/internal-api/build.gradle @@ -189,6 +189,9 @@ excludedClassesCoverage += [ 'datadog.trace.util.stacktrace.StackTraceBatch', 'datadog.trace.util.stacktrace.StackTraceEvent', 'datadog.trace.util.stacktrace.StackTraceFrame', + 'datadog.trace.api.iast.VulnerabilityMarks', + 'datadog.trace.api.iast.securitycontrol.SecurityControlHelper', + 'datadog.trace.api.iast.securitycontrol.SecurityControl' ] excludedClassesBranchCoverage = [ 'datadog.trace.api.ProductActivationConfig', diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java index f27c6cf0272..60a64f9e3d9 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -307,6 +307,7 @@ public static String getHostName() { private final int iastSourceMappingMaxSize; private final boolean iastStackTraceEnabled; private final boolean iastExperimentalPropagationEnabled; + private final String iastSecurityControlsConfiguration; private final boolean ciVisibilityTraceSanitationEnabled; private final boolean ciVisibilityAgentlessEnabled; @@ -1332,6 +1333,8 @@ PROFILING_DATADOG_PROFILER_ENABLED, isDatadogProfilerSafeInCurrentEnvironment()) configProvider.getBoolean(IAST_STACK_TRACE_ENABLED, DEFAULT_IAST_STACK_TRACE_ENABLED); iastExperimentalPropagationEnabled = configProvider.getBoolean(IAST_EXPERIMENTAL_PROPAGATION_ENABLED, false); + iastSecurityControlsConfiguration = + configProvider.getString(IAST_SECURITY_CONTROLS_CONFIGURATION, null); ciVisibilityTraceSanitationEnabled = configProvider.getBoolean(CIVISIBILITY_TRACE_SANITATION_ENABLED, true); @@ -2633,6 +2636,10 @@ public boolean isIastExperimentalPropagationEnabled() { return iastExperimentalPropagationEnabled; } + public String getIastSecurityControlsConfiguration() { + return iastSecurityControlsConfiguration; + } + public boolean isCiVisibilityEnabled() { return instrumenterConfig.isCiVisibilityEnabled(); } diff --git a/internal-api/src/main/java/datadog/trace/api/iast/VulnerabilityMarks.java b/internal-api/src/main/java/datadog/trace/api/iast/VulnerabilityMarks.java index 59b493e3f59..adcc65222ed 100644 --- a/internal-api/src/main/java/datadog/trace/api/iast/VulnerabilityMarks.java +++ b/internal-api/src/main/java/datadog/trace/api/iast/VulnerabilityMarks.java @@ -20,4 +20,55 @@ private VulnerabilityMarks() {} public static final int HEADER_INJECTION_MARK = 1 << 9; public static final int REFLECTION_INJECTION_MARK = 1 << 10; public static final int UNTRUSTED_DESERIALIZATION_MARK = 1 << 11; + + public static final int CUSTOM_SECURITY_CONTROL_MARK = 1 << 13; + + public static int markForAll() { + return XSS_MARK + | XPATH_INJECTION_MARK + | SQL_INJECTION_MARK + | COMMAND_INJECTION_MARK + | PATH_TRAVERSAL_MARK + | LDAP_INJECTION_MARK + | SSRF_MARK + | UNVALIDATED_REDIRECT_MARK + | TRUST_BOUNDARY_VIOLATION_MARK + | HEADER_INJECTION_MARK + | REFLECTION_INJECTION_MARK + | UNTRUSTED_DESERIALIZATION_MARK + | CUSTOM_SECURITY_CONTROL_MARK; + } + + public static int getMarkFromVulnerabitityType(final String vulnerabilityTypeString) { + switch (vulnerabilityTypeString) { + case "XSS": + return XSS_MARK; + case "XPATH_INJECTION": + return XPATH_INJECTION_MARK; + case "SQL_INJECTION": + return SQL_INJECTION_MARK; + case "COMMAND_INJECTION": + return COMMAND_INJECTION_MARK; + case "PATH_TRAVERSAL": + return PATH_TRAVERSAL_MARK; + case "LDAP_INJECTION": + return LDAP_INJECTION_MARK; + case "SSRF": + return SSRF_MARK; + case "UNVALIDATED_REDIRECT": + return UNVALIDATED_REDIRECT_MARK; + case "TRUST_BOUNDARY_VIOLATION": + return TRUST_BOUNDARY_VIOLATION_MARK; + case "HEADER_INJECTION": + return HEADER_INJECTION_MARK; + case "REFLECTION_INJECTION": + return REFLECTION_INJECTION_MARK; + case "UNTRUSTED_DESERIALIZATION": + return UNTRUSTED_DESERIALIZATION_MARK; + case "CUSTOM_SECURITY_CONTROL": + return CUSTOM_SECURITY_CONTROL_MARK; + default: + return NOT_MARKED; + } + } } diff --git a/internal-api/src/main/java/datadog/trace/api/iast/propagation/PropagationModule.java b/internal-api/src/main/java/datadog/trace/api/iast/propagation/PropagationModule.java index ca54e68ff1b..371c3618835 100644 --- a/internal-api/src/main/java/datadog/trace/api/iast/propagation/PropagationModule.java +++ b/internal-api/src/main/java/datadog/trace/api/iast/propagation/PropagationModule.java @@ -345,4 +345,6 @@ int taintObjectDeeply( */ @Nullable Source findSource(@Nullable IastContext ctx, @Nullable Object target); + + void markIfTainted(@Nullable Object target, int mark); } diff --git a/internal-api/src/main/java/datadog/trace/api/iast/securitycontrol/SecurityControl.java b/internal-api/src/main/java/datadog/trace/api/iast/securitycontrol/SecurityControl.java new file mode 100644 index 00000000000..5126d323811 --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/api/iast/securitycontrol/SecurityControl.java @@ -0,0 +1,86 @@ +package datadog.trace.api.iast.securitycontrol; + +import java.util.BitSet; +import java.util.List; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + +public class SecurityControl { + + @Nonnull private SecurityControlType type; + + private int marks; + + @Nonnull private String className; + + @Nonnull private String method; + + @Nullable private List parameterTypes; + + @Nullable private BitSet parametersToMark; + + public SecurityControl( + @Nonnull SecurityControlType type, + int marks, + @Nonnull String className, + @Nonnull String method, + @Nullable List parameterTypes, + @Nullable BitSet parametersToMark) { + this.type = type; + this.marks = marks; + this.className = className; + this.method = method; + this.parameterTypes = parameterTypes; + this.parametersToMark = parametersToMark; + } + + @Nonnull + public SecurityControlType getType() { + return type; + } + + @Nonnull + public int getMarks() { + return marks; + } + + @Nonnull + public String getClassName() { + return className; + } + + @Nonnull + public String getMethod() { + return method; + } + + @Nullable + public List getParameterTypes() { + return parameterTypes; + } + + @Nullable + public BitSet getParametersToMark() { + return parametersToMark; + } + + @Override + public String toString() { + return "SecurityControl{" + + "type=" + + type + + ", marks=" + + marks + + ", className='" + + className + + '\'' + + ", method='" + + method + + '\'' + + ", parameterTypes=" + + parameterTypes + + ", parametersToMark=" + + parametersToMark + + '}'; + } +} diff --git a/internal-api/src/main/java/datadog/trace/api/iast/securitycontrol/SecurityControlFormatter.java b/internal-api/src/main/java/datadog/trace/api/iast/securitycontrol/SecurityControlFormatter.java new file mode 100644 index 00000000000..2d6894c5c8b --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/api/iast/securitycontrol/SecurityControlFormatter.java @@ -0,0 +1,125 @@ +package datadog.trace.api.iast.securitycontrol; + +import static datadog.trace.api.iast.VulnerabilityMarks.CUSTOM_SECURITY_CONTROL_MARK; + +import datadog.trace.api.iast.VulnerabilityMarks; +import de.thetaphi.forbiddenapis.SuppressForbidden; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.BitSet; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; +import org.slf4j.Logger; + +@SuppressForbidden // Suppresses the warning for using split method +public class SecurityControlFormatter { + + private static final Logger log = + org.slf4j.LoggerFactory.getLogger(SecurityControlFormatter.class); + + private static final String SECURITY_CONTROL_DELIMITER = ";"; + private static final String SECURITY_CONTROL_FIELD_DELIMITER = ":"; + private static final String SECURITY_CONTROL_ELEMENT_DELIMITER = ","; + + private static final String ALL = "*"; + + @Nullable + public static Map> format( + final @Nonnull String securityControlString) { + + if (securityControlString.isEmpty()) { + log.warn("Security control configuration is empty"); + return null; + } + + String config = securityControlString.replaceAll("\\s+", ""); + + String[] list = config.split(SECURITY_CONTROL_DELIMITER); + + Map> securityControls = new HashMap<>(); + + for (String s : list) { + try { + SecurityControl securityControl = getSecurityControl(s); + if (securityControl != null) { + securityControls.putIfAbsent(securityControl.getClassName(), new ArrayList<>()); + securityControls.get(securityControl.getClassName()).add(securityControl); + } + } catch (Exception e) { + log.warn("Security control configuration is invalid: {}", s); + } + } + return securityControls.isEmpty() ? null : securityControls; + } + + private static SecurityControl getSecurityControl(@Nonnull final String config) { + if (config.isEmpty()) { + log.warn("Security control configuration is empty"); + return null; + } + String[] split = config.split(SECURITY_CONTROL_FIELD_DELIMITER); + if (split.length < 4) { + log.warn("Security control configuration is invalid: {}", config); + return null; + } + SecurityControlType type = SecurityControlType.valueOf(split[0]); + if (type == null) { + log.warn("Security control type is invalid: {}", split[0]); + return null; + } + int marks = getMarks(split[1]); + String className = split[2].replaceAll("\\.", "/"); + String method = split[3]; + + List parameterTypes = null; + BitSet parametersToMark = null; + + if (split.length > 4) { + String[] elements = split[4].split(SECURITY_CONTROL_ELEMENT_DELIMITER); + if (elements.length > 0) { + if (isNumeric(elements[0])) { + if (split.length != 5) { + log.warn("Security control configuration is invalid: {}", config); + return null; + } + parametersToMark = getParametersToMark(elements); + } else { + parameterTypes = Arrays.asList(elements); + } + } + } + if (split.length > 5) { + String[] elements = split[5].split(SECURITY_CONTROL_ELEMENT_DELIMITER); + parametersToMark = getParametersToMark(elements); + } + + return new SecurityControl(type, marks, className, method, parameterTypes, parametersToMark); + } + + private static int getMarks(String s) { + if (s.equals(ALL)) { + return VulnerabilityMarks.markForAll(); + } + String[] elements = s.split(SECURITY_CONTROL_ELEMENT_DELIMITER); + int marks = CUSTOM_SECURITY_CONTROL_MARK; + for (String element : elements) { + marks |= VulnerabilityMarks.getMarkFromVulnerabitityType(element); + } + return marks; + } + + private static BitSet getParametersToMark(String[] elements) { + BitSet bitSet = new BitSet(); + Arrays.stream(elements) + .map(Integer::parseInt) // Convert each element to an Integer + .forEach(bitSet::set); + return bitSet; + } + + private static boolean isNumeric(String str) { + return str.matches("[0-9]+"); + } +} diff --git a/internal-api/src/main/java/datadog/trace/api/iast/securitycontrol/SecurityControlHelper.java b/internal-api/src/main/java/datadog/trace/api/iast/securitycontrol/SecurityControlHelper.java new file mode 100644 index 00000000000..babf9b20edc --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/api/iast/securitycontrol/SecurityControlHelper.java @@ -0,0 +1,19 @@ +package datadog.trace.api.iast.securitycontrol; + +import datadog.trace.api.iast.InstrumentationBridge; +import datadog.trace.api.iast.propagation.PropagationModule; + +public class SecurityControlHelper { + + public static void setSecureMarks(final Object target, int marks) { + + final PropagationModule module = InstrumentationBridge.PROPAGATION; + try { + if (module != null) { + module.markIfTainted(target, marks); + } + } catch (final Throwable e) { + module.onUnexpectedException("setSecureMarks threw", e); + } + } +} diff --git a/internal-api/src/main/java/datadog/trace/api/iast/securitycontrol/SecurityControlType.java b/internal-api/src/main/java/datadog/trace/api/iast/securitycontrol/SecurityControlType.java new file mode 100644 index 00000000000..f1b5b6a53da --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/api/iast/securitycontrol/SecurityControlType.java @@ -0,0 +1,6 @@ +package datadog.trace.api.iast.securitycontrol; + +public enum SecurityControlType { + INPUT_VALIDATOR, + SANITIZER +} diff --git a/internal-api/src/test/groovy/datadog/trace/api/iast/securitycontrol/SecurityControlFormatterTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/iast/securitycontrol/SecurityControlFormatterTest.groovy new file mode 100644 index 00000000000..d094eefdd9b --- /dev/null +++ b/internal-api/src/test/groovy/datadog/trace/api/iast/securitycontrol/SecurityControlFormatterTest.groovy @@ -0,0 +1,170 @@ +package datadog.trace.api.iast.securitycontrol + +import datadog.trace.api.iast.VulnerabilityMarks +import datadog.trace.test.util.DDSpecification + +class SecurityControlFormatterTest extends DDSpecification{ + + void 'test simple Input validator'() { + setup: + final formatter = new SecurityControlFormatter() + final config = 'INPUT_VALIDATOR:COMMAND_INJECTION:bar.foo.CustomInputValidator:validate' + final result = formatter.format(config) + + expect: + result.size() == 1 + def securityControls = result.get('bar/foo/CustomInputValidator') + securityControls.size() == 1 + def securityControl = securityControls.get(0) + securityControl.getType() == SecurityControlType.INPUT_VALIDATOR + securityControl.getMarks() == (VulnerabilityMarks.COMMAND_INJECTION_MARK | VulnerabilityMarks.CUSTOM_SECURITY_CONTROL_MARK) + securityControl.getClassName() == "bar/foo/CustomInputValidator" + securityControl.getMethod() == "validate" + securityControl.getParameterTypes() == null + securityControl.getParametersToMark() == null + } + + void 'test simple sanitizer'() { + setup: + final formatter = new SecurityControlFormatter() + final config = 'SANITIZER:COMMAND_INJECTION:bar.foo.CustomSanitizer:sanitize' + final result = formatter.format(config) + + expect: + result.size() == 1 + def securityControls = result.get('bar/foo/CustomSanitizer') + securityControls.size() == 1 + def securityControl = securityControls.get(0) + securityControl.getType() == SecurityControlType.SANITIZER + securityControl.getMarks() == (VulnerabilityMarks.COMMAND_INJECTION_MARK | VulnerabilityMarks.CUSTOM_SECURITY_CONTROL_MARK) + securityControl.getClassName() == "bar/foo/CustomSanitizer" + securityControl.getMethod() == "sanitize" + securityControl.getParameterTypes() == null + securityControl.getParametersToMark() == null + } + + void 'test multiple security controls'(){ + setup: + final formatter = new SecurityControlFormatter() + final config = 'INPUT_VALIDATOR:COMMAND_INJECTION:bar.foo.CustomInputValidator:validate;SANITIZER:COMMAND_INJECTION:bar.foo.CustomSanitizer:sanitize' + final result = formatter.format(config) + + expect: + result.size() == 2 + def inputValidator = result.get('bar/foo/CustomInputValidator').get(0) + inputValidator.getType() == SecurityControlType.INPUT_VALIDATOR + inputValidator.getMarks() == (VulnerabilityMarks.COMMAND_INJECTION_MARK | VulnerabilityMarks.CUSTOM_SECURITY_CONTROL_MARK) + inputValidator.getClassName() == "bar/foo/CustomInputValidator" + inputValidator.getMethod() == "validate" + inputValidator.getParameterTypes() == null + inputValidator.getParametersToMark() == null + + def sanitizer = result.get('bar/foo/CustomSanitizer').get(0) + sanitizer.getType() == SecurityControlType.SANITIZER + sanitizer.getMarks() == (VulnerabilityMarks.COMMAND_INJECTION_MARK | VulnerabilityMarks.CUSTOM_SECURITY_CONTROL_MARK) + sanitizer.getClassName() == "bar/foo/CustomSanitizer" + sanitizer.getMethod() == "sanitize" + sanitizer.getParameterTypes() == null + sanitizer.getParametersToMark() == null + } + + void 'test multiple secure marks'() { + setup: + final formatter = new SecurityControlFormatter() + final config = 'INPUT_VALIDATOR:COMMAND_INJECTION,SQL_INJECTION:bar.foo.CustomInputValidator:validate' + final result = formatter.format(config) + + expect: + result.size() == 1 + def securityControl = result.get('bar/foo/CustomInputValidator').get(0) + securityControl.getType() == SecurityControlType.INPUT_VALIDATOR + securityControl.getMarks() == (VulnerabilityMarks.COMMAND_INJECTION_MARK | VulnerabilityMarks.SQL_INJECTION_MARK | VulnerabilityMarks.CUSTOM_SECURITY_CONTROL_MARK) + securityControl.getClassName() == "bar/foo/CustomInputValidator" + securityControl.getMethod() == "validate" + securityControl.getParameterTypes() == null + securityControl.getParametersToMark() == null + } + + void 'test overcharged methods'() { + setup: + final formatter = new SecurityControlFormatter() + final config = 'INPUT_VALIDATOR:COMMAND_INJECTION:bar.foo.CustomInputValidator:validate:java.lang.Object,java.lang.String,java.lang.String' + final result = formatter.format(config) + + expect: + result.size() == 1 + def securityControl = result.get('bar/foo/CustomInputValidator').get(0) + securityControl.getType() == SecurityControlType.INPUT_VALIDATOR + securityControl.getMarks() == (VulnerabilityMarks.COMMAND_INJECTION_MARK | VulnerabilityMarks.CUSTOM_SECURITY_CONTROL_MARK) + securityControl.getClassName() == "bar/foo/CustomInputValidator" + securityControl.getMethod() == "validate" + securityControl.getParameterTypes() == ["java.lang.Object", "java.lang.String", "java.lang.String"] + securityControl.getParametersToMark() == null + } + + void 'test parameters to mark'() { + setup: + final formatter = new SecurityControlFormatter() + final config = 'INPUT_VALIDATOR:COMMAND_INJECTION:bar.foo.CustomInputValidator:validate:1,2' + final result = formatter.format(config) + + expect: + result.size() == 1 + def securityControl = result.get('bar/foo/CustomInputValidator').get(0) + securityControl.getType() == SecurityControlType.INPUT_VALIDATOR + securityControl.getMarks() == (VulnerabilityMarks.COMMAND_INJECTION_MARK | VulnerabilityMarks.CUSTOM_SECURITY_CONTROL_MARK) + securityControl.getClassName() == "bar/foo/CustomInputValidator" + securityControl.getMethod() == "validate" + securityControl.getParameterTypes() == null + securityControl.getParametersToMark().cardinality() == 2 + securityControl.getParametersToMark().get(1) + securityControl.getParametersToMark().get(2) + } + + void 'test overcharged methods with parameters to mark'() { + setup: + final formatter = new SecurityControlFormatter() + final config = 'INPUT_VALIDATOR:COMMAND_INJECTION:bar.foo.CustomInputValidator:validate:java.lang.Object,java.lang.String,java.lang.String:1,2' + final result = formatter.format(config) + + expect: + result.size() == 1 + def securityControl = result.get('bar/foo/CustomInputValidator').get(0) + securityControl.getType() == SecurityControlType.INPUT_VALIDATOR + securityControl.getMarks() == (VulnerabilityMarks.COMMAND_INJECTION_MARK | VulnerabilityMarks.CUSTOM_SECURITY_CONTROL_MARK) + securityControl.getClassName() == "bar/foo/CustomInputValidator" + securityControl.getMethod() == "validate" + securityControl.getParameterTypes() == ["java.lang.Object", "java.lang.String", "java.lang.String"] + securityControl.getParametersToMark().cardinality() == 2 + securityControl.getParametersToMark().get(1) + securityControl.getParametersToMark().get(2) + } + + void 'test error control'() { + setup: + final formatter = new SecurityControlFormatter() + Throwable thrown = null + def result = null + + when: + try { + result = formatter.format(config) + } catch (Throwable t) { + thrown = t + } + + then: + thrown == null + result == null + + where: + config << [ + '', + 'This is not a valid configuration', + 'INPUT_VALIDATOR', + 'INPUT_VALIDATOR:COMMAND_INJECTION', + 'INPUT_VALIDATOR:COMMAND_INJECTION:bar.foo.CustomInputValidator', + 'INPUT_VALIDATOR:COMMAND_INJECTION:bar.foo.CustomInputValidator:validate:1,2:java.lang.Object,java.lang.String,java.lang.String' + ] + } +} diff --git a/internal-api/src/test/groovy/datadog/trace/api/iast/securitycontrol/SecurityControlHelperTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/iast/securitycontrol/SecurityControlHelperTest.groovy new file mode 100644 index 00000000000..135d535d391 --- /dev/null +++ b/internal-api/src/test/groovy/datadog/trace/api/iast/securitycontrol/SecurityControlHelperTest.groovy @@ -0,0 +1,37 @@ +package datadog.trace.api.iast.securitycontrol + +import datadog.trace.api.iast.InstrumentationBridge +import datadog.trace.api.iast.VulnerabilityMarks +import datadog.trace.api.iast.propagation.PropagationModule +import datadog.trace.test.util.DDSpecification + +class SecurityControlHelperTest extends DDSpecification { + + + void 'test no module'(){ + setup: + final toValidate = 'test' + final marks = VulnerabilityMarks.XSS_MARK + + when: + SecurityControlHelper.setSecureMarks(toValidate, marks) + + then: + 0 * _ + } + + void 'test'(){ + setup: + final iastModule = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(iastModule) + final toValidate = 'test' + final marks = VulnerabilityMarks.XSS_MARK + + when: + SecurityControlHelper.setSecureMarks(toValidate, marks) + + then: + 1 * iastModule.markIfTainted(toValidate, marks) + 0 * _ + } +}