Skip to content

Commit

Permalink
Add NullAway to IAST module and fix errors
Browse files Browse the repository at this point in the history
  • Loading branch information
manuel-alvarez-alvarez committed Oct 30, 2023
1 parent cbff199 commit 47bbfd7
Show file tree
Hide file tree
Showing 80 changed files with 410 additions and 257 deletions.
34 changes: 33 additions & 1 deletion dd-java-agent/agent-iast/build.gradle
Original file line number Diff line number Diff line change
@@ -1,13 +1,29 @@
import net.ltgt.gradle.errorprone.CheckSeverity

plugins {
id 'com.github.johnrengelman.shadow'
id 'me.champeau.jmh'
id 'java-test-fixtures'
id 'com.google.protobuf' version '0.8.18'
id 'net.ltgt.errorprone' version '3.1.0'
}

apply from: "$rootDir/gradle/java.gradle"
apply from: "$rootDir/gradle/version.gradle"

java {
toolchain {
languageVersion.set(JavaLanguageVersion.of(11))
}
sourceCompatibility = 1.8
targetCompatibility = 1.8
}

tasks.withType(JavaCompile).configureEach {
// ensure no APIs beyond JDK8 are used
options.compilerArgs.addAll(['--release', '8'])
}

// First version with Mac M1 support
def grpcVersion = '1.42.2'
protobuf {
Expand Down Expand Up @@ -49,6 +65,10 @@ dependencies {
jmh project(':dd-java-agent:agent-builder')
jmh project(':dd-java-agent:instrumentation:iast-instrumenter')
jmh project(':dd-java-agent:instrumentation:java-lang')

compileOnly('org.jetbrains:annotations:24.0.0')
errorprone('com.uber.nullaway:nullaway:0.10.15')
errorprone('com.google.errorprone:error_prone_core:2.23.0')
}

shadowJar {
Expand Down Expand Up @@ -80,7 +100,6 @@ ext {
tasks.withType(Test).configureEach {
jvmArgs += ['-Ddd.iast.enabled=true']
}
def rootDir = project.rootDir
spotless {
java {
target 'src/**/*.java'
Expand All @@ -104,3 +123,16 @@ sourceSets {
}
}
}

tasks.withType(JavaCompile).configureEach {
if (name == 'compileJava') {
options.errorprone {
check("NullAway", CheckSeverity.ERROR)
option("NullAway:AnnotatedPackages", "com.datadog.iast")
disableAllWarnings = true // only errors for now
}
} else {
// disable null away for test and jmh
options.errorprone.enabled = false
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package com.datadog.iast;

import com.datadog.iast.overhead.OverheadController;
import datadog.trace.api.Config;
import datadog.trace.util.stacktrace.StackWalker;
import javax.annotation.Nonnull;

public class Dependencies {

private final Config config;
private final Reporter reporter;
private final OverheadController overheadController;
private final StackWalker stackWalker;

public Dependencies(
@Nonnull final Config config,
@Nonnull final Reporter reporter,
@Nonnull final OverheadController overheadController,
@Nonnull final StackWalker stackWalker) {
this.config = config;
this.reporter = reporter;
this.overheadController = overheadController;
this.stackWalker = stackWalker;
}

public Config getConfig() {
return config;
}

public Reporter getReporter() {
return reporter;
}

public OverheadController getOverheadController() {
return overheadController;
}

public StackWalker getStackWalker() {
return stackWalker;
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ public class IastRequestContext implements IastContext, HasMetricCollector {
private final AtomicBoolean spanDataIsSet;
private final TaintedObjects taintedObjects;
private final OverheadContext overheadContext;
private final IastMetricCollector collector;
private volatile String strictTransportSecurity;
private volatile String xContentTypeOptions;
private volatile String xForwardedProto;
private volatile String contentType;
@Nullable private final IastMetricCollector collector;
@Nullable private volatile String strictTransportSecurity;
@Nullable private volatile String xContentTypeOptions;
@Nullable private volatile String xForwardedProto;
@Nullable private volatile String contentType;

public IastRequestContext() {
this(TaintedObjects.acquire(), null);
Expand All @@ -33,7 +33,7 @@ public IastRequestContext(final TaintedObjects taintedObjects) {
}

public IastRequestContext(
final TaintedObjects taintedObjects, final IastMetricCollector collector) {
final TaintedObjects taintedObjects, @Nullable final IastMetricCollector collector) {
this.vulnerabilityBatch = new VulnerabilityBatch();
this.spanDataIsSet = new AtomicBoolean(false);
this.overheadContext = new OverheadContext();
Expand All @@ -45,6 +45,7 @@ public VulnerabilityBatch getVulnerabilityBatch() {
return vulnerabilityBatch;
}

@Nullable
public String getStrictTransportSecurity() {
return strictTransportSecurity;
}
Expand All @@ -53,6 +54,7 @@ public void setStrictTransportSecurity(final String strictTransportSecurity) {
this.strictTransportSecurity = strictTransportSecurity;
}

@Nullable
public String getxContentTypeOptions() {
return xContentTypeOptions;
}
Expand All @@ -61,6 +63,7 @@ public void setxContentTypeOptions(final String xContentTypeOptions) {
this.xContentTypeOptions = xContentTypeOptions;
}

@Nullable
public String getxForwardedProto() {
return xForwardedProto;
}
Expand All @@ -69,6 +72,7 @@ public void setxForwardedProto(final String xForwardedProto) {
this.xForwardedProto = xForwardedProto;
}

@Nullable
public String getContentType() {
return contentType;
}
Expand Down Expand Up @@ -111,6 +115,7 @@ public static IastRequestContext get(final RequestContext reqCtx) {
return asRequestContext(IastContext.Provider.get(reqCtx));
}

@Nullable
private static IastRequestContext asRequestContext(final IastContext ctx) {
return ctx instanceof IastRequestContext ? (IastRequestContext) ctx : null;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package com.datadog.iast;

import com.datadog.iast.HasDependencies.Dependencies;
import com.datadog.iast.overhead.OverheadController;
import com.datadog.iast.propagation.FastCodecModule;
import com.datadog.iast.propagation.PropagationModuleImpl;
Expand Down Expand Up @@ -40,9 +39,9 @@
import datadog.trace.util.AgentTaskScheduler;
import datadog.trace.util.stacktrace.StackWalkerFactory;
import java.util.function.BiFunction;
import java.util.function.Consumer;
import java.util.function.Supplier;
import java.util.stream.Stream;
import javax.annotation.Nullable;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -55,7 +54,8 @@ public static void start(final SubscriptionService ss) {
start(ss, null);
}

public static void start(final SubscriptionService ss, OverheadController overheadController) {
public static void start(
final SubscriptionService ss, @Nullable OverheadController overheadController) {
final Config config = Config.get();
if (config.getIastActivation() != ProductActivation.FULLY_ENABLED) {
LOGGER.debug("IAST is disabled");
Expand All @@ -70,46 +70,37 @@ public static void start(final SubscriptionService ss, OverheadController overhe
final Dependencies dependencies =
new Dependencies(config, reporter, overheadController, StackWalkerFactory.INSTANCE);
final boolean addTelemetry = config.getIastTelemetryVerbosity() != Verbosity.OFF;
iastModules().forEach(registerModule(dependencies));
iastModules(dependencies).forEach(InstrumentationBridge::registerIastModule);
registerRequestStartedCallback(ss, addTelemetry, dependencies);
registerRequestEndedCallback(ss, addTelemetry, dependencies);
registerHeadersCallback(ss);
registerGrpcServerRequestMessageCallback(ss);
LOGGER.debug("IAST started");
}

private static Consumer<IastModule> registerModule(final Dependencies dependencies) {
return module -> {
if (module instanceof HasDependencies) {
((HasDependencies) module).registerDependencies(dependencies);
}
InstrumentationBridge.registerIastModule(module);
};
}

private static Stream<IastModule> iastModules() {
private static Stream<IastModule> iastModules(final Dependencies dependencies) {
return Stream.of(
new StringModuleImpl(),
new FastCodecModule(),
new SqlInjectionModuleImpl(),
new PathTraversalModuleImpl(),
new CommandInjectionModuleImpl(),
new WeakCipherModuleImpl(),
new WeakHashModuleImpl(),
new LdapInjectionModuleImpl(),
new SqlInjectionModuleImpl(dependencies),
new PathTraversalModuleImpl(dependencies),
new CommandInjectionModuleImpl(dependencies),
new WeakCipherModuleImpl(dependencies),
new WeakHashModuleImpl(dependencies),
new LdapInjectionModuleImpl(dependencies),
new PropagationModuleImpl(),
new HttpResponseHeaderModuleImpl(),
new HstsMissingHeaderModuleImpl(),
new HttpResponseHeaderModuleImpl(dependencies),
new HstsMissingHeaderModuleImpl(dependencies),
new InsecureCookieModuleImpl(),
new NoHttpOnlyCookieModuleImpl(),
new XContentTypeModuleImpl(),
new XContentTypeModuleImpl(dependencies),
new NoSameSiteCookieModuleImpl(),
new SsrfModuleImpl(),
new UnvalidatedRedirectModuleImpl(),
new WeakRandomnessModuleImpl(),
new XPathInjectionModuleImpl(),
new TrustBoundaryViolationModuleImpl(),
new XssModuleImpl());
new SsrfModuleImpl(dependencies),
new UnvalidatedRedirectModuleImpl(dependencies),
new WeakRandomnessModuleImpl(dependencies),
new XPathInjectionModuleImpl(dependencies),
new TrustBoundaryViolationModuleImpl(dependencies),
new XssModuleImpl(dependencies));
}

private static void registerRequestStartedCallback(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public class Reporter {
this(Config.get(), null);
}

public Reporter(final Config config, final AgentTaskScheduler taskScheduler) {
public Reporter(final Config config, @Nullable final AgentTaskScheduler taskScheduler) {
this(
config.isIastDeduplicationEnabled()
? new HashBasedDeduplication(taskScheduler)
Expand Down Expand Up @@ -108,11 +108,11 @@ protected static class HashBasedDeduplication implements Predicate<Vulnerability

private final Set<Long> hashes;

public HashBasedDeduplication(final AgentTaskScheduler taskScheduler) {
public HashBasedDeduplication(@Nullable final AgentTaskScheduler taskScheduler) {
this(DEFAULT_MAX_SIZE, taskScheduler);
}

HashBasedDeduplication(final int size, final AgentTaskScheduler taskScheduler) {
HashBasedDeduplication(final int size, @Nullable final AgentTaskScheduler taskScheduler) {
maxSize = size;
hashes = ConcurrentHashMap.newKeySet(size);
if (taskScheduler != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import static com.datadog.iast.IastTag.ANALYZED;
import static com.datadog.iast.IastTag.SKIPPED;

import com.datadog.iast.HasDependencies.Dependencies;
import com.datadog.iast.overhead.OverheadController;
import com.datadog.iast.taint.TaintedObjects;
import datadog.trace.api.gateway.Flow;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package com.datadog.iast;

import com.datadog.iast.HasDependencies.Dependencies;
import com.datadog.iast.overhead.OverheadController;
import datadog.trace.api.gateway.Flow;
import java.util.function.Supplier;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ public final class Evidence {
private final transient @Nonnull Context context = new Evidence.Context(4);

/** For deserialization in tests via moshi */
@Deprecated
@SuppressWarnings({"NullAway", "DataFlowIssue", "unused"})
private Evidence() {
this(null, null);
}
Expand All @@ -25,15 +27,17 @@ public Evidence(final String value) {
this(value, null);
}

public Evidence(final String value, final Range[] ranges) {
public Evidence(@Nonnull final String value, @Nullable final Range[] ranges) {
this.value = value;
this.ranges = ranges;
}

@Nonnull
public String getValue() {
return value;
}

@Nullable
public Range[] getRanges() {
return ranges;
}
Expand Down Expand Up @@ -76,6 +80,7 @@ public boolean put(final String key, final Object value) {
return true;
}

@Nullable
@SuppressWarnings("unchecked")
public <E> E get(@Nonnull final String key) {
return (E) context.get(key);
Expand Down
Loading

0 comments on commit 47bbfd7

Please sign in to comment.