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 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,
|
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.
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
Member
Author
Not sure if make sense to make it configurable, what do you think? |
Member
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
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.