Conversation
|
Hi! 👋 Looks like you updated a Git Submodule.
|
…est.body.files_content Introduces a new AppSec WAF address `server.request.body.files_content` (`List<String>`) that exposes the content of each uploaded file in a multipart/form-data request. Entries correspond positionally to the existing `server.request.body.filenames` address. Content is capped at 4 096 bytes per file (ISO-8859-1) to keep memory usage bounded. Changes: - KnownAddresses: add REQUEST_FILES_CONTENT + forName() case - Events: add requestFilesContent event (ID 31); FILE_WRITTEN bumped to 32 - InstrumentationGateway: register the new BiFunction case - GatewayBridge: add onRequestFilesContent handler + DATA_DEPENDENCIES entry - CommonsFileUploadAppSecModule: after firing filenames, fire content (skipped when the filenames event already blocked the request) - Unit tests: GatewayBridgeSpecification, GatewayBridgeIGRegistrationSpecification, KnownAddressesSpecificationForkedTest - Smoke test: 'block request based on malicious file upload content' verifies end-to-end blocking via a custom WAF rule on the new address Closes APPSEC-61875
0de9320 to
37e7d09
Compare
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 63 metrics, 8 unstable metrics. Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.62.0-SNAPSHOT~63cdda064c, baseline=1.62.0-SNAPSHOT~71f9713d93
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.056 s) : 0, 1056467
Total [baseline] (8.812 s) : 0, 8811965
Agent [candidate] (1.055 s) : 0, 1055069
Total [candidate] (8.827 s) : 0, 8826835
section iast
Agent [baseline] (1.247 s) : 0, 1247204
Total [baseline] (9.559 s) : 0, 9559402
Agent [candidate] (1.233 s) : 0, 1233198
Total [candidate] (9.523 s) : 0, 9522936
gantt
title insecure-bank - break down per module: candidate=1.62.0-SNAPSHOT~63cdda064c, baseline=1.62.0-SNAPSHOT~71f9713d93
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.223 ms) : 0, 1223
crashtracking [candidate] (1.215 ms) : 0, 1215
BytebuddyAgent [baseline] (634.016 ms) : 0, 634016
BytebuddyAgent [candidate] (632.317 ms) : 0, 632317
AgentMeter [baseline] (29.52 ms) : 0, 29520
AgentMeter [candidate] (29.519 ms) : 0, 29519
GlobalTracer [baseline] (248.596 ms) : 0, 248596
GlobalTracer [candidate] (248.677 ms) : 0, 248677
AppSec [baseline] (32.369 ms) : 0, 32369
AppSec [candidate] (32.377 ms) : 0, 32377
Debugger [baseline] (59.133 ms) : 0, 59133
Debugger [candidate] (58.969 ms) : 0, 58969
Remote Config [baseline] (599.384 µs) : 0, 599
Remote Config [candidate] (594.057 µs) : 0, 594
Telemetry [baseline] (8.009 ms) : 0, 8009
Telemetry [candidate] (7.95 ms) : 0, 7950
Flare Poller [baseline] (6.78 ms) : 0, 6780
Flare Poller [candidate] (7.321 ms) : 0, 7321
section iast
crashtracking [baseline] (1.255 ms) : 0, 1255
crashtracking [candidate] (1.225 ms) : 0, 1225
BytebuddyAgent [baseline] (821.217 ms) : 0, 821217
BytebuddyAgent [candidate] (812.846 ms) : 0, 812846
AgentMeter [baseline] (11.63 ms) : 0, 11630
AgentMeter [candidate] (11.476 ms) : 0, 11476
GlobalTracer [baseline] (241.092 ms) : 0, 241092
GlobalTracer [candidate] (238.704 ms) : 0, 238704
IAST [baseline] (29.532 ms) : 0, 29532
IAST [candidate] (31.475 ms) : 0, 31475
AppSec [baseline] (29.849 ms) : 0, 29849
AppSec [candidate] (26.363 ms) : 0, 26363
Debugger [baseline] (64.421 ms) : 0, 64421
Debugger [candidate] (63.54 ms) : 0, 63540
Remote Config [baseline] (547.252 µs) : 0, 547
Remote Config [candidate] (521.194 µs) : 0, 521
Telemetry [baseline] (7.865 ms) : 0, 7865
Telemetry [candidate] (7.639 ms) : 0, 7639
Flare Poller [baseline] (3.427 ms) : 0, 3427
Flare Poller [candidate] (3.412 ms) : 0, 3412
Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.62.0-SNAPSHOT~63cdda064c, baseline=1.62.0-SNAPSHOT~71f9713d93
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.057 s) : 0, 1057093
Total [baseline] (11.11 s) : 0, 11110188
Agent [candidate] (1.056 s) : 0, 1055516
Total [candidate] (11.072 s) : 0, 11071883
section appsec
Agent [baseline] (1.27 s) : 0, 1270444
Total [baseline] (11.001 s) : 0, 11000925
Agent [candidate] (1.26 s) : 0, 1260349
Total [candidate] (11.031 s) : 0, 11030600
section iast
Agent [baseline] (1.24 s) : 0, 1240436
Total [baseline] (11.366 s) : 0, 11365621
Agent [candidate] (1.231 s) : 0, 1231433
Total [candidate] (11.295 s) : 0, 11295145
section profiling
Agent [baseline] (1.2 s) : 0, 1199872
Total [baseline] (11.13 s) : 0, 11130262
Agent [candidate] (1.19 s) : 0, 1190445
Total [candidate] (10.976 s) : 0, 10975855
gantt
title petclinic - break down per module: candidate=1.62.0-SNAPSHOT~63cdda064c, baseline=1.62.0-SNAPSHOT~71f9713d93
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.226 ms) : 0, 1226
crashtracking [candidate] (1.223 ms) : 0, 1223
BytebuddyAgent [baseline] (633.325 ms) : 0, 633325
BytebuddyAgent [candidate] (631.892 ms) : 0, 631892
AgentMeter [baseline] (29.568 ms) : 0, 29568
AgentMeter [candidate] (29.566 ms) : 0, 29566
GlobalTracer [baseline] (248.768 ms) : 0, 248768
GlobalTracer [candidate] (248.828 ms) : 0, 248828
AppSec [baseline] (32.279 ms) : 0, 32279
AppSec [candidate] (32.348 ms) : 0, 32348
Debugger [baseline] (59.759 ms) : 0, 59759
Debugger [candidate] (59.843 ms) : 0, 59843
Remote Config [baseline] (595.318 µs) : 0, 595
Remote Config [candidate] (587.71 µs) : 0, 588
Telemetry [baseline] (8.022 ms) : 0, 8022
Telemetry [candidate] (7.981 ms) : 0, 7981
Flare Poller [baseline] (7.457 ms) : 0, 7457
Flare Poller [candidate] (7.284 ms) : 0, 7284
section appsec
crashtracking [baseline] (1.223 ms) : 0, 1223
crashtracking [candidate] (1.233 ms) : 0, 1233
BytebuddyAgent [baseline] (676.101 ms) : 0, 676101
BytebuddyAgent [candidate] (673.252 ms) : 0, 673252
AgentMeter [baseline] (12.314 ms) : 0, 12314
AgentMeter [candidate] (12.158 ms) : 0, 12158
GlobalTracer [baseline] (251.804 ms) : 0, 251804
GlobalTracer [candidate] (248.629 ms) : 0, 248629
IAST [baseline] (24.882 ms) : 0, 24882
IAST [candidate] (24.263 ms) : 0, 24263
AppSec [baseline] (188.904 ms) : 0, 188904
AppSec [candidate] (185.578 ms) : 0, 185578
Debugger [baseline] (66.535 ms) : 0, 66535
Debugger [candidate] (66.954 ms) : 0, 66954
Remote Config [baseline] (591.37 µs) : 0, 591
Remote Config [candidate] (575.077 µs) : 0, 575
Telemetry [baseline] (8.049 ms) : 0, 8049
Telemetry [candidate] (7.934 ms) : 0, 7934
Flare Poller [baseline] (3.598 ms) : 0, 3598
Flare Poller [candidate] (3.452 ms) : 0, 3452
section iast
crashtracking [baseline] (1.242 ms) : 0, 1242
crashtracking [candidate] (1.221 ms) : 0, 1221
BytebuddyAgent [baseline] (815.988 ms) : 0, 815988
BytebuddyAgent [candidate] (808.333 ms) : 0, 808333
AgentMeter [baseline] (11.53 ms) : 0, 11530
AgentMeter [candidate] (11.411 ms) : 0, 11411
GlobalTracer [baseline] (240.471 ms) : 0, 240471
GlobalTracer [candidate] (239.081 ms) : 0, 239081
IAST [baseline] (31.87 ms) : 0, 31870
IAST [candidate] (30.017 ms) : 0, 30017
AppSec [baseline] (27.427 ms) : 0, 27427
AppSec [candidate] (27.852 ms) : 0, 27852
Debugger [baseline] (63.947 ms) : 0, 63947
Debugger [candidate] (65.774 ms) : 0, 65774
Remote Config [baseline] (535.041 µs) : 0, 535
Remote Config [candidate] (533.073 µs) : 0, 533
Telemetry [baseline] (7.773 ms) : 0, 7773
Telemetry [candidate] (7.797 ms) : 0, 7797
Flare Poller [baseline] (3.442 ms) : 0, 3442
Flare Poller [candidate] (3.38 ms) : 0, 3380
section profiling
crashtracking [baseline] (1.204 ms) : 0, 1204
crashtracking [candidate] (1.178 ms) : 0, 1178
BytebuddyAgent [baseline] (701.955 ms) : 0, 701955
BytebuddyAgent [candidate] (696.201 ms) : 0, 696201
AgentMeter [baseline] (9.323 ms) : 0, 9323
AgentMeter [candidate] (9.29 ms) : 0, 9290
GlobalTracer [baseline] (209.526 ms) : 0, 209526
GlobalTracer [candidate] (208.035 ms) : 0, 208035
AppSec [baseline] (33.058 ms) : 0, 33058
AppSec [candidate] (32.748 ms) : 0, 32748
Debugger [baseline] (66.205 ms) : 0, 66205
Debugger [candidate] (65.577 ms) : 0, 65577
Remote Config [baseline] (582.869 µs) : 0, 583
Remote Config [candidate] (578.681 µs) : 0, 579
Telemetry [baseline] (7.88 ms) : 0, 7880
Telemetry [candidate] (7.817 ms) : 0, 7817
Flare Poller [baseline] (3.554 ms) : 0, 3554
Flare Poller [candidate] (3.52 ms) : 0, 3520
ProfilingAgent [baseline] (94.438 ms) : 0, 94438
ProfilingAgent [candidate] (94.03 ms) : 0, 94030
Profiling [baseline] (95.025 ms) : 0, 95025
Profiling [candidate] (94.592 ms) : 0, 94592
LoadParameters
See matching parameters
SummaryFound 1 performance improvements and 3 performance regressions! Performance is the same for 16 metrics, 16 unstable metrics.
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.62.0-SNAPSHOT~63cdda064c, baseline=1.62.0-SNAPSHOT~71f9713d93
dateFormat X
axisFormat %s
section baseline
no_agent (1.238 ms) : 1226, 1250
. : milestone, 1238,
iast (3.487 ms) : 3436, 3539
. : milestone, 3487,
iast_FULL (6.011 ms) : 5950, 6071
. : milestone, 6011,
iast_GLOBAL (3.562 ms) : 3509, 3615
. : milestone, 3562,
profiling (2.198 ms) : 2179, 2218
. : milestone, 2198,
tracing (1.975 ms) : 1958, 1993
. : milestone, 1975,
section candidate
no_agent (1.253 ms) : 1241, 1265
. : milestone, 1253,
iast (3.332 ms) : 3282, 3382
. : milestone, 3332,
iast_FULL (6.123 ms) : 6060, 6186
. : milestone, 6123,
iast_GLOBAL (3.786 ms) : 3716, 3856
. : milestone, 3786,
profiling (2.176 ms) : 2156, 2196
. : milestone, 2176,
tracing (1.913 ms) : 1895, 1931
. : milestone, 1913,
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.62.0-SNAPSHOT~63cdda064c, baseline=1.62.0-SNAPSHOT~71f9713d93
dateFormat X
axisFormat %s
section baseline
no_agent (18.079 ms) : 17896, 18261
. : milestone, 18079,
appsec (18.851 ms) : 18659, 19044
. : milestone, 18851,
code_origins (18.018 ms) : 17841, 18195
. : milestone, 18018,
iast (18.72 ms) : 18533, 18906
. : milestone, 18720,
profiling (19.319 ms) : 19125, 19513
. : milestone, 19319,
tracing (18.469 ms) : 18287, 18651
. : milestone, 18469,
section candidate
no_agent (18.069 ms) : 17886, 18251
. : milestone, 18069,
appsec (19.19 ms) : 18996, 19384
. : milestone, 19190,
code_origins (19.574 ms) : 19382, 19766
. : milestone, 19574,
iast (18.085 ms) : 17903, 18267
. : milestone, 18085,
profiling (19.01 ms) : 18822, 19198
. : milestone, 19010,
tracing (17.999 ms) : 17823, 18175
. : milestone, 17999,
DacapoParameters
See matching parameters
SummaryFound 1 performance improvements and 0 performance regressions! Performance is the same for 11 metrics, 0 unstable metrics.
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.62.0-SNAPSHOT~63cdda064c, baseline=1.62.0-SNAPSHOT~71f9713d93
dateFormat X
axisFormat %s
section baseline
no_agent (15.567 s) : 15567000, 15567000
. : milestone, 15567000,
appsec (14.693 s) : 14693000, 14693000
. : milestone, 14693000,
iast (18.669 s) : 18669000, 18669000
. : milestone, 18669000,
iast_GLOBAL (17.807 s) : 17807000, 17807000
. : milestone, 17807000,
profiling (14.771 s) : 14771000, 14771000
. : milestone, 14771000,
tracing (14.854 s) : 14854000, 14854000
. : milestone, 14854000,
section candidate
no_agent (15.443 s) : 15443000, 15443000
. : milestone, 15443000,
appsec (15.014 s) : 15014000, 15014000
. : milestone, 15014000,
iast (18.668 s) : 18668000, 18668000
. : milestone, 18668000,
iast_GLOBAL (18.066 s) : 18066000, 18066000
. : milestone, 18066000,
profiling (15.493 s) : 15493000, 15493000
. : milestone, 15493000,
tracing (14.767 s) : 14767000, 14767000
. : milestone, 14767000,
Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.62.0-SNAPSHOT~63cdda064c, baseline=1.62.0-SNAPSHOT~71f9713d93
dateFormat X
axisFormat %s
section baseline
no_agent (1.488 ms) : 1476, 1499
. : milestone, 1488,
appsec (3.832 ms) : 3609, 4056
. : milestone, 3832,
iast (2.272 ms) : 2203, 2342
. : milestone, 2272,
iast_GLOBAL (2.33 ms) : 2259, 2401
. : milestone, 2330,
profiling (2.1 ms) : 2045, 2155
. : milestone, 2100,
tracing (2.077 ms) : 2023, 2130
. : milestone, 2077,
section candidate
no_agent (1.486 ms) : 1474, 1497
. : milestone, 1486,
appsec (2.548 ms) : 2493, 2603
. : milestone, 2548,
iast (2.271 ms) : 2201, 2341
. : milestone, 2271,
iast_GLOBAL (2.321 ms) : 2251, 2391
. : milestone, 2321,
profiling (2.101 ms) : 2046, 2156
. : milestone, 2101,
tracing (2.077 ms) : 2024, 2131
. : milestone, 2077,
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: baf17b2c8e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
baf17b2 to
304846b
Compare
a36ff44 to
22d70a0
Compare
The static readContent method in ParseRequestAdvice created a self-reference in the inlined advice bytecode (invokestatic on CommonsFileUploadAppSecModule$ParseRequestAdvice) that muzzle could not resolve in the application classloader, causing the instrumentation to be silently skipped. Moves readContent to a new FileItemContentReader helper class declared via helperClassNames(), which muzzle skips and the HelperInjector injects into the application classloader at runtime.
4610028 to
2076c7b
Compare
Without a bound, uploading N files would pass up to N × 4096 bytes to the WAF in a single call. MAX_FILES_TO_INSPECT = 25 limits total content to at most 100 KB, consistent with the per-file MAX_CONTENT_BYTES cap.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 92ed45d8a8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…strumentation - Rename CommonsFileUploadAppSecModule to CommonsFileUploadAppSecInstrumentation - Extract readContents() loop to FileItemContentReader for testability - Add unit tests for readContents() including MAX_FILES_TO_INSPECT cap - Parametrize readContent size boundary tests with where: block - Fix incorrect positional-alignment claim in KnownAddresses Javadoc - Fix FileItemContentReader Javadoc to describe purpose not deployment
What Does This Do
server.request.body.files_contentaddress andrequestFilesContent()event in the gateway API, wired throughGatewayBridgeandInstrumentationGatewayServletFileUpload.parseRequest()instrumentation (commons-fileupload) to read up to 4096 bytes of each uploaded file's content and fire the new WAF callback; blocks with aBlockingExceptiononRequestBlockingAction; content event is skipped when the filenames event has already blocked the requestAdditional Info
request.getParts(), Jetty, Liberty) will follow in successive PRsContributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issueJira ticket: APPSEC-61875
Note: Once your PR is ready to merge, add it to the merge queue by commenting
/merge./merge -ccancels the queue request./merge -f --reason "reason"skips all merge queue checks; please use this judiciously, as some checks do not run at the PR-level. For more information, see this doc.