Conversation
Contributor
|
Hi! 👋 Looks like you updated a Git Submodule.
|
jandro996
added
type: enhancement
…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
jandro996
force-pushed
the
alejandro.gonzalez/APPSEC-61875-file-upload-content
branch
from
0de9320 to
37e7d09
Compare
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 60 metrics, 11 unstable metrics. Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.62.0-SNAPSHOT~92ed45d8a8, baseline=1.62.0-SNAPSHOT~71f9713d93
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.071 s) : 0, 1070782
Total [baseline] (8.873 s) : 0, 8873480
Agent [candidate] (1.055 s) : 0, 1055142
Total [candidate] (8.824 s) : 0, 8824488
section iast
Agent [baseline] (1.229 s) : 0, 1228929
Total [baseline] (9.571 s) : 0, 9570763
Agent [candidate] (1.231 s) : 0, 1231252
Total [candidate] (9.558 s) : 0, 9557987
gantt
title insecure-bank - break down per module: candidate=1.62.0-SNAPSHOT~92ed45d8a8, baseline=1.62.0-SNAPSHOT~71f9713d93
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.266 ms) : 0, 1266
crashtracking [candidate] (1.222 ms) : 0, 1222
BytebuddyAgent [baseline] (641.403 ms) : 0, 641403
BytebuddyAgent [candidate] (632.84 ms) : 0, 632840
AgentMeter [baseline] (29.972 ms) : 0, 29972
AgentMeter [candidate] (29.516 ms) : 0, 29516
GlobalTracer [baseline] (251.715 ms) : 0, 251715
GlobalTracer [candidate] (248.303 ms) : 0, 248303
AppSec [baseline] (32.993 ms) : 0, 32993
AppSec [candidate] (32.511 ms) : 0, 32511
Debugger [baseline] (59.958 ms) : 0, 59958
Debugger [candidate] (58.837 ms) : 0, 58837
Remote Config [baseline] (600.293 µs) : 0, 600
Remote Config [candidate] (586.016 µs) : 0, 586
Telemetry [baseline] (8.192 ms) : 0, 8192
Telemetry [candidate] (8.697 ms) : 0, 8697
Flare Poller [baseline] (8.273 ms) : 0, 8273
Flare Poller [candidate] (6.538 ms) : 0, 6538
section iast
crashtracking [baseline] (1.244 ms) : 0, 1244
crashtracking [candidate] (1.218 ms) : 0, 1218
BytebuddyAgent [baseline] (807.737 ms) : 0, 807737
BytebuddyAgent [candidate] (811.217 ms) : 0, 811217
AgentMeter [baseline] (11.363 ms) : 0, 11363
AgentMeter [candidate] (11.473 ms) : 0, 11473
GlobalTracer [baseline] (238.669 ms) : 0, 238669
GlobalTracer [candidate] (237.875 ms) : 0, 237875
IAST [baseline] (28.322 ms) : 0, 28322
IAST [candidate] (29.099 ms) : 0, 29099
AppSec [baseline] (30.124 ms) : 0, 30124
AppSec [candidate] (29.098 ms) : 0, 29098
Debugger [baseline] (62.862 ms) : 0, 62862
Debugger [candidate] (63.624 ms) : 0, 63624
Remote Config [baseline] (536.169 µs) : 0, 536
Remote Config [candidate] (529.786 µs) : 0, 530
Telemetry [baseline] (7.751 ms) : 0, 7751
Telemetry [candidate] (7.662 ms) : 0, 7662
Flare Poller [baseline] (3.41 ms) : 0, 3410
Flare Poller [candidate] (3.448 ms) : 0, 3448
Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.62.0-SNAPSHOT~92ed45d8a8, baseline=1.62.0-SNAPSHOT~71f9713d93
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.062 s) : 0, 1061904
Total [baseline] (11.081 s) : 0, 11081376
Agent [candidate] (1.058 s) : 0, 1057889
Total [candidate] (10.986 s) : 0, 10986189
section appsec
Agent [baseline] (1.258 s) : 0, 1257511
Total [baseline] (10.968 s) : 0, 10968156
Agent [candidate] (1.262 s) : 0, 1261763
Total [candidate] (11.011 s) : 0, 11010659
section iast
Agent [baseline] (1.231 s) : 0, 1230679
Total [baseline] (11.292 s) : 0, 11291646
Agent [candidate] (1.231 s) : 0, 1231238
Total [candidate] (11.311 s) : 0, 11311242
section profiling
Agent [baseline] (1.197 s) : 0, 1197479
Total [baseline] (11.042 s) : 0, 11042041
Agent [candidate] (1.18 s) : 0, 1179907
Total [candidate] (10.923 s) : 0, 10923066
gantt
title petclinic - break down per module: candidate=1.62.0-SNAPSHOT~92ed45d8a8, baseline=1.62.0-SNAPSHOT~71f9713d93
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.225 ms) : 0, 1225
crashtracking [candidate] (1.234 ms) : 0, 1234
BytebuddyAgent [baseline] (634.441 ms) : 0, 634441
BytebuddyAgent [candidate] (632.475 ms) : 0, 632475
AgentMeter [baseline] (29.567 ms) : 0, 29567
AgentMeter [candidate] (29.464 ms) : 0, 29464
GlobalTracer [baseline] (249.21 ms) : 0, 249210
GlobalTracer [candidate] (248.654 ms) : 0, 248654
AppSec [baseline] (32.488 ms) : 0, 32488
AppSec [candidate] (32.528 ms) : 0, 32528
Debugger [baseline] (60.173 ms) : 0, 60173
Debugger [candidate] (59.751 ms) : 0, 59751
Remote Config [baseline] (621.778 µs) : 0, 622
Remote Config [candidate] (591.234 µs) : 0, 591
Telemetry [baseline] (8.064 ms) : 0, 8064
Telemetry [candidate] (7.977 ms) : 0, 7977
Flare Poller [baseline] (9.904 ms) : 0, 9904
Flare Poller [candidate] (8.923 ms) : 0, 8923
section appsec
crashtracking [baseline] (1.217 ms) : 0, 1217
crashtracking [candidate] (1.238 ms) : 0, 1238
BytebuddyAgent [baseline] (672.452 ms) : 0, 672452
BytebuddyAgent [candidate] (675.223 ms) : 0, 675223
AgentMeter [baseline] (12.083 ms) : 0, 12083
AgentMeter [candidate] (12.16 ms) : 0, 12160
GlobalTracer [baseline] (248.556 ms) : 0, 248556
GlobalTracer [candidate] (247.834 ms) : 0, 247834
IAST [baseline] (24.223 ms) : 0, 24223
IAST [candidate] (24.16 ms) : 0, 24160
AppSec [baseline] (186.433 ms) : 0, 186433
AppSec [candidate] (186.504 ms) : 0, 186504
Debugger [baseline] (64.538 ms) : 0, 64538
Debugger [candidate] (66.178 ms) : 0, 66178
Remote Config [baseline] (571.4 µs) : 0, 571
Remote Config [candidate] (591.261 µs) : 0, 591
Telemetry [baseline] (7.737 ms) : 0, 7737
Telemetry [candidate] (7.874 ms) : 0, 7874
Flare Poller [baseline] (3.425 ms) : 0, 3425
Flare Poller [candidate] (3.484 ms) : 0, 3484
section iast
crashtracking [baseline] (1.212 ms) : 0, 1212
crashtracking [candidate] (1.218 ms) : 0, 1218
BytebuddyAgent [baseline] (807.331 ms) : 0, 807331
BytebuddyAgent [candidate] (808.904 ms) : 0, 808904
AgentMeter [baseline] (11.379 ms) : 0, 11379
AgentMeter [candidate] (11.4 ms) : 0, 11400
GlobalTracer [baseline] (238.377 ms) : 0, 238377
GlobalTracer [candidate] (238.839 ms) : 0, 238839
IAST [baseline] (27.621 ms) : 0, 27621
IAST [candidate] (29.029 ms) : 0, 29029
AppSec [baseline] (28.992 ms) : 0, 28992
AppSec [candidate] (30.206 ms) : 0, 30206
Debugger [baseline] (67.736 ms) : 0, 67736
Debugger [candidate] (63.959 ms) : 0, 63959
Remote Config [baseline] (557.55 µs) : 0, 558
Remote Config [candidate] (527.943 µs) : 0, 528
Telemetry [baseline] (7.892 ms) : 0, 7892
Telemetry [candidate] (7.756 ms) : 0, 7756
Flare Poller [baseline] (3.498 ms) : 0, 3498
Flare Poller [candidate] (3.409 ms) : 0, 3409
section profiling
crashtracking [baseline] (1.186 ms) : 0, 1186
crashtracking [candidate] (1.167 ms) : 0, 1167
BytebuddyAgent [baseline] (700.205 ms) : 0, 700205
BytebuddyAgent [candidate] (688.149 ms) : 0, 688149
AgentMeter [baseline] (9.221 ms) : 0, 9221
AgentMeter [candidate] (9.146 ms) : 0, 9146
GlobalTracer [baseline] (208.887 ms) : 0, 208887
GlobalTracer [candidate] (206.43 ms) : 0, 206430
AppSec [baseline] (33.143 ms) : 0, 33143
AppSec [candidate] (32.8 ms) : 0, 32800
Debugger [baseline] (66.381 ms) : 0, 66381
Debugger [candidate] (65.539 ms) : 0, 65539
Remote Config [baseline] (580.517 µs) : 0, 581
Remote Config [candidate] (573.533 µs) : 0, 574
Telemetry [baseline] (7.95 ms) : 0, 7950
Telemetry [candidate] (7.755 ms) : 0, 7755
Flare Poller [baseline] (3.565 ms) : 0, 3565
Flare Poller [candidate] (3.557 ms) : 0, 3557
ProfilingAgent [baseline] (94.587 ms) : 0, 94587
ProfilingAgent [candidate] (93.915 ms) : 0, 93915
Profiling [baseline] (95.154 ms) : 0, 95154
Profiling [candidate] (94.475 ms) : 0, 94475
LoadParameters
See matching parameters
SummaryFound 0 performance improvements and 1 performance regressions! Performance is the same for 16 metrics, 19 unstable metrics.
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.62.0-SNAPSHOT~92ed45d8a8, baseline=1.62.0-SNAPSHOT~71f9713d93
dateFormat X
axisFormat %s
section baseline
no_agent (1.237 ms) : 1225, 1249
. : milestone, 1237,
iast (3.286 ms) : 3241, 3331
. : milestone, 3286,
iast_FULL (5.863 ms) : 5804, 5921
. : milestone, 5863,
iast_GLOBAL (3.732 ms) : 3669, 3795
. : milestone, 3732,
profiling (2.095 ms) : 2076, 2115
. : milestone, 2095,
tracing (1.855 ms) : 1839, 1870
. : milestone, 1855,
section candidate
no_agent (1.242 ms) : 1230, 1254
. : milestone, 1242,
iast (3.42 ms) : 3379, 3461
. : milestone, 3420,
iast_FULL (6.042 ms) : 5980, 6104
. : milestone, 6042,
iast_GLOBAL (3.75 ms) : 3688, 3812
. : milestone, 3750,
profiling (2.291 ms) : 2270, 2313
. : milestone, 2291,
tracing (1.92 ms) : 1903, 1937
. : milestone, 1920,
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.62.0-SNAPSHOT~92ed45d8a8, baseline=1.62.0-SNAPSHOT~71f9713d93
dateFormat X
axisFormat %s
section baseline
no_agent (18.149 ms) : 17965, 18333
. : milestone, 18149,
appsec (18.776 ms) : 18585, 18966
. : milestone, 18776,
code_origins (17.719 ms) : 17547, 17891
. : milestone, 17719,
iast (17.842 ms) : 17666, 18019
. : milestone, 17842,
profiling (18.528 ms) : 18344, 18712
. : milestone, 18528,
tracing (18.033 ms) : 17851, 18215
. : milestone, 18033,
section candidate
no_agent (18.243 ms) : 18057, 18428
. : milestone, 18243,
appsec (18.7 ms) : 18514, 18886
. : milestone, 18700,
code_origins (17.603 ms) : 17432, 17773
. : milestone, 17603,
iast (17.706 ms) : 17530, 17882
. : milestone, 17706,
profiling (18.761 ms) : 18571, 18950
. : milestone, 18761,
tracing (17.843 ms) : 17668, 18018
. : milestone, 17843,
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.62.0-SNAPSHOT~92ed45d8a8, baseline=1.62.0-SNAPSHOT~71f9713d93
dateFormat X
axisFormat %s
section baseline
no_agent (1.481 ms) : 1470, 1493
. : milestone, 1481,
appsec (3.738 ms) : 3524, 3953
. : milestone, 3738,
iast (2.269 ms) : 2199, 2338
. : milestone, 2269,
iast_GLOBAL (2.321 ms) : 2251, 2391
. : milestone, 2321,
profiling (2.095 ms) : 2040, 2149
. : milestone, 2095,
tracing (2.068 ms) : 2015, 2121
. : milestone, 2068,
section candidate
no_agent (1.485 ms) : 1473, 1496
. : milestone, 1485,
appsec (3.795 ms) : 3574, 4017
. : milestone, 3795,
iast (2.267 ms) : 2198, 2337
. : milestone, 2267,
iast_GLOBAL (2.313 ms) : 2243, 2383
. : milestone, 2313,
profiling (2.092 ms) : 2038, 2147
. : milestone, 2092,
tracing (2.064 ms) : 2010, 2117
. : milestone, 2064,
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.62.0-SNAPSHOT~92ed45d8a8, baseline=1.62.0-SNAPSHOT~71f9713d93
dateFormat X
axisFormat %s
section baseline
no_agent (15.654 s) : 15654000, 15654000
. : milestone, 15654000,
appsec (14.84 s) : 14840000, 14840000
. : milestone, 14840000,
iast (18.383 s) : 18383000, 18383000
. : milestone, 18383000,
iast_GLOBAL (18.042 s) : 18042000, 18042000
. : milestone, 18042000,
profiling (15.455 s) : 15455000, 15455000
. : milestone, 15455000,
tracing (14.72 s) : 14720000, 14720000
. : milestone, 14720000,
section candidate
no_agent (14.945 s) : 14945000, 14945000
. : milestone, 14945000,
appsec (14.787 s) : 14787000, 14787000
. : milestone, 14787000,
iast (18.743 s) : 18743000, 18743000
. : milestone, 18743000,
iast_GLOBAL (18.42 s) : 18420000, 18420000
. : milestone, 18420000,
profiling (14.879 s) : 14879000, 14879000
. : milestone, 14879000,
tracing (15.079 s) : 15079000, 15079000
. : milestone, 15079000,
|
jandro996
changed the title
jandro996
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
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".
jandro996
force-pushed
the
alejandro.gonzalez/APPSEC-61875-file-upload-content
branch
from
baf17b2 to
304846b
Compare
jandro996
force-pushed
the
alejandro.gonzalez/APPSEC-61875-file-upload-content
branch
from
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.
jandro996
force-pushed
the
alejandro.gonzalez/APPSEC-61875-file-upload-content
branch
from
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.
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
2 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
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.