-
Notifications
You must be signed in to change notification settings - Fork 292
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add experimental taint propagation to the String replace, replaceFirst, replaceAll methods #7741
Conversation
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 55 metrics, 8 unstable metrics. Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.43.0-SNAPSHOT~9bd24c3774, baseline=1.43.0-SNAPSHOT~8233fc5ff8
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.091 s) : 0, 1091436
Total [baseline] (10.382 s) : 0, 10382414
Agent [candidate] (1.084 s) : 0, 1083545
Total [candidate] (10.368 s) : 0, 10367757
section appsec
Agent [baseline] (1.223 s) : 0, 1223113
Total [baseline] (10.706 s) : 0, 10706018
Agent [candidate] (1.216 s) : 0, 1216157
Total [candidate] (10.699 s) : 0, 10699455
section iast
Agent [baseline] (1.219 s) : 0, 1218775
Total [baseline] (10.953 s) : 0, 10952952
Agent [candidate] (1.208 s) : 0, 1208432
Total [candidate] (10.957 s) : 0, 10957490
section profiling
Agent [baseline] (1.279 s) : 0, 1278994
Total [baseline] (10.736 s) : 0, 10735746
Agent [candidate] (1.279 s) : 0, 1278532
Total [candidate] (10.741 s) : 0, 10740989
gantt
title petclinic - break down per module: candidate=1.43.0-SNAPSHOT~9bd24c3774, baseline=1.43.0-SNAPSHOT~8233fc5ff8
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (693.084 ms) : 0, 693084
BytebuddyAgent [candidate] (688.232 ms) : 0, 688232
GlobalTracer [baseline] (318.109 ms) : 0, 318109
GlobalTracer [candidate] (315.976 ms) : 0, 315976
AppSec [baseline] (54.55 ms) : 0, 54550
AppSec [candidate] (54.21 ms) : 0, 54210
Remote Config [baseline] (679.131 µs) : 0, 679
Remote Config [candidate] (669.432 µs) : 0, 669
Telemetry [baseline] (11.208 ms) : 0, 11208
Telemetry [candidate] (10.716 ms) : 0, 10716
section appsec
BytebuddyAgent [baseline] (709.466 ms) : 0, 709466
BytebuddyAgent [candidate] (705.036 ms) : 0, 705036
GlobalTracer [baseline] (315.14 ms) : 0, 315140
GlobalTracer [candidate] (312.648 ms) : 0, 312648
AppSec [baseline] (164.852 ms) : 0, 164852
AppSec [candidate] (166.696 ms) : 0, 166696
Remote Config [baseline] (642.005 µs) : 0, 642
Remote Config [candidate] (634.522 µs) : 0, 635
Telemetry [baseline] (8.85 ms) : 0, 8850
Telemetry [candidate] (7.741 ms) : 0, 7741
IAST [baseline] (20.792 ms) : 0, 20792
IAST [candidate] (19.357 ms) : 0, 19357
section iast
BytebuddyAgent [baseline] (810.944 ms) : 0, 810944
BytebuddyAgent [candidate] (804.154 ms) : 0, 804154
GlobalTracer [baseline] (307.096 ms) : 0, 307096
GlobalTracer [candidate] (304.571 ms) : 0, 304571
AppSec [baseline] (57.148 ms) : 0, 57148
AppSec [candidate] (57.471 ms) : 0, 57471
Remote Config [baseline] (628.952 µs) : 0, 629
Remote Config [candidate] (600.118 µs) : 0, 600
Telemetry [baseline] (7.502 ms) : 0, 7502
Telemetry [candidate] (7.417 ms) : 0, 7417
IAST [baseline] (21.621 ms) : 0, 21621
IAST [candidate] (20.469 ms) : 0, 20469
section profiling
BytebuddyAgent [baseline] (681.98 ms) : 0, 681980
BytebuddyAgent [candidate] (681.09 ms) : 0, 681090
GlobalTracer [baseline] (398.401 ms) : 0, 398401
GlobalTracer [candidate] (397.783 ms) : 0, 397783
AppSec [baseline] (54.544 ms) : 0, 54544
AppSec [candidate] (54.772 ms) : 0, 54772
Remote Config [baseline] (679.355 µs) : 0, 679
Remote Config [candidate] (686.03 µs) : 0, 686
Telemetry [baseline] (10.085 ms) : 0, 10085
Telemetry [candidate] (10.767 ms) : 0, 10767
ProfilingAgent [baseline] (94.298 ms) : 0, 94298
ProfilingAgent [candidate] (94.371 ms) : 0, 94371
Profiling [baseline] (94.321 ms) : 0, 94321
Profiling [candidate] (94.394 ms) : 0, 94394
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.43.0-SNAPSHOT~9bd24c3774, baseline=1.43.0-SNAPSHOT~8233fc5ff8
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.086 s) : 0, 1085895
Total [baseline] (8.606 s) : 0, 8605775
Agent [candidate] (1.086 s) : 0, 1085832
Total [candidate] (8.625 s) : 0, 8624982
section iast
Agent [baseline] (1.205 s) : 0, 1205396
Total [baseline] (9.145 s) : 0, 9144588
Agent [candidate] (1.21 s) : 0, 1209751
Total [candidate] (9.184 s) : 0, 9184316
section iast_HARDCODED_SECRET_DISABLED
Agent [baseline] (1.209 s) : 0, 1209075
Total [baseline] (9.136 s) : 0, 9135726
Agent [candidate] (1.215 s) : 0, 1214669
Total [candidate] (9.125 s) : 0, 9125392
section iast_TELEMETRY_OFF
Agent [baseline] (1.211 s) : 0, 1210601
Total [baseline] (9.142 s) : 0, 9142376
Agent [candidate] (1.212 s) : 0, 1211857
Total [candidate] (9.168 s) : 0, 9168363
gantt
title insecure-bank - break down per module: candidate=1.43.0-SNAPSHOT~9bd24c3774, baseline=1.43.0-SNAPSHOT~8233fc5ff8
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (690.733 ms) : 0, 690733
BytebuddyAgent [candidate] (690.524 ms) : 0, 690524
GlobalTracer [baseline] (316.949 ms) : 0, 316949
GlobalTracer [candidate] (316.849 ms) : 0, 316849
AppSec [baseline] (54.5 ms) : 0, 54500
AppSec [candidate] (54.707 ms) : 0, 54707
Remote Config [baseline] (667.71 µs) : 0, 668
Remote Config [candidate] (667.97 µs) : 0, 668
Telemetry [baseline] (9.254 ms) : 0, 9254
Telemetry [candidate] (9.31 ms) : 0, 9310
section iast
BytebuddyAgent [baseline] (801.769 ms) : 0, 801769
BytebuddyAgent [candidate] (804.588 ms) : 0, 804588
GlobalTracer [baseline] (304.187 ms) : 0, 304187
GlobalTracer [candidate] (304.989 ms) : 0, 304989
AppSec [baseline] (57.07 ms) : 0, 57070
AppSec [candidate] (57.586 ms) : 0, 57586
Remote Config [baseline] (632.094 µs) : 0, 632
Remote Config [candidate] (607.067 µs) : 0, 607
Telemetry [baseline] (7.5 ms) : 0, 7500
Telemetry [candidate] (7.482 ms) : 0, 7482
IAST [baseline] (20.525 ms) : 0, 20525
IAST [candidate] (20.674 ms) : 0, 20674
section iast_HARDCODED_SECRET_DISABLED
BytebuddyAgent [baseline] (804.199 ms) : 0, 804199
BytebuddyAgent [candidate] (808.059 ms) : 0, 808059
GlobalTracer [baseline] (304.6 ms) : 0, 304600
GlobalTracer [candidate] (306.253 ms) : 0, 306253
AppSec [baseline] (56.83 ms) : 0, 56830
AppSec [candidate] (57.552 ms) : 0, 57552
Remote Config [baseline] (622.837 µs) : 0, 623
Remote Config [candidate] (626.492 µs) : 0, 626
Telemetry [baseline] (7.511 ms) : 0, 7511
Telemetry [candidate] (7.523 ms) : 0, 7523
IAST [baseline] (21.506 ms) : 0, 21506
IAST [candidate] (20.799 ms) : 0, 20799
section iast_TELEMETRY_OFF
BytebuddyAgent [baseline] (804.254 ms) : 0, 804254
BytebuddyAgent [candidate] (805.569 ms) : 0, 805569
GlobalTracer [baseline] (306.047 ms) : 0, 306047
GlobalTracer [candidate] (305.963 ms) : 0, 305963
AppSec [baseline] (57.943 ms) : 0, 57943
AppSec [candidate] (57.276 ms) : 0, 57276
Remote Config [baseline] (641.134 µs) : 0, 641
Remote Config [candidate] (617.976 µs) : 0, 618
Telemetry [baseline] (7.461 ms) : 0, 7461
Telemetry [candidate] (7.461 ms) : 0, 7461
IAST [baseline] (20.441 ms) : 0, 20441
IAST [candidate] (21.142 ms) : 0, 21142
LoadParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 12 metrics, 16 unstable metrics. Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.43.0-SNAPSHOT~9bd24c3774, baseline=1.43.0-SNAPSHOT~8233fc5ff8
dateFormat X
axisFormat %s
section baseline
no_agent (365.523 µs) : 346, 385
. : milestone, 366,
iast (482.234 µs) : 461, 504
. : milestone, 482,
iast_FULL (641.638 µs) : 620, 663
. : milestone, 642,
iast_GLOBAL (517.771 µs) : 495, 540
. : milestone, 518,
iast_HARDCODED_SECRET_DISABLED (482.578 µs) : 461, 504
. : milestone, 483,
iast_INACTIVE (456.763 µs) : 436, 478
. : milestone, 457,
iast_TELEMETRY_OFF (470.624 µs) : 449, 492
. : milestone, 471,
tracing (438.082 µs) : 418, 458
. : milestone, 438,
section candidate
no_agent (367.092 µs) : 347, 388
. : milestone, 367,
iast (483.284 µs) : 462, 504
. : milestone, 483,
iast_FULL (639.971 µs) : 618, 662
. : milestone, 640,
iast_GLOBAL (506.591 µs) : 485, 528
. : milestone, 507,
iast_HARDCODED_SECRET_DISABLED (486.688 µs) : 466, 508
. : milestone, 487,
iast_INACTIVE (448.603 µs) : 427, 470
. : milestone, 449,
iast_TELEMETRY_OFF (468.698 µs) : 448, 490
. : milestone, 469,
tracing (449.133 µs) : 428, 470
. : milestone, 449,
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.43.0-SNAPSHOT~9bd24c3774, baseline=1.43.0-SNAPSHOT~8233fc5ff8
dateFormat X
axisFormat %s
section baseline
no_agent (1.341 ms) : 1322, 1361
. : milestone, 1341,
appsec (1.73 ms) : 1706, 1754
. : milestone, 1730,
appsec_no_iast (1.727 ms) : 1703, 1751
. : milestone, 1727,
iast (1.461 ms) : 1438, 1484
. : milestone, 1461,
profiling (1.478 ms) : 1455, 1500
. : milestone, 1478,
tracing (1.45 ms) : 1425, 1475
. : milestone, 1450,
section candidate
no_agent (1.334 ms) : 1315, 1352
. : milestone, 1334,
appsec (1.727 ms) : 1704, 1750
. : milestone, 1727,
appsec_no_iast (1.716 ms) : 1692, 1741
. : milestone, 1716,
iast (1.481 ms) : 1459, 1503
. : milestone, 1481,
profiling (1.514 ms) : 1490, 1538
. : milestone, 1514,
tracing (1.454 ms) : 1430, 1479
. : milestone, 1454,
DacapoParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 11 metrics, 1 unstable metrics. Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.43.0-SNAPSHOT~9bd24c3774, baseline=1.43.0-SNAPSHOT~8233fc5ff8
dateFormat X
axisFormat %s
section baseline
no_agent (1.462 ms) : 1451, 1474
. : milestone, 1462,
appsec (2.336 ms) : 2294, 2378
. : milestone, 2336,
iast (2.075 ms) : 2023, 2127
. : milestone, 2075,
iast_GLOBAL (2.119 ms) : 2067, 2172
. : milestone, 2119,
profiling (2.27 ms) : 2115, 2425
. : milestone, 2270,
tracing (1.919 ms) : 1880, 1959
. : milestone, 1919,
section candidate
no_agent (1.46 ms) : 1449, 1472
. : milestone, 1460,
appsec (2.333 ms) : 2291, 2374
. : milestone, 2333,
iast (2.068 ms) : 2016, 2120
. : milestone, 2068,
iast_GLOBAL (2.111 ms) : 2060, 2163
. : milestone, 2111,
profiling (1.942 ms) : 1900, 1984
. : milestone, 1942,
tracing (1.912 ms) : 1872, 1952
. : milestone, 1912,
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.43.0-SNAPSHOT~9bd24c3774, baseline=1.43.0-SNAPSHOT~8233fc5ff8
dateFormat X
axisFormat %s
section baseline
no_agent (15.058 s) : 15058000, 15058000
. : milestone, 15058000,
appsec (15.365 s) : 15365000, 15365000
. : milestone, 15365000,
iast (18.525 s) : 18525000, 18525000
. : milestone, 18525000,
iast_GLOBAL (18.162 s) : 18162000, 18162000
. : milestone, 18162000,
profiling (15.155 s) : 15155000, 15155000
. : milestone, 15155000,
tracing (15.518 s) : 15518000, 15518000
. : milestone, 15518000,
section candidate
no_agent (15.072 s) : 15072000, 15072000
. : milestone, 15072000,
appsec (15.073 s) : 15073000, 15073000
. : milestone, 15073000,
iast (18.425 s) : 18425000, 18425000
. : milestone, 18425000,
iast_GLOBAL (18.327 s) : 18327000, 18327000
. : milestone, 18327000,
profiling (15.052 s) : 15052000, 15052000
. : milestone, 15052000,
tracing (15.076 s) : 15076000, 15076000
. : milestone, 15076000,
|
dd-java-agent/agent-iast/src/main/java/com/datadog/iast/propagation/StringModuleImpl.java
Show resolved
Hide resolved
...entation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/StringCallSite.java
Outdated
Show resolved
Hide resolved
...entation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/StringCallSite.java
Outdated
Show resolved
Hide resolved
.../java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/StringCallSiteTest.groovy
Outdated
Show resolved
Hide resolved
dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/taint/RangesTest.groovy
Show resolved
Hide resolved
dd-java-agent/agent-iast/src/main/java/com/datadog/iast/taint/Ranges.java
Show resolved
Hide resolved
dd-java-agent/agent-iast/src/main/java/com/datadog/iast/util/StringUtils.java
Show resolved
Hide resolved
...entation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/StringCallSite.java
Outdated
Show resolved
Hide resolved
dd-java-agent/agent-iast/src/main/java/com/datadog/iast/util/StringUtils.java
Outdated
Show resolved
Hide resolved
Rolling out this feature like this really makes me feel nervous, I think we should:
Once we are more confident with the solution we could return it to the most optimal state without having to rely on the JDK. CC @smola |
-4 | " ==>123\r\n 1<==2==>3<==" | "==>123\n1<==2==>3<==" | ||
} | ||
|
||
void 'test replace with a single char and make sure IastRequestContext is called'() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the replacing methods did you have look to Regex.java and LiteralReplace.java, there are many cases including ones that produce out of memory errors that we should contemplate for this PR.
dd-java-agent/agent-iast/src/main/java/com/datadog/iast/util/StringUtils.java
Show resolved
Hide resolved
dd-java-agent/agent-iast/src/main/java/com/datadog/iast/taint/Ranges.java
Outdated
Show resolved
Hide resolved
dd-java-agent/agent-iast/src/main/java/com/datadog/iast/util/StringUtils.java
Outdated
Show resolved
Hide resolved
.../java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/StringCallSiteTest.groovy
Outdated
Show resolved
Hide resolved
...entation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/StringCallSite.java
Show resolved
Hide resolved
dd-java-agent/agent-iast/src/main/java/com/datadog/iast/util/RangeBuilder.java
Show resolved
Hide resolved
Hi! 👋 Thanks for your pull request! 🎉 To help us review it, please make sure to:
If you need help, please check our contributing guidelines. |
What Does This Do
This adds the instrumentation to propagate the taint values through the methods of
String
calledreplace
,replaceFirst
,replaceAll
.The approach is using a
CallSite.Around
to avoid iterating all the string to check the tainted values. This will add an overhead to the method as the algorithm is not as efficient as the original, but will increase the efficiency in as it will not iterate a second time through the string.Motivation
Increase propagation of
String
methodsAdditional Notes
Contributor Checklist
type:
and (comp:
orinst:
) labels in addition to any usefull labelsclose
,fix
or any linking keywords when referencing an issue.Use
solves
instead, and assign the PR milestone to the issueJira ticket: APPSEC-5763