Skip to content

Commit 44bab01

Browse files
amarzialijandro996
amarziali
authored and
jandro996
committed
Fix npe on ConflatingMetricsAggregator when the resource is null (#9909)
1 parent 4255dc0 commit 44bab01

File tree

18 files changed

+1192
-40
lines changed

18 files changed

+1192
-40
lines changed

‎dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/blocking/BlockingActionHelper.java‎

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ public class BlockingActionHelper {
2323
private static final int DEFAULT_HTTP_CODE = 403;
2424
private static final int MAX_ALLOWED_TEMPLATE_SIZE = 1024 * 500; // 500 kiB
2525

26+
// Pattern for removing security_response_id from HTML template when blockId is null/empty
27+
private static final Pattern HTML_SECURITY_RESPONSE_ID_PATTERN =
28+
Pattern.compile("<p[^>]*>Security Response ID: \\[security_response_id]</p>\\s*");
29+
2630
private static volatile byte[] TEMPLATE_HTML;
2731
private static volatile byte[] TEMPLATE_JSON;
2832

@@ -118,12 +122,42 @@ public static TemplateType determineTemplateType(
118122
}
119123

120124
public static byte[] getTemplate(TemplateType type) {
125+
return getTemplate(type, null);
126+
}
127+
128+
public static byte[] getTemplate(TemplateType type, String blockId) {
129+
byte[] template;
121130
if (type == TemplateType.JSON) {
122-
return TEMPLATE_JSON;
131+
template = TEMPLATE_JSON;
123132
} else if (type == TemplateType.HTML) {
124-
return TEMPLATE_HTML;
133+
template = TEMPLATE_HTML;
134+
} else {
135+
return null;
125136
}
126-
return null;
137+
138+
String templateString = new String(template, java.nio.charset.StandardCharsets.UTF_8);
139+
140+
if (blockId == null || blockId.isEmpty()) {
141+
// Remove the security_response_id field/placeholder entirely when blockId is not present
142+
if (type == TemplateType.JSON) {
143+
// Remove the entire security_response_id field from JSON:
144+
// ,"security_response_id":"[security_response_id]"
145+
// Try both variants: with comma before and with comma after
146+
templateString =
147+
templateString.replace(",\"security_response_id\":\"[security_response_id]\"", "");
148+
templateString =
149+
templateString.replace("\"security_response_id\":\"[security_response_id]\",", "");
150+
} else {
151+
// For HTML, remove the entire security_response_id section including any attributes
152+
Matcher matcher = HTML_SECURITY_RESPONSE_ID_PATTERN.matcher(templateString);
153+
templateString = matcher.replaceFirst("");
154+
}
155+
return templateString.getBytes(java.nio.charset.StandardCharsets.UTF_8);
156+
}
157+
158+
// Perform placeholder replacement for [security_response_id]
159+
String replacedTemplate = templateString.replace("[security_response_id]", blockId);
160+
return replacedTemplate.getBytes(java.nio.charset.StandardCharsets.UTF_8);
127161
}
128162

129163
public static String getContentType(TemplateType type) {

‎dd-java-agent/agent-bootstrap/src/main/resources/datadog/trace/bootstrap/blocking/template.html‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
<!DOCTYPE html><html lang="en"><head><meta charset="UTF-8"><meta name="viewport" content="width=device-width,initial-scale=1"><title>You've been blocked</title><style>a,body,div,html,span{margin:0;padding:0;border:0;font-size:100%;font:inherit;vertical-align:baseline}body{background:-webkit-radial-gradient(26% 19%,circle,#fff,#f4f7f9);background:radial-gradient(circle at 26% 19%,#fff,#f4f7f9);display:-webkit-box;display:-ms-flexbox;display:flex;-webkit-box-pack:center;-ms-flex-pack:center;justify-content:center;-webkit-box-align:center;-ms-flex-align:center;align-items:center;-ms-flex-line-pack:center;align-content:center;width:100%;min-height:100vh;line-height:1;flex-direction:column}p{display:block}main{text-align:center;flex:1;display:-webkit-box;display:-ms-flexbox;display:flex;-webkit-box-pack:center;-ms-flex-pack:center;justify-content:center;-webkit-box-align:center;-ms-flex-align:center;align-items:center;-ms-flex-line-pack:center;align-content:center;flex-direction:column}p{font-size:18px;line-height:normal;color:#646464;font-family:sans-serif;font-weight:400}a{color:#4842b7}footer{width:100%;text-align:center}footer p{font-size:16px}</style></head><body><main><p>Sorry, you cannot access this page. Please contact the customer service team.</p></main><footer><p>Security provided by <a href="https://www.datadoghq.com/product/security-platform/application-security-monitoring/" target="_blank">Datadog</a></p></footer></body></html>
1+
<!DOCTYPE html><html lang="en"><head><meta charset="UTF-8"><meta name="viewport" content="width=device-width,initial-scale=1"><title>You've been blocked</title><style>a,body,div,html,span{margin:0;padding:0;border:0;font-size:100%;font:inherit;vertical-align:baseline}body{background:-webkit-radial-gradient(26% 19%,circle,#fff,#f4f7f9);background:radial-gradient(circle at 26% 19%,#fff,#f4f7f9);display:-webkit-box;display:-ms-flexbox;display:flex;-webkit-box-pack:center;-ms-flex-pack:center;justify-content:center;-webkit-box-align:center;-ms-flex-align:center;align-items:center;-ms-flex-line-pack:center;align-content:center;width:100%;min-height:100vh;line-height:1;flex-direction:column}p{display:block}main{text-align:center;flex:1;display:-webkit-box;display:-ms-flexbox;display:flex;-webkit-box-pack:center;-ms-flex-pack:center;justify-content:center;-webkit-box-align:center;-ms-flex-align:center;align-items:center;-ms-flex-line-pack:center;align-content:center;flex-direction:column}p{font-size:18px;line-height:normal;color:#646464;font-family:sans-serif;font-weight:400}a{color:#4842b7}footer{width:100%;text-align:center}footer p{font-size:16px}.security-response-id{font-size:14px;color:#999;margin-top:20px;font-family:monospace}</style></head><body><main><p>Sorry, you cannot access this page. Please contact the customer service team.</p><p class="security-response-id">Security Response ID: [security_response_id]</p></main><footer><p>Security provided by <a href="https://www.datadoghq.com/product/security-platform/application-security-monitoring/" target="_blank">Datadog</a></p></footer></body></html>

‎dd-java-agent/agent-bootstrap/src/main/resources/datadog/trace/bootstrap/blocking/template.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
{"errors":[{"title":"You've been blocked","detail":"Sorry, you cannot access this page. Please contact the customer service team. Security provided by Datadog."}]}
1+
{"errors":[{"title":"You've been blocked","detail":"Sorry, you cannot access this page. Please contact the customer service team. Security provided by Datadog."}],"security_response_id":"[security_response_id]"}

‎dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/blocking/BlockingActionHelperSpecification.groovy‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,4 +167,110 @@ class BlockingActionHelperSpecification extends DDSpecification {
167167
BlockingActionHelper.reset(Config.get())
168168
tempDir.deleteDir()
169169
}
170+
171+
void 'getTemplate with block_id replaces placeholder in HTML template'() {
172+
given:
173+
def blockId = '12345678-1234-1234-1234-123456789abc'
174+
175+
when:
176+
def template = BlockingActionHelper.getTemplate(HTML, blockId)
177+
def templateStr = new String(template, StandardCharsets.UTF_8)
178+
179+
then:
180+
templateStr.contains("Security Response ID: ${blockId}")
181+
!templateStr.contains('[security_response_id]')
182+
}
183+
184+
void 'getTemplate with block_id replaces placeholder in JSON template'() {
185+
given:
186+
def blockId = '12345678-1234-1234-1234-123456789abc'
187+
188+
when:
189+
def template = BlockingActionHelper.getTemplate(JSON, blockId)
190+
def templateStr = new String(template, StandardCharsets.UTF_8)
191+
192+
then:
193+
templateStr.contains("\"security_response_id\":\"${blockId}\"")
194+
!templateStr.contains('[security_response_id]')
195+
}
196+
197+
void 'getTemplate without block_id removes block_id field from HTML template'() {
198+
when:
199+
def template = BlockingActionHelper.getTemplate(HTML, null)
200+
def templateStr = new String(template, StandardCharsets.UTF_8)
201+
202+
then:
203+
!templateStr.contains('[security_response_id]')
204+
!templateStr.contains('Security Response ID:')
205+
}
206+
207+
void 'getTemplate without block_id removes block_id field from JSON template'() {
208+
when:
209+
def template = BlockingActionHelper.getTemplate(JSON, null)
210+
def templateStr = new String(template, StandardCharsets.UTF_8)
211+
212+
then:
213+
!templateStr.contains('[security_response_id]')
214+
!templateStr.contains('"security_response_id"')
215+
}
216+
217+
void 'getTemplate with empty block_id removes block_id field'() {
218+
when:
219+
def htmlTemplate = BlockingActionHelper.getTemplate(HTML, '')
220+
def jsonTemplate = BlockingActionHelper.getTemplate(JSON, '')
221+
222+
then:
223+
!new String(htmlTemplate, StandardCharsets.UTF_8).contains('[security_response_id]')
224+
!new String(htmlTemplate, StandardCharsets.UTF_8).contains('Security Response ID:')
225+
!new String(jsonTemplate, StandardCharsets.UTF_8).contains('[security_response_id]')
226+
!new String(jsonTemplate, StandardCharsets.UTF_8).contains('"security_response_id"')
227+
}
228+
229+
void 'getTemplate with block_id works with custom HTML template'() {
230+
setup:
231+
File tempDir = File.createTempDir('testTempDir-', '')
232+
Config config = Mock(Config)
233+
File tempFile = new File(tempDir, 'template.html')
234+
tempFile << '<body>Custom template with security_response_id: [security_response_id]</body>'
235+
def blockId = 'test-block-id-123'
236+
237+
when:
238+
BlockingActionHelper.reset(config)
239+
def template = BlockingActionHelper.getTemplate(HTML, blockId)
240+
def templateStr = new String(template, StandardCharsets.UTF_8)
241+
242+
then:
243+
1 * config.getAppSecHttpBlockedTemplateHtml() >> tempFile.toString()
244+
1 * config.getAppSecHttpBlockedTemplateJson() >> null
245+
templateStr.contains("Custom template with security_response_id: ${blockId}")
246+
!templateStr.contains('[security_response_id]')
247+
248+
cleanup:
249+
BlockingActionHelper.reset(Config.get())
250+
tempDir.deleteDir()
251+
}
252+
253+
void 'getTemplate with block_id works with custom JSON template'() {
254+
setup:
255+
File tempDir = File.createTempDir('testTempDir-', '')
256+
Config config = Mock(Config)
257+
File tempFile = new File(tempDir, 'template.json')
258+
tempFile << '{"error":"blocked","id":"[security_response_id]"}'
259+
def blockId = 'test-block-id-456'
260+
261+
when:
262+
BlockingActionHelper.reset(config)
263+
def template = BlockingActionHelper.getTemplate(JSON, blockId)
264+
def templateStr = new String(template, StandardCharsets.UTF_8)
265+
266+
then:
267+
1 * config.getAppSecHttpBlockedTemplateHtml() >> null
268+
1 * config.getAppSecHttpBlockedTemplateJson() >> tempFile.toString()
269+
templateStr.contains("\"error\":\"blocked\",\"id\":\"${blockId}\"")
270+
!templateStr.contains('[security_response_id]')
271+
272+
cleanup:
273+
BlockingActionHelper.reset(Config.get())
274+
tempDir.deleteDir()
275+
}
170276
}

‎dd-java-agent/appsec/build.gradle‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ dependencies {
1515
implementation project(':internal-api')
1616
implementation project(':communication')
1717
implementation project(':telemetry')
18-
implementation group: 'io.sqreen', name: 'libsqreen', version: '17.2.0'
18+
implementation group: 'io.sqreen', name: 'libsqreen', version: '17.3.0'
1919
implementation libs.moshi
2020

2121
testImplementation libs.bytebuddy

‎dd-java-agent/appsec/src/main/java/com/datadog/appsec/ddwaf/WAFModule.java‎

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,7 @@ public void onDataAvailable(
363363
WafMetricCollector.get().raspRuleMatch(gwCtx.raspRuleType);
364364
}
365365

366+
String security_response_id = null;
366367
for (Map.Entry<String, Map<String, Object>> action : resultWithData.actions.entrySet()) {
367368
String actionType = action.getKey();
368369
Map<String, Object> actionParams = action.getValue();
@@ -373,10 +374,14 @@ public void onDataAvailable(
373374
Flow.Action.RequestBlockingAction rba =
374375
createBlockRequestAction(actionInfo, reqCtx, gwCtx.isRasp);
375376
flow.setAction(rba);
377+
// Extract security_response_id from action parameters for use in triggers
378+
security_response_id = rba.getSecurityResponseId();
376379
} else if ("redirect_request".equals(actionInfo.type)) {
377380
Flow.Action.RequestBlockingAction rba =
378381
createRedirectRequestAction(actionInfo, reqCtx, gwCtx.isRasp);
379382
flow.setAction(rba);
383+
// Extract security_response_id from action parameters for use in triggers
384+
security_response_id = rba.getSecurityResponseId();
380385
} else if ("generate_stack".equals(actionInfo.type)) {
381386
if (Config.get().isAppSecStackTraceEnabled()) {
382387
String stackId = (String) actionInfo.parameters.get("stack_id");
@@ -412,7 +417,7 @@ public void onDataAvailable(
412417
}
413418
}
414419
}
415-
Collection<AppSecEvent> events = buildEvents(resultWithData);
420+
Collection<AppSecEvent> events = buildEvents(resultWithData, security_response_id);
416421
boolean isThrottled = reqCtx.isThrottled(rateLimiter);
417422

418423
if (!isThrottled) {
@@ -477,7 +482,9 @@ private Flow.Action.RequestBlockingAction createBlockRequestAction(
477482
} catch (IllegalArgumentException iae) {
478483
log.warn("Unknown content type: {}; using auto", contentType);
479484
}
480-
return new Flow.Action.RequestBlockingAction(statusCode, blockingContentType);
485+
String securityResponseId = (String) actionInfo.parameters.get("security_response_id");
486+
return new Flow.Action.RequestBlockingAction(
487+
statusCode, blockingContentType, Collections.emptyMap(), securityResponseId);
481488
} catch (RuntimeException cce) {
482489
log.warn("Invalid blocking action data", cce);
483490
if (!isRasp) {
@@ -506,7 +513,18 @@ private Flow.Action.RequestBlockingAction createRedirectRequestAction(
506513
if (location == null) {
507514
throw new RuntimeException("redirect_request action has no location");
508515
}
509-
return Flow.Action.RequestBlockingAction.forRedirect(statusCode, location);
516+
String securityResponseId = (String) actionInfo.parameters.get("security_response_id");
517+
if (securityResponseId != null && !securityResponseId.isEmpty()) {
518+
// For custom redirects, only replace [security_response_id] placeholder if present in the
519+
// URL.
520+
// The client decides whether to include security_response_id by adding the placeholder.
521+
// We don't automatically append security_response_id as a URL parameter.
522+
if (location.contains("[security_response_id]")) {
523+
location = location.replace("[security_response_id]", securityResponseId);
524+
}
525+
}
526+
return Flow.Action.RequestBlockingAction.forRedirect(
527+
statusCode, location, securityResponseId);
510528
} catch (RuntimeException cce) {
511529
log.warn("Invalid blocking action data", cce);
512530
if (!isRasp) {
@@ -572,7 +590,7 @@ private Waf.ResultWithData runWafTransient(
572590
new DataBundleMapWrapper(ctxAndAddr.addressesOfInterest, newData), LIMITS, metrics);
573591
}
574592

575-
private Collection<AppSecEvent> buildEvents(Waf.ResultWithData actionWithData) {
593+
private Collection<AppSecEvent> buildEvents(Waf.ResultWithData actionWithData, String blockId) {
576594
if (actionWithData.data == null) {
577595
log.debug(SEND_TELEMETRY, "WAF result data is null");
578596
return Collections.emptyList();
@@ -590,14 +608,14 @@ private Collection<AppSecEvent> buildEvents(Waf.ResultWithData actionWithData) {
590608

591609
if (listResults != null && !listResults.isEmpty()) {
592610
return listResults.stream()
593-
.map(this::buildEvent)
611+
.map(wafResult -> buildEvent(wafResult, blockId))
594612
.filter(Objects::nonNull)
595613
.collect(Collectors.toList());
596614
}
597615
return emptyList();
598616
}
599617

600-
private AppSecEvent buildEvent(WAFResultData wafResult) {
618+
private AppSecEvent buildEvent(WAFResultData wafResult, String blockId) {
601619

602620
if (wafResult == null || wafResult.rule == null || wafResult.rule_matches == null) {
603621
log.warn("WAF result is empty: {}", wafResult);
@@ -615,6 +633,7 @@ private AppSecEvent buildEvent(WAFResultData wafResult) {
615633
.withRuleMatches(wafResult.rule_matches)
616634
.withSpanId(spanId)
617635
.withStackId(wafResult.stack_id)
636+
.withSecurityResponseId(blockId)
618637
.build();
619638
}
620639

‎dd-java-agent/appsec/src/main/java/com/datadog/appsec/report/AppSecEvent.java‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@ public class AppSecEvent {
2121
@com.squareup.moshi.Json(name = "stack_id")
2222
private String stackId;
2323

24+
@com.squareup.moshi.Json(name = "security_response_id")
25+
private String securityResponseId;
26+
2427
public Rule getRule() {
2528
return rule;
2629
}
@@ -37,6 +40,10 @@ public String getStackId() {
3740
return stackId;
3841
}
3942

43+
public String getSecurityResponseId() {
44+
return securityResponseId;
45+
}
46+
4047
@Override
4148
public String toString() {
4249
StringBuilder sb = new StringBuilder();
@@ -58,6 +65,10 @@ public String toString() {
5865
sb.append("stackId");
5966
sb.append('=');
6067
sb.append(((this.stackId == null) ? "<null>" : this.stackId));
68+
sb.append(',');
69+
sb.append("securityResponseId");
70+
sb.append('=');
71+
sb.append(((this.securityResponseId == null) ? "<null>" : this.securityResponseId));
6172
if (sb.charAt((sb.length() - 1)) == ',') {
6273
sb.setCharAt((sb.length() - 1), ']');
6374
} else {
@@ -73,6 +84,9 @@ public int hashCode() {
7384
result = ((result * 31) + ((this.ruleMatches == null) ? 0 : this.ruleMatches.hashCode()));
7485
result = ((result * 31) + ((this.spanId == null) ? 0 : this.spanId.hashCode()));
7586
result = ((result * 31) + ((this.stackId == null) ? 0 : this.stackId.hashCode()));
87+
result =
88+
((result * 31)
89+
+ ((this.securityResponseId == null) ? 0 : this.securityResponseId.hashCode()));
7690
return result;
7791
}
7892

@@ -88,7 +102,8 @@ public boolean equals(Object other) {
88102
return ((Objects.equals(this.rule, rhs.rule))
89103
&& (Objects.equals(this.ruleMatches, rhs.ruleMatches))
90104
&& (Objects.equals(this.spanId, rhs.spanId))
91-
&& (Objects.equals(this.stackId, rhs.stackId)));
105+
&& (Objects.equals(this.stackId, rhs.stackId))
106+
&& (Objects.equals(this.securityResponseId, rhs.securityResponseId)));
92107
}
93108

94109
public static class Builder {
@@ -125,5 +140,10 @@ public Builder withStackId(String stackId) {
125140
this.instance.stackId = stackId;
126141
return this;
127142
}
143+
144+
public Builder withSecurityResponseId(String securityResponseId) {
145+
this.instance.securityResponseId = securityResponseId;
146+
return this;
147+
}
128148
}
129149
}

0 commit comments

Comments
 (0)