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 org.json instrumentation (bad JSONObject advice) #6578

Merged
merged 1 commit into from
Jan 29, 2024

Conversation

smola
Copy link
Member

@smola smola commented Jan 29, 2024

What Does This Do

Instrumentation for JSONObject#get in IAST is not using an inlined advice. This seems to be causing a NoClassDefFoundError. This only happens when DD_IAST_ENABLED=true and has no impact in the application since it is caught by our mechanism for unhandled exceptions in instrumentation. It might have had some performance impact in services with DD_IAST_ENABLED=true and intensive use of JSONObject#get.

Motivation

Additional Notes

Jira ticket: APPSEC-51413

Instrumentation for `JSONObject#get` in IAST is not using an inlined
advice. This seems to be causing a NoClassDefFoundError. This only
happens when `DD_IAST_ENABLED=true` and has no impact in the application
since it is caught by our mechanism for unhandled exceptions in
instrumentation. It might have had some performance impact in
services with `DD_IAST_ENABLED=true` and intensive use of
`JSONObject#get`.
@smola smola added type: bug comp: asm iast Application Security Management (IAST) labels Jan 29, 2024
@smola smola requested review from a team, anderruiz and jandro996 January 29, 2024 11:54
@smola smola changed the title Fix JSONObject instrumentation advice Fix org.json instrumentation (bad JSONObject advice) Jan 29, 2024
@pr-commenter
Copy link

pr-commenter bot commented Jan 29, 2024

Benchmarks

Startup

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
git_branch master smola/APPSEC-51413-org-json-noclassdeffounderror
git_commit_date 1706524930 1706529068
git_commit_sha 7b4295b 06e1cec
release_version 1.29.0-SNAPSHOT~7b4295bdd5 1.29.0-SNAPSHOT~06e1cecc95
See matching parameters
Baseline Candidate
application insecure-bank insecure-bank
ci_job_date 1706532018 1706532018
ci_job_id 420457404 420457404
ci_pipeline_id 27312197 27312197
cpu_model Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz
module Agent Agent
parent None None
variant iast iast

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 44 metrics, 10 unstable metrics.

Startup time reports for petclinic
gantt
    title petclinic - global startup overhead: candidate=1.29.0-SNAPSHOT~06e1cecc95, baseline=1.29.0-SNAPSHOT~7b4295bdd5

    dateFormat X
    axisFormat %s
section tracing
Agent [baseline] (1.059 s) : 0, 1058812
Total [baseline] (9.356 s) : 0, 9356381
Agent [candidate] (1.067 s) : 0, 1067003
Total [candidate] (9.447 s) : 0, 9447305
section appsec
Agent [baseline] (1.165 s) : 0, 1164771
Total [baseline] (9.52 s) : 0, 9519518
Agent [candidate] (1.165 s) : 0, 1164896
Total [candidate] (9.577 s) : 0, 9577372
section iast
Agent [baseline] (1.184 s) : 0, 1183886
Total [baseline] (9.776 s) : 0, 9776428
Agent [candidate] (1.192 s) : 0, 1191676
Total [candidate] (9.756 s) : 0, 9756466
section profiling
Agent [baseline] (1.291 s) : 0, 1290864
Total [baseline] (9.613 s) : 0, 9612986
Agent [candidate] (1.299 s) : 0, 1299445
Total [candidate] (9.743 s) : 0, 9742999
Loading
  • baseline results
Module Variant Duration Δ tracing
Agent tracing 1.059 s -
Agent appsec 1.165 s 105.959 ms (10.0%)
Agent iast 1.184 s 125.074 ms (11.8%)
Agent profiling 1.291 s 232.052 ms (21.9%)
Total tracing 9.356 s -
Total appsec 9.52 s 163.137 ms (1.7%)
Total iast 9.776 s 420.047 ms (4.5%)
Total profiling 9.613 s 256.604 ms (2.7%)
  • candidate results
Module Variant Duration Δ tracing
Agent tracing 1.067 s -
Agent appsec 1.165 s 97.893 ms (9.2%)
Agent iast 1.192 s 124.673 ms (11.7%)
Agent profiling 1.299 s 232.441 ms (21.8%)
Total tracing 9.447 s -
Total appsec 9.577 s 130.067 ms (1.4%)
Total iast 9.756 s 309.161 ms (3.3%)
Total profiling 9.743 s 295.694 ms (3.1%)
gantt
    title petclinic - break down per module: candidate=1.29.0-SNAPSHOT~06e1cecc95, baseline=1.29.0-SNAPSHOT~7b4295bdd5

    dateFormat X
    axisFormat %s
section tracing
BytebuddyAgent [baseline] (667.708 ms) : 0, 667708
BytebuddyAgent [candidate] (672.836 ms) : 0, 672836
GlobalTracer [baseline] (296.726 ms) : 0, 296726
GlobalTracer [candidate] (299.455 ms) : 0, 299455
AppSec [baseline] (51.801 ms) : 0, 51801
AppSec [candidate] (51.886 ms) : 0, 51886
Remote Config [baseline] (699.011 µs) : 0, 699
Remote Config [candidate] (711.926 µs) : 0, 712
Telemetry [baseline] (7.564 ms) : 0, 7564
Telemetry [candidate] (7.573 ms) : 0, 7573
section appsec
BytebuddyAgent [baseline] (672.599 ms) : 0, 672599
BytebuddyAgent [candidate] (672.918 ms) : 0, 672918
GlobalTracer [baseline] (298.943 ms) : 0, 298943
GlobalTracer [candidate] (299.178 ms) : 0, 299178
AppSec [baseline] (151.14 ms) : 0, 151140
AppSec [candidate] (150.853 ms) : 0, 150853
Remote Config [baseline] (660.025 µs) : 0, 660
Remote Config [candidate] (657.227 µs) : 0, 657
Telemetry [baseline] (6.798 ms) : 0, 6798
Telemetry [candidate] (6.824 ms) : 0, 6824
section iast
BytebuddyAgent [baseline] (778.747 ms) : 0, 778747
BytebuddyAgent [candidate] (784.002 ms) : 0, 784002
GlobalTracer [baseline] (287.803 ms) : 0, 287803
GlobalTracer [candidate] (289.988 ms) : 0, 289988
AppSec [baseline] (53.872 ms) : 0, 53872
AppSec [candidate] (52.504 ms) : 0, 52504
Remote Config [baseline] (618.891 µs) : 0, 619
Remote Config [candidate] (623.584 µs) : 0, 624
Telemetry [baseline] (7.41 ms) : 0, 7410
Telemetry [candidate] (6.638 ms) : 0, 6638
IAST [baseline] (21.05 ms) : 0, 21050
IAST [candidate] (23.333 ms) : 0, 23333
section profiling
BytebuddyAgent [baseline] (669.206 ms) : 0, 669206
BytebuddyAgent [candidate] (672.321 ms) : 0, 672321
GlobalTracer [baseline] (380.901 ms) : 0, 380901
GlobalTracer [candidate] (384.073 ms) : 0, 384073
AppSec [baseline] (52.361 ms) : 0, 52361
AppSec [candidate] (52.895 ms) : 0, 52895
Remote Config [baseline] (663.711 µs) : 0, 664
Remote Config [candidate] (668.156 µs) : 0, 668
Telemetry [baseline] (7.433 ms) : 0, 7433
Telemetry [candidate] (7.58 ms) : 0, 7580
ProfilingAgent [baseline] (125.352 ms) : 0, 125352
ProfilingAgent [candidate] (126.827 ms) : 0, 126827
Profiling [baseline] (125.376 ms) : 0, 125376
Profiling [candidate] (126.852 ms) : 0, 126852
Loading

Load

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
end_time 2024-01-29T12:16:12 2024-01-29T12:35:14
git_branch master smola/APPSEC-51413-org-json-noclassdeffounderror
git_commit_date 1706524930 1706529068
git_commit_sha 7b4295b 06e1cec
release_version 1.29.0-SNAPSHOT~7b4295bdd5 1.29.0-SNAPSHOT~06e1cecc95
start_time 2024-01-29T12:15:59 2024-01-29T12:35:01
See matching parameters
Baseline Candidate
application insecure-bank insecure-bank
ci_job_date 1706532018 1706532018
ci_job_id 420457404 420457404
ci_pipeline_id 27312197 27312197
cpu_model Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz
variant iast iast

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 10 metrics, 16 unstable metrics.

Request duration reports for petclinic
gantt
    title petclinic - request duration [CI 0.99] : candidate=1.29.0-SNAPSHOT~06e1cecc95, baseline=1.29.0-SNAPSHOT~7b4295bdd5
    dateFormat X
    axisFormat %s
section baseline
no_agent (1.362 ms) : 1343, 1381
.   : milestone, 1362,
appsec (1.8 ms) : 1775, 1826
.   : milestone, 1800,
iast (1.538 ms) : 1514, 1563
.   : milestone, 1538,
profiling (1.545 ms) : 1518, 1571
.   : milestone, 1545,
tracing (1.504 ms) : 1479, 1529
.   : milestone, 1504,
section candidate
no_agent (1.344 ms) : 1325, 1363
.   : milestone, 1344,
appsec (1.782 ms) : 1757, 1808
.   : milestone, 1782,
iast (1.522 ms) : 1497, 1546
.   : milestone, 1522,
profiling (1.539 ms) : 1513, 1565
.   : milestone, 1539,
tracing (1.501 ms) : 1475, 1527
.   : milestone, 1501,
Loading
  • baseline results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 1.362 ms [1.343 ms, 1.381 ms] -
appsec 1.8 ms [1.775 ms, 1.826 ms] 438.404 µs (32.2%)
iast 1.538 ms [1.514 ms, 1.563 ms] 176.653 µs (13.0%)
profiling 1.545 ms [1.518 ms, 1.571 ms] 183.001 µs (13.4%)
tracing 1.504 ms [1.479 ms, 1.529 ms] 142.359 µs (10.5%)
  • candidate results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 1.344 ms [1.325 ms, 1.363 ms] -
appsec 1.782 ms [1.757 ms, 1.808 ms] 437.906 µs (32.6%)
iast 1.522 ms [1.497 ms, 1.546 ms] 177.37 µs (13.2%)
profiling 1.539 ms [1.513 ms, 1.565 ms] 194.703 µs (14.5%)
tracing 1.501 ms [1.475 ms, 1.527 ms] 156.494 µs (11.6%)
Request duration reports for insecure-bank
gantt
    title insecure-bank - request duration [CI 0.99] : candidate=1.29.0-SNAPSHOT~06e1cecc95, baseline=1.29.0-SNAPSHOT~7b4295bdd5
    dateFormat X
    axisFormat %s
section baseline
no_agent (364.716 µs) : 345, 384
.   : milestone, 365,
iast (479.269 µs) : 458, 500
.   : milestone, 479,
iast_FULL (543.884 µs) : 523, 564
.   : milestone, 544,
iast_GLOBAL (504.732 µs) : 483, 527
.   : milestone, 505,
iast_HARDCODED_SECRET_DISABLED (474.884 µs) : 454, 496
.   : milestone, 475,
iast_INACTIVE (444.447 µs) : 424, 465
.   : milestone, 444,
iast_TELEMETRY_OFF (472.828 µs) : 452, 494
.   : milestone, 473,
tracing (449.366 µs) : 428, 471
.   : milestone, 449,
section candidate
no_agent (370.895 µs) : 350, 391
.   : milestone, 371,
iast (483.734 µs) : 462, 505
.   : milestone, 484,
iast_FULL (540.686 µs) : 520, 561
.   : milestone, 541,
iast_GLOBAL (489.586 µs) : 469, 510
.   : milestone, 490,
iast_HARDCODED_SECRET_DISABLED (477.861 µs) : 457, 498
.   : milestone, 478,
iast_INACTIVE (451.608 µs) : 430, 473
.   : milestone, 452,
iast_TELEMETRY_OFF (472.642 µs) : 451, 494
.   : milestone, 473,
tracing (444.969 µs) : 424, 466
.   : milestone, 445,
Loading
  • baseline results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 364.716 µs [345.031 µs, 384.4 µs] -
iast 479.269 µs [458.186 µs, 500.351 µs] 114.553 µs (31.4%)
iast_FULL 543.884 µs [523.297 µs, 564.471 µs] 179.168 µs (49.1%)
iast_GLOBAL 504.732 µs [482.79 µs, 526.673 µs] 140.016 µs (38.4%)
iast_HARDCODED_SECRET_DISABLED 474.884 µs [454.127 µs, 495.642 µs] 110.169 µs (30.2%)
iast_INACTIVE 444.447 µs [423.769 µs, 465.124 µs] 79.731 µs (21.9%)
iast_TELEMETRY_OFF 472.828 µs [451.542 µs, 494.113 µs] 108.112 µs (29.6%)
tracing 449.366 µs [427.655 µs, 471.078 µs] 84.65 µs (23.2%)
  • candidate results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 370.895 µs [350.486 µs, 391.305 µs] -
iast 483.734 µs [462.133 µs, 505.336 µs] 112.839 µs (30.4%)
iast_FULL 540.686 µs [519.929 µs, 561.443 µs] 169.79 µs (45.8%)
iast_GLOBAL 489.586 µs [468.911 µs, 510.26 µs] 118.69 µs (32.0%)
iast_HARDCODED_SECRET_DISABLED 477.861 µs [457.382 µs, 498.34 µs] 106.965 µs (28.8%)
iast_INACTIVE 451.608 µs [430.262 µs, 472.954 µs] 80.713 µs (21.8%)
iast_TELEMETRY_OFF 472.642 µs [451.372 µs, 493.913 µs] 101.747 µs (27.4%)
tracing 444.969 µs [424.065 µs, 465.872 µs] 74.073 µs (20.0%)

@smola smola marked this pull request as ready for review January 29, 2024 13:35
@smola smola requested a review from a team as a code owner January 29, 2024 13:35
@smola smola merged commit 054d1ba into master Jan 29, 2024
82 checks passed
@smola smola deleted the smola/APPSEC-51413-org-json-noclassdeffounderror branch January 29, 2024 15:53
@github-actions github-actions bot added this to the 1.29.0 milestone Jan 29, 2024
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.

2 participants