Skip to content
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

Fix IAST ranges for StringBuffer.toString #6050

Closed
wants to merge 1 commit into from

Conversation

smola
Copy link
Member

@smola smola commented Oct 16, 2023

What Does This Do

A StringBuffer or StringBuilder object might be tainted directly. In that case, we'll use an open Range (signaling that the full object is tainted, without a specific length). On StringBuffer#toString, this will lead to a String with an open Range, which the rest of our code base does not expect.

Motivation

This is required to be able to taint StringBuffers in IAST sources (but probably cannot be triggered with any released version).

Additional Notes

Jira ticket: APPSEC-11732

A StringBuffer or StringBuilder object might be tainted directly. In
that case, we'll use an open Range (signaling that the full object is
tainted, without a specific length). On StringBuffer#toString, this will
lead to a String with an open Range, which the rest of our code base
does not expect.
@smola smola added type: bug comp: asm iast Application Security Management (IAST) labels Oct 16, 2023
@smola smola requested a review from a team as a code owner October 16, 2023 15:07
@pr-commenter
Copy link

pr-commenter bot commented Oct 16, 2023

Benchmarks

Startup

Parameters

Baseline Candidate
commit 1.22.0-SNAPSHOT~f282e8c0d6 1.22.0-SNAPSHOT~f61db53b25
config baseline candidate
See matching parameters
Baseline Candidate
application insecure-bank insecure-bank
module Agent Agent
parent None None
variant iast iast

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 54 cases.

Startup time reports for petclinic
gantt
    title petclinic - global startup overhead: candidate=1.22.0-SNAPSHOT~f61db53b25, baseline=1.22.0-SNAPSHOT~f282e8c0d6

    dateFormat X
    axisFormat %s
section tracing
Agent [baseline] (1.016 s) : 0, 1016366
Total [baseline] (9.314 s) : 0, 9314057
Agent [candidate] (1.021 s) : 0, 1021216
Total [candidate] (9.263 s) : 0, 9262680
section appsec
Agent [baseline] (1.104 s) : 0, 1104494
Total [baseline] (9.291 s) : 0, 9290795
Agent [candidate] (1.103 s) : 0, 1102832
Total [candidate] (9.269 s) : 0, 9269467
section iast
Agent [baseline] (1.14 s) : 0, 1139768
Total [baseline] (9.424 s) : 0, 9424176
Agent [candidate] (1.142 s) : 0, 1142027
Total [candidate] (9.389 s) : 0, 9388799
section profiling
Agent [baseline] (1.196 s) : 0, 1196033
Total [baseline] (9.531 s) : 0, 9530669
Agent [candidate] (1.194 s) : 0, 1193679
Total [candidate] (9.441 s) : 0, 9440771
Loading
  • baseline results
Module Variant Duration Δ tracing
Agent tracing 1.016 s -
Agent appsec 1.104 s 88.128 ms (8.7%)
Agent iast 1.14 s 123.403 ms (12.1%)
Agent profiling 1.196 s 179.667 ms (17.7%)
Total tracing 9.314 s -
Total appsec 9.291 s -23.261 ms (-0.2%)
Total iast 9.424 s 110.119 ms (1.2%)
Total profiling 9.531 s 216.612 ms (2.3%)
  • candidate results
Module Variant Duration Δ tracing
Agent tracing 1.021 s -
Agent appsec 1.103 s 81.616 ms (8.0%)
Agent iast 1.142 s 120.811 ms (11.8%)
Agent profiling 1.194 s 172.464 ms (16.9%)
Total tracing 9.263 s -
Total appsec 9.269 s 6.787 ms (0.1%)
Total iast 9.389 s 126.12 ms (1.4%)
Total profiling 9.441 s 178.091 ms (1.9%)
gantt
    title petclinic - break down per module: candidate=1.22.0-SNAPSHOT~f61db53b25, baseline=1.22.0-SNAPSHOT~f282e8c0d6

    dateFormat X
    axisFormat %s
section tracing
BytebuddyAgent [baseline] (633.301 ms) : 0, 633301
BytebuddyAgent [candidate] (636.331 ms) : 0, 636331
GlobalTracer [baseline] (292.975 ms) : 0, 292975
GlobalTracer [candidate] (294.263 ms) : 0, 294263
AppSec [baseline] (49.022 ms) : 0, 49022
AppSec [candidate] (49.46 ms) : 0, 49460
Remote Config [baseline] (670.462 µs) : 0, 670
Remote Config [candidate] (656.854 µs) : 0, 657
Telemetry [baseline] (6.027 ms) : 0, 6027
Telemetry [candidate] (6.025 ms) : 0, 6025
section appsec
BytebuddyAgent [baseline] (634.052 ms) : 0, 634052
BytebuddyAgent [candidate] (633.547 ms) : 0, 633547
GlobalTracer [baseline] (291.864 ms) : 0, 291864
GlobalTracer [candidate] (291.508 ms) : 0, 291508
AppSec [baseline] (137.837 ms) : 0, 137837
AppSec [candidate] (137.185 ms) : 0, 137185
Remote Config [baseline] (638.596 µs) : 0, 639
Remote Config [candidate] (630.72 µs) : 0, 631
Telemetry [baseline] (5.676 ms) : 0, 5676
Telemetry [candidate] (5.671 ms) : 0, 5671
section iast
BytebuddyAgent [baseline] (761.669 ms) : 0, 761669
BytebuddyAgent [candidate] (761.861 ms) : 0, 761861
GlobalTracer [baseline] (273.045 ms) : 0, 273045
GlobalTracer [candidate] (272.52 ms) : 0, 272520
AppSec [baseline] (46.429 ms) : 0, 46429
AppSec [candidate] (46.027 ms) : 0, 46027
Remote Config [baseline] (570.151 µs) : 0, 570
Remote Config [candidate] (566.88 µs) : 0, 567
Telemetry [baseline] (7.04 ms) : 0, 7040
Telemetry [candidate] (8.334 ms) : 0, 8334
IAST [baseline] (16.779 ms) : 0, 16779
IAST [candidate] (18.292 ms) : 0, 18292
section profiling
BytebuddyAgent [baseline] (648.355 ms) : 0, 648355
BytebuddyAgent [candidate] (647.669 ms) : 0, 647669
GlobalTracer [baseline] (356.921 ms) : 0, 356921
GlobalTracer [candidate] (355.798 ms) : 0, 355798
AppSec [baseline] (49.423 ms) : 0, 49423
AppSec [candidate] (49.046 ms) : 0, 49046
Remote Config [baseline] (656.957 µs) : 0, 657
Remote Config [candidate] (654.731 µs) : 0, 655
Telemetry [baseline] (6.127 ms) : 0, 6127
Telemetry [candidate] (6.132 ms) : 0, 6132
ProfilingAgent [baseline] (80.918 ms) : 0, 80918
ProfilingAgent [candidate] (80.897 ms) : 0, 80897
Profiling [baseline] (80.942 ms) : 0, 80942
Profiling [candidate] (80.921 ms) : 0, 80921
Loading
Startup time reports for insecure-bank
gantt
    title insecure-bank - global startup overhead: candidate=1.22.0-SNAPSHOT~f61db53b25, baseline=1.22.0-SNAPSHOT~f282e8c0d6

    dateFormat X
    axisFormat %s
section tracing
Agent [baseline] (1.016 s) : 0, 1015616
Total [baseline] (8.678 s) : 0, 8677971
Agent [candidate] (1.014 s) : 0, 1013830
Total [candidate] (8.698 s) : 0, 8697541
section iast
Agent [baseline] (1.14 s) : 0, 1140292
Total [baseline] (9.214 s) : 0, 9213551
Agent [candidate] (1.14 s) : 0, 1140159
Total [candidate] (9.22 s) : 0, 9219560
section iast_TELEMETRY_OFF
Agent [baseline] (1.134 s) : 0, 1134064
Total [baseline] (9.189 s) : 0, 9189157
Agent [candidate] (1.133 s) : 0, 1132764
Total [candidate] (9.211 s) : 0, 9210710
Loading
  • baseline results
Module Variant Duration Δ tracing
Agent tracing 1.016 s -
Agent iast 1.14 s 124.676 ms (12.3%)
Agent iast_TELEMETRY_OFF 1.134 s 118.448 ms (11.7%)
Total tracing 8.678 s -
Total iast 9.214 s 535.58 ms (6.2%)
Total iast_TELEMETRY_OFF 9.189 s 511.187 ms (5.9%)
  • candidate results
Module Variant Duration Δ tracing
Agent tracing 1.014 s -
Agent iast 1.14 s 126.329 ms (12.5%)
Agent iast_TELEMETRY_OFF 1.133 s 118.935 ms (11.7%)
Total tracing 8.698 s -
Total iast 9.22 s 522.019 ms (6.0%)
Total iast_TELEMETRY_OFF 9.211 s 513.169 ms (5.9%)
gantt
    title insecure-bank - break down per module: candidate=1.22.0-SNAPSHOT~f61db53b25, baseline=1.22.0-SNAPSHOT~f282e8c0d6

    dateFormat X
    axisFormat %s
section tracing
BytebuddyAgent [baseline] (632.269 ms) : 0, 632269
BytebuddyAgent [candidate] (631.238 ms) : 0, 631238
GlobalTracer [baseline] (293.07 ms) : 0, 293070
GlobalTracer [candidate] (292.531 ms) : 0, 292531
AppSec [baseline] (49.252 ms) : 0, 49252
AppSec [candidate] (49.164 ms) : 0, 49164
Remote Config [baseline] (652.321 µs) : 0, 652
Remote Config [candidate] (651.898 µs) : 0, 652
Telemetry [baseline] (5.975 ms) : 0, 5975
Telemetry [candidate] (5.99 ms) : 0, 5990
section iast
BytebuddyAgent [baseline] (761.689 ms) : 0, 761689
BytebuddyAgent [candidate] (760.921 ms) : 0, 760921
GlobalTracer [baseline] (272.893 ms) : 0, 272893
GlobalTracer [candidate] (272.297 ms) : 0, 272297
AppSec [baseline] (46.356 ms) : 0, 46356
AppSec [candidate] (45.997 ms) : 0, 45997
Remote Config [baseline] (578.256 µs) : 0, 578
Remote Config [candidate] (573.5 µs) : 0, 574
Telemetry [baseline] (5.587 ms) : 0, 5587
Telemetry [candidate] (6.875 ms) : 0, 6875
IAST [baseline] (18.853 ms) : 0, 18853
IAST [candidate] (19.345 ms) : 0, 19345
section iast_TELEMETRY_OFF
BytebuddyAgent [baseline] (755.172 ms) : 0, 755172
BytebuddyAgent [candidate] (754.774 ms) : 0, 754774
GlobalTracer [baseline] (273.167 ms) : 0, 273167
GlobalTracer [candidate] (272.423 ms) : 0, 272423
AppSec [baseline] (46.376 ms) : 0, 46376
AppSec [candidate] (46.112 ms) : 0, 46112
Remote Config [baseline] (575.765 µs) : 0, 576
Remote Config [candidate] (562.891 µs) : 0, 563
Telemetry [baseline] (6.827 ms) : 0, 6827
Telemetry [candidate] (6.187 ms) : 0, 6187
IAST [baseline] (17.597 ms) : 0, 17597
IAST [candidate] (18.447 ms) : 0, 18447
Loading

Load

Parameters

Baseline Candidate
commit 1.22.0-SNAPSHOT~f282e8c0d6 1.22.0-SNAPSHOT~f61db53b25
config baseline candidate
end_time 2023-10-16T15:29:42 2023-10-16T15:45:55
start_time 2023-10-16T15:29:29 2023-10-16T15:45:42
See matching parameters
Baseline Candidate
application insecure-bank insecure-bank
variant iast iast

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 22 cases.

Request duration reports for petclinic
gantt
    title petclinic - request duration [CI 0.99] : candidate=1.22.0-SNAPSHOT~f61db53b25, baseline=1.22.0-SNAPSHOT~f282e8c0d6
    dateFormat X
    axisFormat %s
section baseline
no_agent (1.322 ms) : 1303, 1341
.   : milestone, 1322,
appsec (1.684 ms) : 1660, 1708
.   : milestone, 1684,
iast (1.466 ms) : 1441, 1490
.   : milestone, 1466,
profiling (1.455 ms) : 1430, 1480
.   : milestone, 1455,
tracing (1.463 ms) : 1438, 1488
.   : milestone, 1463,
section candidate
no_agent (1.329 ms) : 1310, 1348
.   : milestone, 1329,
appsec (1.704 ms) : 1679, 1728
.   : milestone, 1704,
iast (1.449 ms) : 1425, 1474
.   : milestone, 1449,
profiling (1.462 ms) : 1437, 1486
.   : milestone, 1462,
tracing (1.445 ms) : 1421, 1469
.   : milestone, 1445,
Loading
  • baseline results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 1.322 ms [1.303 ms, 1.341 ms] -
appsec 1.684 ms [1.66 ms, 1.708 ms] 362.226 µs (27.4%)
iast 1.466 ms [1.441 ms, 1.49 ms] 143.961 µs (10.9%)
profiling 1.455 ms [1.43 ms, 1.48 ms] 133.525 µs (10.1%)
tracing 1.463 ms [1.438 ms, 1.488 ms] 141.148 µs (10.7%)
  • candidate results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 1.329 ms [1.31 ms, 1.348 ms] -
appsec 1.704 ms [1.679 ms, 1.728 ms] 374.796 µs (28.2%)
iast 1.449 ms [1.425 ms, 1.474 ms] 120.336 µs (9.1%)
profiling 1.462 ms [1.437 ms, 1.486 ms] 132.918 µs (10.0%)
tracing 1.445 ms [1.421 ms, 1.469 ms] 116.082 µs (8.7%)
Request duration reports for insecure-bank
gantt
    title insecure-bank - request duration [CI 0.99] : candidate=1.22.0-SNAPSHOT~f61db53b25, baseline=1.22.0-SNAPSHOT~f282e8c0d6
    dateFormat X
    axisFormat %s
section baseline
no_agent (357.203 µs) : 337, 378
.   : milestone, 357,
iast (451.873 µs) : 431, 472
.   : milestone, 452,
iast_FULL (521.542 µs) : 501, 542
.   : milestone, 522,
iast_INACTIVE (426.441 µs) : 405, 447
.   : milestone, 426,
iast_TELEMETRY_OFF (454.921 µs) : 434, 476
.   : milestone, 455,
tracing (423.407 µs) : 403, 444
.   : milestone, 423,
section candidate
no_agent (365.393 µs) : 345, 386
.   : milestone, 365,
iast (457.577 µs) : 437, 478
.   : milestone, 458,
iast_FULL (524.382 µs) : 504, 545
.   : milestone, 524,
iast_INACTIVE (428.264 µs) : 408, 449
.   : milestone, 428,
iast_TELEMETRY_OFF (456.526 µs) : 436, 478
.   : milestone, 457,
tracing (434.632 µs) : 413, 456
.   : milestone, 435,
Loading
  • baseline results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 357.203 µs [336.871 µs, 377.535 µs] -
iast 451.873 µs [431.402 µs, 472.345 µs] 94.67 µs (26.5%)
iast_FULL 521.542 µs [501.228 µs, 541.856 µs] 164.339 µs (46.0%)
iast_INACTIVE 426.441 µs [405.456 µs, 447.426 µs] 69.238 µs (19.4%)
iast_TELEMETRY_OFF 454.921 µs [433.633 µs, 476.209 µs] 97.718 µs (27.4%)
tracing 423.407 µs [402.893 µs, 443.92 µs] 66.203 µs (18.5%)
  • candidate results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 365.393 µs [344.8 µs, 385.985 µs] -
iast 457.577 µs [436.806 µs, 478.348 µs] 92.184 µs (25.2%)
iast_FULL 524.382 µs [504.164 µs, 544.6 µs] 158.989 µs (43.5%)
iast_INACTIVE 428.264 µs [407.686 µs, 448.843 µs] 62.872 µs (17.2%)
iast_TELEMETRY_OFF 456.526 µs [435.525 µs, 477.526 µs] 91.133 µs (24.9%)
tracing 434.632 µs [413.158 µs, 456.107 µs] 69.24 µs (18.9%)

Copy link
Member

@manuel-alvarez-alvarez manuel-alvarez-alvarez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We coud add support for StringBuilder StringBuffer without bounds in the new refactor of the sources (treat them as CharSequence):
#6033

@smola
Copy link
Member Author

smola commented Oct 17, 2023

@manuel-alvarez-alvarez Indeed. In fact, I think I'd prefer to not merge this PR, and hold #6031, and instead go for tainting StringBuilder/StringBuffer as CharSequence, with proper bound start/length ranges. Then StringBuffer#toString() would not need to special-case this.

@smola
Copy link
Member Author

smola commented Oct 19, 2023

Not a proper fix. This will be superseded by handling of CharSequence objects in sources.

@smola smola closed this Oct 19, 2023
@smola smola deleted the smola/fix-stringbuffer-tostring-range branch October 19, 2023 12:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp: asm iast Application Security Management (IAST) type: bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants