Fix NullPointerException in unvalidated redirect detection#5755
Merged
Conversation
Under some circumstances, the `name` field of an IAST source can be `null` (very rare, and generally an indicative of another issue, but it's currently legal). In this case, the unvalidated redirect detection can throw a NPE. This NPE is caught in the instrumentation, so it will not break the app, but it will log to debug: ``` [dd.trace 2023-08-22 18:49:20:322 +0200] [http-nio-20002-exec-3] DEBUG datadog.trace.bootstrap.ExceptionLogger - Failed to handle exception in instrumentation for org.springframework.web.method.support.InvocableHandlerMethod java.lang.NullPointerException at com.datadog.iast.sink.UnvalidatedRedirectModuleImpl.isRefererHeader(UnvalidatedRedirectModuleImpl.java:102) at com.datadog.iast.sink.UnvalidatedRedirectModuleImpl.checkUnvalidatedRedirect(UnvalidatedRedirectModuleImpl.java:74) at com.datadog.iast.sink.UnvalidatedRedirectModuleImpl.onRedirect(UnvalidatedRedirectModuleImpl.java:41) at org.springframework.web.method.support.InvocableHandlerMethod.invokeForRequest(InvocableHandlerMethod.java:150) at org.springframework.web.servlet.mvc.method.annotation.ServletInvocableHandlerMethod.invokeAndHandle(ServletInvocableHandlerMethod.java:117) at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.invokeHandlerMethod(RequestMappingHandlerAdapter.java:895) at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.handleInternal(RequestMappingHandlerAdapter.java:808) at org.springframework.web.servlet.mvc.method.AbstractHandlerMethodAdapter.handle(AbstractHandlerMethodAdapter.java:87) at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:1070) at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:963) at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:1006) at org.springframework.web.servlet.FrameworkServlet.doGet(FrameworkServlet.java:898) at javax.servlet.http.HttpServlet.service(HttpServlet.java:655) ```
smola
added
type: bug fix
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 62 cases. Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.20.0-SNAPSHOT~84c825fda7, baseline=1.20.0-SNAPSHOT~44d47a398d
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (990.298 ms) : 0, 990298
Total [baseline] (9.199 s) : 0, 9198841
Agent [candidate] (992.525 ms) : 0, 992525
Total [candidate] (9.244 s) : 0, 9244144
section appsec
Agent [baseline] (1.073 s) : 0, 1073499
Total [baseline] (9.28 s) : 0, 9280020
Agent [candidate] (1.073 s) : 0, 1072553
Total [candidate] (9.316 s) : 0, 9316077
section iast
Agent [baseline] (1.092 s) : 0, 1092150
Total [baseline] (9.44 s) : 0, 9439710
Agent [candidate] (1.099 s) : 0, 1098789
Total [candidate] (9.435 s) : 0, 9434553
section profiling
Agent [baseline] (1.165 s) : 0, 1165397
Total [baseline] (9.547 s) : 0, 9547412
Agent [candidate] (1.16 s) : 0, 1159535
Total [candidate] (9.508 s) : 0, 9508237
gantt
title petclinic - break down per module: candidate=1.20.0-SNAPSHOT~84c825fda7, baseline=1.20.0-SNAPSHOT~44d47a398d
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (613.457 ms) : 0, 613457
BytebuddyAgent [candidate] (614.853 ms) : 0, 614853
GlobalTracer [baseline] (285.322 ms) : 0, 285322
GlobalTracer [candidate] (286.668 ms) : 0, 286668
AppSec [baseline] (50.333 ms) : 0, 50333
AppSec [candidate] (49.755 ms) : 0, 49755
Remote Config [baseline] (668.947 µs) : 0, 669
Remote Config [candidate] (672.931 µs) : 0, 673
Telemetry [baseline] (6.058 ms) : 0, 6058
Telemetry [candidate] (6.061 ms) : 0, 6061
section appsec
BytebuddyAgent [baseline] (612.834 ms) : 0, 612834
BytebuddyAgent [candidate] (611.707 ms) : 0, 611707
GlobalTracer [baseline] (284.92 ms) : 0, 284920
GlobalTracer [candidate] (285.405 ms) : 0, 285405
AppSec [baseline] (134.901 ms) : 0, 134901
AppSec [candidate] (134.62 ms) : 0, 134620
Remote Config [baseline] (722.511 µs) : 0, 723
Remote Config [candidate] (730.386 µs) : 0, 730
Telemetry [baseline] (5.769 ms) : 0, 5769
Telemetry [candidate] (5.767 ms) : 0, 5767
section iast
BytebuddyAgent [baseline] (718.838 ms) : 0, 718838
BytebuddyAgent [candidate] (723.512 ms) : 0, 723512
GlobalTracer [baseline] (271.351 ms) : 0, 271351
GlobalTracer [candidate] (272.714 ms) : 0, 272714
AppSec [baseline] (47.331 ms) : 0, 47331
AppSec [candidate] (47.624 ms) : 0, 47624
Remote Config [baseline] (556.796 µs) : 0, 557
Remote Config [candidate] (561.302 µs) : 0, 561
Telemetry [baseline] (5.748 ms) : 0, 5748
Telemetry [candidate] (5.774 ms) : 0, 5774
IAST [baseline] (13.892 ms) : 0, 13892
IAST [candidate] (13.965 ms) : 0, 13965
section profiling
BytebuddyAgent [baseline] (629.293 ms) : 0, 629293
BytebuddyAgent [candidate] (625.726 ms) : 0, 625726
GlobalTracer [baseline] (347.947 ms) : 0, 347947
GlobalTracer [candidate] (347.401 ms) : 0, 347401
AppSec [baseline] (49.587 ms) : 0, 49587
AppSec [candidate] (49.17 ms) : 0, 49170
Remote Config [baseline] (650.663 µs) : 0, 651
Remote Config [candidate] (656.512 µs) : 0, 657
Telemetry [baseline] (6.002 ms) : 0, 6002
Telemetry [candidate] (5.967 ms) : 0, 5967
ProfilingAgent [baseline] (78.516 ms) : 0, 78516
ProfilingAgent [candidate] (77.656 ms) : 0, 77656
Profiling [baseline] (78.541 ms) : 0, 78541
Profiling [candidate] (77.681 ms) : 0, 77681
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.20.0-SNAPSHOT~84c825fda7, baseline=1.20.0-SNAPSHOT~44d47a398d
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (996.2 ms) : 0, 996200
Total [baseline] (8.683 s) : 0, 8682546
Agent [candidate] (987.783 ms) : 0, 987783
Total [candidate] (8.667 s) : 0, 8667390
section appsec
Agent [baseline] (1.074 s) : 0, 1073981
Total [baseline] (8.727 s) : 0, 8726919
Agent [candidate] (1.074 s) : 0, 1074468
Total [candidate] (8.721 s) : 0, 8720621
section iast
Agent [baseline] (1.097 s) : 0, 1096891
Total [baseline] (9.182 s) : 0, 9181585
Agent [candidate] (1.092 s) : 0, 1092488
Total [candidate] (9.169 s) : 0, 9169379
section profiling
Agent [baseline] (1.159 s) : 0, 1159084
Total [baseline] (8.884 s) : 0, 8883818
Agent [candidate] (1.164 s) : 0, 1164367
Total [candidate] (8.871 s) : 0, 8871121
gantt
title insecure-bank - break down per module: candidate=1.20.0-SNAPSHOT~84c825fda7, baseline=1.20.0-SNAPSHOT~44d47a398d
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (616.834 ms) : 0, 616834
BytebuddyAgent [candidate] (611.965 ms) : 0, 611965
GlobalTracer [baseline] (287.47 ms) : 0, 287470
GlobalTracer [candidate] (284.785 ms) : 0, 284785
AppSec [baseline] (50.441 ms) : 0, 50441
AppSec [candidate] (50.049 ms) : 0, 50049
Remote Config [baseline] (677.103 µs) : 0, 677
Remote Config [candidate] (663.474 µs) : 0, 663
Telemetry [baseline] (6.077 ms) : 0, 6077
Telemetry [candidate] (5.963 ms) : 0, 5963
section appsec
BytebuddyAgent [baseline] (612.051 ms) : 0, 612051
BytebuddyAgent [candidate] (612.815 ms) : 0, 612815
GlobalTracer [baseline] (285.288 ms) : 0, 285288
GlobalTracer [candidate] (285.652 ms) : 0, 285652
AppSec [baseline] (135.807 ms) : 0, 135807
AppSec [candidate] (135.086 ms) : 0, 135086
Remote Config [baseline] (723.941 µs) : 0, 724
Remote Config [candidate] (733.27 µs) : 0, 733
Telemetry [baseline] (5.771 ms) : 0, 5771
Telemetry [candidate] (5.787 ms) : 0, 5787
section iast
BytebuddyAgent [baseline] (722.782 ms) : 0, 722782
BytebuddyAgent [candidate] (719.073 ms) : 0, 719073
GlobalTracer [baseline] (271.351 ms) : 0, 271351
GlobalTracer [candidate] (271.438 ms) : 0, 271438
AppSec [baseline] (47.822 ms) : 0, 47822
AppSec [candidate] (47.482 ms) : 0, 47482
Remote Config [baseline] (555.22 µs) : 0, 555
Remote Config [candidate] (542.726 µs) : 0, 543
Telemetry [baseline] (5.774 ms) : 0, 5774
Telemetry [candidate] (5.73 ms) : 0, 5730
IAST [baseline] (14.023 ms) : 0, 14023
IAST [candidate] (13.859 ms) : 0, 13859
section profiling
BytebuddyAgent [baseline] (625.567 ms) : 0, 625567
BytebuddyAgent [candidate] (628.444 ms) : 0, 628444
GlobalTracer [baseline] (346.933 ms) : 0, 346933
GlobalTracer [candidate] (349.001 ms) : 0, 349001
AppSec [baseline] (49.24 ms) : 0, 49240
AppSec [candidate] (49.108 ms) : 0, 49108
Remote Config [baseline] (649.013 µs) : 0, 649
Remote Config [candidate] (655.625 µs) : 0, 656
Telemetry [baseline] (5.99 ms) : 0, 5990
Telemetry [candidate] (6.024 ms) : 0, 6024
ProfilingAgent [baseline] (77.739 ms) : 0, 77739
ProfilingAgent [candidate] (77.807 ms) : 0, 77807
Profiling [baseline] (77.765 ms) : 0, 77765
Profiling [candidate] (77.831 ms) : 0, 77831
LoadParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 24 cases. Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.20.0-SNAPSHOT~84c825fda7, baseline=1.20.0-SNAPSHOT~44d47a398d
dateFormat X
axisFormat %s
section baseline
no_agent (1.34 ms) : 1321, 1359
. : milestone, 1340,
appsec (1.68 ms) : 1656, 1705
. : milestone, 1680,
iast (1.444 ms) : 1419, 1468
. : milestone, 1444,
profiling (1.469 ms) : 1445, 1493
. : milestone, 1469,
tracing (1.443 ms) : 1418, 1467
. : milestone, 1443,
section candidate
no_agent (1.324 ms) : 1305, 1342
. : milestone, 1324,
appsec (1.685 ms) : 1660, 1710
. : milestone, 1685,
iast (1.48 ms) : 1456, 1504
. : milestone, 1480,
profiling (1.487 ms) : 1460, 1513
. : milestone, 1487,
tracing (1.46 ms) : 1436, 1484
. : milestone, 1460,
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.20.0-SNAPSHOT~84c825fda7, baseline=1.20.0-SNAPSHOT~44d47a398d
dateFormat X
axisFormat %s
section baseline
no_agent (360.655 µs) : 340, 381
. : milestone, 361,
appsec (659.88 µs) : 639, 681
. : milestone, 660,
iast (456.303 µs) : 435, 477
. : milestone, 456,
iast_FULL (515.268 µs) : 495, 536
. : milestone, 515,
iast_INACTIVE (430.865 µs) : 410, 452
. : milestone, 431,
profiling (439.878 µs) : 419, 461
. : milestone, 440,
tracing (436.258 µs) : 415, 457
. : milestone, 436,
section candidate
no_agent (360.568 µs) : 340, 381
. : milestone, 361,
appsec (664.263 µs) : 644, 685
. : milestone, 664,
iast (465.36 µs) : 444, 487
. : milestone, 465,
iast_FULL (518.554 µs) : 498, 539
. : milestone, 519,
iast_INACTIVE (435.522 µs) : 415, 456
. : milestone, 436,
profiling (437.972 µs) : 417, 459
. : milestone, 438,
tracing (431.648 µs) : 411, 452
. : milestone, 432,
|
manuel-alvarez-alvarez
approved these changes
Aug 23, 2023
jandro996
approved these changes
Aug 23, 2023
smola
added a commit
that referenced
this pull request
Aug 23, 2023
Under some circumstances, the `name` field of an IAST source can be `null` (very rare, and generally an indicative of another issue, but it's currently legal). In this case, the unvalidated redirect detection can throw a NPE. This NPE is caught in the instrumentation, so it will not break the app, but it will log to debug: ``` [dd.trace 2023-08-22 18:49:20:322 +0200] [http-nio-20002-exec-3] DEBUG datadog.trace.bootstrap.ExceptionLogger - Failed to handle exception in instrumentation for org.springframework.web.method.support.InvocableHandlerMethod java.lang.NullPointerException at com.datadog.iast.sink.UnvalidatedRedirectModuleImpl.isRefererHeader(UnvalidatedRedirectModuleImpl.java:102) at com.datadog.iast.sink.UnvalidatedRedirectModuleImpl.checkUnvalidatedRedirect(UnvalidatedRedirectModuleImpl.java:74) at com.datadog.iast.sink.UnvalidatedRedirectModuleImpl.onRedirect(UnvalidatedRedirectModuleImpl.java:41) at org.springframework.web.method.support.InvocableHandlerMethod.invokeForRequest(InvocableHandlerMethod.java:150) at org.springframework.web.servlet.mvc.method.annotation.ServletInvocableHandlerMethod.invokeAndHandle(ServletInvocableHandlerMethod.java:117) at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.invokeHandlerMethod(RequestMappingHandlerAdapter.java:895) at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.handleInternal(RequestMappingHandlerAdapter.java:808) at org.springframework.web.servlet.mvc.method.AbstractHandlerMethodAdapter.handle(AbstractHandlerMethodAdapter.java:87) at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:1070) at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:963) at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:1006) at org.springframework.web.servlet.FrameworkServlet.doGet(FrameworkServlet.java:898) at javax.servlet.http.HttpServlet.service(HttpServlet.java:655) ```
smola
added a commit
that referenced
this pull request
Aug 23, 2023
…5759) Under some circumstances, the `name` field of an IAST source can be `null` (very rare, and generally an indicative of another issue, but it's currently legal). In this case, the unvalidated redirect detection can throw a NPE. This NPE is caught in the instrumentation, so it will not break the app, but it will log to debug: ``` [dd.trace 2023-08-22 18:49:20:322 +0200] [http-nio-20002-exec-3] DEBUG datadog.trace.bootstrap.ExceptionLogger - Failed to handle exception in instrumentation for org.springframework.web.method.support.InvocableHandlerMethod java.lang.NullPointerException at com.datadog.iast.sink.UnvalidatedRedirectModuleImpl.isRefererHeader(UnvalidatedRedirectModuleImpl.java:102) at com.datadog.iast.sink.UnvalidatedRedirectModuleImpl.checkUnvalidatedRedirect(UnvalidatedRedirectModuleImpl.java:74) at com.datadog.iast.sink.UnvalidatedRedirectModuleImpl.onRedirect(UnvalidatedRedirectModuleImpl.java:41) at org.springframework.web.method.support.InvocableHandlerMethod.invokeForRequest(InvocableHandlerMethod.java:150) at org.springframework.web.servlet.mvc.method.annotation.ServletInvocableHandlerMethod.invokeAndHandle(ServletInvocableHandlerMethod.java:117) at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.invokeHandlerMethod(RequestMappingHandlerAdapter.java:895) at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.handleInternal(RequestMappingHandlerAdapter.java:808) at org.springframework.web.servlet.mvc.method.AbstractHandlerMethodAdapter.handle(AbstractHandlerMethodAdapter.java:87) at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:1070) at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:963) at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:1006) at org.springframework.web.servlet.FrameworkServlet.doGet(FrameworkServlet.java:898) at javax.servlet.http.HttpServlet.service(HttpServlet.java:655) ```
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What Does This Do
Under some circumstances, the
namefield of an IAST source can benull(very rare, and generally an indicative of another issue, but it's currently legal). In this case, the unvalidated redirect detection can throw a NPE.This NPE is caught in the instrumentation, so it will not break the app, but it will log to debug:
Motivation
Additional Notes