Add server.request.body.filenames AppSec address for commons-fileupload#10949
Open
Add server.request.body.filenames AppSec address for commons-fileupload#10949
Conversation
- Add REQUEST_FILES_FILENAMES_ID=30 event to Events.java with BiFunction<RequestContext, List<String>, Flow<Void>> callback type - Register case in InstrumentationGateway switch to wrap with try-catch - Wire GatewayBridge: conditional registration, handler, cache field, reset, and IGAppSecEventDependencies entry - Add unit tests in InstrumentationGatewayTest and GatewayBridgeSpecification tag: ai generated Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Instrument ServletFileUpload.parseRequest() to extract filenames from non-form-field FileItems and fire the requestFilesFilenames() IG event. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Certifies that the commons-fileupload instrumentation fires server.request.body.filenames and the WAF can block on it end-to-end: - Add /upload endpoint using ServletFileUpload.parseRequest() (mirrors client's fileupload.jsp pattern) - Disable Spring multipart auto-config so Commons FileUpload handles the request before Spring intercepts it - Add commons-fileupload:1.5 dependency to the smoke test app - Add __test_file_upload_block WAF rule matching .jsp/.php/.asp/.aspx filenames and block request based on malicious file upload filename test Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Spring's MultipartAutoConfiguration was activating despite spring.servlet.multipart.enabled=false in application.properties, causing StandardServletMultipartResolver to consume the request InputStream before Commons FileUpload could read it. Explicitly exclude MultipartAutoConfiguration via @SpringBootApplication so the raw InputStream is available to ServletFileUpload.parseRequest(). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- InstrumentationGateway.java: restore alphabetical import order (REQUEST_FILES_FILENAMES_ID belongs after REQUEST_ENDED_ID) - CommonsFileUploadAppSecModule.java: use NameMatchers.named instead of ElementMatchers.named, consistent with adjacent IAST instrumentation Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
jandro996
added
type: enhancement
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 62 metrics, 9 unstable metrics. Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.61.0-SNAPSHOT~f186acc614, baseline=1.61.0-SNAPSHOT~68aa369a4f
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.063 s) : 0, 1063405
Total [baseline] (11.032 s) : 0, 11031542
Agent [candidate] (1.072 s) : 0, 1072285
Total [candidate] (11.16 s) : 0, 11160347
section appsec
Agent [baseline] (1.245 s) : 0, 1245063
Total [baseline] (11.213 s) : 0, 11213445
Agent [candidate] (1.249 s) : 0, 1248688
Total [candidate] (11.137 s) : 0, 11136683
section iast
Agent [baseline] (1.23 s) : 0, 1229808
Total [baseline] (11.323 s) : 0, 11322514
Agent [candidate] (1.241 s) : 0, 1240663
Total [candidate] (11.282 s) : 0, 11282072
section profiling
Agent [baseline] (1.183 s) : 0, 1183073
Total [baseline] (10.954 s) : 0, 10953856
Agent [candidate] (1.182 s) : 0, 1182478
Total [candidate] (10.954 s) : 0, 10953869
gantt
title petclinic - break down per module: candidate=1.61.0-SNAPSHOT~f186acc614, baseline=1.61.0-SNAPSHOT~68aa369a4f
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.195 ms) : 0, 1195
crashtracking [candidate] (1.211 ms) : 0, 1211
BytebuddyAgent [baseline] (631.681 ms) : 0, 631681
BytebuddyAgent [candidate] (638.0 ms) : 0, 638000
AgentMeter [baseline] (29.543 ms) : 0, 29543
AgentMeter [candidate] (29.823 ms) : 0, 29823
GlobalTracer [baseline] (257.796 ms) : 0, 257796
GlobalTracer [candidate] (259.748 ms) : 0, 259748
AppSec [baseline] (31.853 ms) : 0, 31853
AppSec [candidate] (32.261 ms) : 0, 32261
Debugger [baseline] (60.742 ms) : 0, 60742
Debugger [candidate] (61.066 ms) : 0, 61066
Remote Config [baseline] (590.859 µs) : 0, 591
Remote Config [candidate] (602.644 µs) : 0, 603
Telemetry [baseline] (8.084 ms) : 0, 8084
Telemetry [candidate] (8.845 ms) : 0, 8845
Flare Poller [baseline] (5.807 ms) : 0, 5807
Flare Poller [candidate] (4.441 ms) : 0, 4441
section appsec
crashtracking [baseline] (1.203 ms) : 0, 1203
crashtracking [candidate] (1.185 ms) : 0, 1185
BytebuddyAgent [baseline] (657.516 ms) : 0, 657516
BytebuddyAgent [candidate] (660.198 ms) : 0, 660198
AgentMeter [baseline] (12.079 ms) : 0, 12079
AgentMeter [candidate] (12.086 ms) : 0, 12086
GlobalTracer [baseline] (257.455 ms) : 0, 257455
GlobalTracer [candidate] (258.37 ms) : 0, 258370
AppSec [baseline] (177.832 ms) : 0, 177832
AppSec [candidate] (177.623 ms) : 0, 177623
Debugger [baseline] (66.122 ms) : 0, 66122
Debugger [candidate] (66.158 ms) : 0, 66158
Remote Config [baseline] (620.264 µs) : 0, 620
Remote Config [candidate] (624.38 µs) : 0, 624
Telemetry [baseline] (8.256 ms) : 0, 8256
Telemetry [candidate] (8.353 ms) : 0, 8353
Flare Poller [baseline] (3.546 ms) : 0, 3546
Flare Poller [candidate] (3.633 ms) : 0, 3633
IAST [baseline] (24.114 ms) : 0, 24114
IAST [candidate] (24.16 ms) : 0, 24160
section iast
crashtracking [baseline] (1.22 ms) : 0, 1220
crashtracking [candidate] (1.211 ms) : 0, 1211
BytebuddyAgent [baseline] (798.199 ms) : 0, 798199
BytebuddyAgent [candidate] (806.022 ms) : 0, 806022
AgentMeter [baseline] (11.451 ms) : 0, 11451
AgentMeter [candidate] (11.548 ms) : 0, 11548
GlobalTracer [baseline] (247.22 ms) : 0, 247220
GlobalTracer [candidate] (249.269 ms) : 0, 249269
AppSec [baseline] (26.433 ms) : 0, 26433
AppSec [candidate] (26.815 ms) : 0, 26815
Debugger [baseline] (69.86 ms) : 0, 69860
Debugger [candidate] (70.679 ms) : 0, 70679
Remote Config [baseline] (529.041 µs) : 0, 529
Remote Config [candidate] (524.158 µs) : 0, 524
Telemetry [baseline] (9.781 ms) : 0, 9781
Telemetry [candidate] (9.153 ms) : 0, 9153
Flare Poller [baseline] (3.565 ms) : 0, 3565
Flare Poller [candidate] (3.412 ms) : 0, 3412
IAST [baseline] (25.32 ms) : 0, 25320
IAST [candidate] (25.631 ms) : 0, 25631
section profiling
crashtracking [baseline] (1.174 ms) : 0, 1174
crashtracking [candidate] (1.159 ms) : 0, 1159
BytebuddyAgent [baseline] (683.011 ms) : 0, 683011
BytebuddyAgent [candidate] (682.83 ms) : 0, 682830
AgentMeter [baseline] (8.975 ms) : 0, 8975
AgentMeter [candidate] (8.958 ms) : 0, 8958
GlobalTracer [baseline] (215.572 ms) : 0, 215572
GlobalTracer [candidate] (215.308 ms) : 0, 215308
AppSec [baseline] (32.038 ms) : 0, 32038
AppSec [candidate] (32.109 ms) : 0, 32109
Debugger [baseline] (64.264 ms) : 0, 64264
Debugger [candidate] (64.785 ms) : 0, 64785
Remote Config [baseline] (563.621 µs) : 0, 564
Remote Config [candidate] (564.885 µs) : 0, 565
Telemetry [baseline] (9.306 ms) : 0, 9306
Telemetry [candidate] (8.46 ms) : 0, 8460
Flare Poller [baseline] (3.459 ms) : 0, 3459
Flare Poller [candidate] (3.464 ms) : 0, 3464
ProfilingAgent [baseline] (93.72 ms) : 0, 93720
ProfilingAgent [candidate] (93.924 ms) : 0, 93924
Profiling [baseline] (94.284 ms) : 0, 94284
Profiling [candidate] (94.475 ms) : 0, 94475
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.61.0-SNAPSHOT~f186acc614, baseline=1.61.0-SNAPSHOT~68aa369a4f
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.069 s) : 0, 1069350
Total [baseline] (8.856 s) : 0, 8855667
Agent [candidate] (1.056 s) : 0, 1055508
Total [candidate] (8.839 s) : 0, 8838953
section iast
Agent [baseline] (1.236 s) : 0, 1235845
Total [baseline] (9.558 s) : 0, 9558397
Agent [candidate] (1.236 s) : 0, 1236389
Total [candidate] (9.556 s) : 0, 9555677
gantt
title insecure-bank - break down per module: candidate=1.61.0-SNAPSHOT~f186acc614, baseline=1.61.0-SNAPSHOT~68aa369a4f
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.211 ms) : 0, 1211
crashtracking [candidate] (1.187 ms) : 0, 1187
BytebuddyAgent [baseline] (636.772 ms) : 0, 636772
BytebuddyAgent [candidate] (628.37 ms) : 0, 628370
AgentMeter [baseline] (29.845 ms) : 0, 29845
AgentMeter [candidate] (29.332 ms) : 0, 29332
GlobalTracer [baseline] (259.5 ms) : 0, 259500
GlobalTracer [candidate] (256.792 ms) : 0, 256792
AppSec [baseline] (32.238 ms) : 0, 32238
AppSec [candidate] (31.866 ms) : 0, 31866
Debugger [baseline] (60.388 ms) : 0, 60388
Debugger [candidate] (59.757 ms) : 0, 59757
Remote Config [baseline] (589.309 µs) : 0, 589
Remote Config [candidate] (597.219 µs) : 0, 597
Telemetry [baseline] (8.12 ms) : 0, 8120
Telemetry [candidate] (8.037 ms) : 0, 8037
Flare Poller [baseline] (4.346 ms) : 0, 4346
Flare Poller [candidate] (3.551 ms) : 0, 3551
section iast
crashtracking [baseline] (1.216 ms) : 0, 1216
crashtracking [candidate] (1.243 ms) : 0, 1243
BytebuddyAgent [baseline] (802.866 ms) : 0, 802866
BytebuddyAgent [candidate] (803.014 ms) : 0, 803014
AgentMeter [baseline] (11.632 ms) : 0, 11632
AgentMeter [candidate] (11.67 ms) : 0, 11670
GlobalTracer [baseline] (248.495 ms) : 0, 248495
GlobalTracer [candidate] (248.91 ms) : 0, 248910
AppSec [baseline] (26.786 ms) : 0, 26786
AppSec [candidate] (26.735 ms) : 0, 26735
Debugger [baseline] (68.526 ms) : 0, 68526
Debugger [candidate] (68.692 ms) : 0, 68692
Remote Config [baseline] (527.206 µs) : 0, 527
Remote Config [candidate] (523.668 µs) : 0, 524
Telemetry [baseline] (10.183 ms) : 0, 10183
Telemetry [candidate] (10.118 ms) : 0, 10118
Flare Poller [baseline] (3.61 ms) : 0, 3610
Flare Poller [candidate] (3.641 ms) : 0, 3641
IAST [baseline] (25.607 ms) : 0, 25607
IAST [candidate] (25.596 ms) : 0, 25596
LoadParameters
See matching parameters
SummaryFound 0 performance improvements and 3 performance regressions! Performance is the same for 17 metrics, 16 unstable metrics.
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.61.0-SNAPSHOT~f186acc614, baseline=1.61.0-SNAPSHOT~68aa369a4f
dateFormat X
axisFormat %s
section baseline
no_agent (1.174 ms) : 1162, 1186
. : milestone, 1174,
iast (3.223 ms) : 3180, 3267
. : milestone, 3223,
iast_FULL (5.959 ms) : 5898, 6020
. : milestone, 5959,
iast_GLOBAL (3.39 ms) : 3342, 3437
. : milestone, 3390,
profiling (2.1 ms) : 2082, 2119
. : milestone, 2100,
tracing (1.772 ms) : 1757, 1786
. : milestone, 1772,
section candidate
no_agent (1.188 ms) : 1176, 1200
. : milestone, 1188,
iast (3.299 ms) : 3255, 3343
. : milestone, 3299,
iast_FULL (5.887 ms) : 5828, 5947
. : milestone, 5887,
iast_GLOBAL (3.498 ms) : 3447, 3549
. : milestone, 3498,
profiling (2.067 ms) : 2048, 2086
. : milestone, 2067,
tracing (1.775 ms) : 1761, 1790
. : milestone, 1775,
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.61.0-SNAPSHOT~f186acc614, baseline=1.61.0-SNAPSHOT~68aa369a4f
dateFormat X
axisFormat %s
section baseline
no_agent (19.232 ms) : 19033, 19431
. : milestone, 19232,
appsec (18.918 ms) : 18721, 19114
. : milestone, 18918,
code_origins (17.851 ms) : 17673, 18029
. : milestone, 17851,
iast (18.023 ms) : 17842, 18205
. : milestone, 18023,
profiling (19.043 ms) : 18851, 19234
. : milestone, 19043,
tracing (17.663 ms) : 17486, 17839
. : milestone, 17663,
section candidate
no_agent (19.449 ms) : 19250, 19648
. : milestone, 19449,
appsec (18.85 ms) : 18663, 19038
. : milestone, 18850,
code_origins (17.806 ms) : 17629, 17982
. : milestone, 17806,
iast (19.152 ms) : 18959, 19345
. : milestone, 19152,
profiling (18.565 ms) : 18377, 18753
. : milestone, 18565,
tracing (17.89 ms) : 17713, 18068
. : milestone, 17890,
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 biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.61.0-SNAPSHOT~f186acc614, baseline=1.61.0-SNAPSHOT~68aa369a4f
dateFormat X
axisFormat %s
section baseline
no_agent (14.867 s) : 14867000, 14867000
. : milestone, 14867000,
appsec (14.5 s) : 14500000, 14500000
. : milestone, 14500000,
iast (18.175 s) : 18175000, 18175000
. : milestone, 18175000,
iast_GLOBAL (18.173 s) : 18173000, 18173000
. : milestone, 18173000,
profiling (15.292 s) : 15292000, 15292000
. : milestone, 15292000,
tracing (15.139 s) : 15139000, 15139000
. : milestone, 15139000,
section candidate
no_agent (15.462 s) : 15462000, 15462000
. : milestone, 15462000,
appsec (15.188 s) : 15188000, 15188000
. : milestone, 15188000,
iast (18.534 s) : 18534000, 18534000
. : milestone, 18534000,
iast_GLOBAL (18.068 s) : 18068000, 18068000
. : milestone, 18068000,
profiling (15.038 s) : 15038000, 15038000
. : milestone, 15038000,
tracing (14.785 s) : 14785000, 14785000
. : milestone, 14785000,
Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.61.0-SNAPSHOT~f186acc614, baseline=1.61.0-SNAPSHOT~68aa369a4f
dateFormat X
axisFormat %s
section baseline
no_agent (1.474 ms) : 1463, 1486
. : milestone, 1474,
appsec (3.816 ms) : 3594, 4038
. : milestone, 3816,
iast (2.265 ms) : 2195, 2334
. : milestone, 2265,
iast_GLOBAL (2.305 ms) : 2236, 2374
. : milestone, 2305,
profiling (2.112 ms) : 2056, 2168
. : milestone, 2112,
tracing (2.081 ms) : 2027, 2135
. : milestone, 2081,
section candidate
no_agent (1.482 ms) : 1470, 1493
. : milestone, 1482,
appsec (3.818 ms) : 3594, 4042
. : milestone, 3818,
iast (2.262 ms) : 2193, 2331
. : milestone, 2262,
iast_GLOBAL (2.3 ms) : 2231, 2369
. : milestone, 2300,
profiling (2.115 ms) : 2058, 2171
. : milestone, 2115,
tracing (2.075 ms) : 2022, 2128
. : milestone, 2075,
|
![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: f186acc614
ℹ️ 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".
Comment on lines
+205
to
+207
| if (additionalIGEvents.contains(EVENTS.requestFilesFilenames())) { | ||
| subscriptionService.registerCallback( | ||
| EVENTS.requestFilesFilenames(), this::onRequestFilesFilenames); |
There was a problem hiding this comment.
requestFilesFilenames is conditionally registered only when the address is present in additionalIGEvents during GatewayBridge.init(). In production, AppSec subscriptions are reloaded via reloadSubscriptions()/GatewayBridge.reset() without rerunning init(), so if server.request.body.filenames is added later (for example by a remote custom rule), this callback remains unregistered and CommonsFileUploadAppSecModule keeps seeing null from getCallback(...); filename rules will not execute until restart.
Useful? React with 👍 / 👎.
Member
Author
There was a problem hiding this comment.
Good point in general, but in practice this won't be an issue: server.request.body.filenames will be part of the default ruleset, so it will always be present in additionalIGEvents at startup and the callback will always be registered via init().
The scenario you describe (address added later via remote config without restart) is a pre-existing limitation that equally affects requestPathParams and requestBodyProcessed, which use the same conditional registration pattern. We followed that same pattern intentionally for consistency.
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
1 participant
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.filenamesIG address andrequestFilesFilenames()event in the gateway API, wired throughGatewayBridgeServletFileUpload.parseRequest(HttpServletRequest)(commons-fileupload ≥ 1.5) to extract uploaded file names and fire the WAF callback; blocks with aBlockingExceptiononRequestBlockingActionPOST /upload) and a test that verifies malicious file names (e.g.exploit.jsp) are blocked via a custom WAF rule matchingserver.request.body.filenamesMotivation
The customer's application uses Apache Commons FileUpload to handle multipart requests:
// fileupload.jsp (customer app)
ServletFileUpload upload = new ServletFileUpload(factory);
List items = upload.parseRequest(request);
for (FileItem item : items) { ... }
Uploading a file named exploit.jsp was not detected nor blocked because server.request.body.filenames was never populated — the IG event and GatewayBridge wiring did not exist yet.
This PR adds the event and instruments ServletFileUpload.parseRequest() as the first integration since it is the exact entry point the customer is using. Successive PRs will extend coverage to other multipart entry points (HttpServletRequest.getParts(), Spring MultipartFile, etc.).
Additional Notes
Contributor 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-61873-1]
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.