Skip to content

Add server.request.body.filenames AppSec address for Vert.x 3.4, 4.0 and 5.0#11268

Draft
jandro996 wants to merge 1 commit intomasterfrom
alejandro.gonzalez/APPSEC-61873-vertx
Draft

Add server.request.body.filenames AppSec address for Vert.x 3.4, 4.0 and 5.0#11268
jandro996 wants to merge 1 commit intomasterfrom
alejandro.gonzalez/APPSEC-61873-vertx

Conversation

@jandro996
Copy link
Copy Markdown
Member

What Does This Do

Instruments RoutingContextImpl.fileUploads() in the Vert.x Web 3.4, 4.0 and 5.0 modules to fire the server.request.body.filenames AppSec IG event, enabling WAF-level detection and blocking on uploaded file names.

New callsites

  • vertx-web-3.4: new RoutingContextFilenamesAdvice (returns Set<FileUpload>), wired in RoutingContextImplInstrumentation
  • vertx-web-4.0: new RoutingContextFilenamesAdvice (returns Collection<FileUpload> to cover both Set in 4.0.x and List in 4.5.x+), wired in RoutingContextImplInstrumentation
  • vertx-web-5.0: new RoutingContextFilenamesAdvice (returns List<FileUpload>) + new RoutingContextImplInstrumentation

All three advice classes use FileUpload.class as the CallDepthThreadLocalMap key (public interface, avoids IllegalAccessError when ByteBuddy inlines the advice).

Test server fixes

All four VertxTestServer variants (3.4, 4.0/test, 4.0/latestDep, 5.0) had a broken BODY_MULTIPART handler that used setExpectMultipart(true) + an async endHandler, bypassing BodyHandler. This prevented fileUploads() from ever being populated. Replaced with:

  1. BodyHandler.create() registered as a prior handler on the same route
  2. A synchronous handler that calls ctx.fileUploads() before convertFormAttributes(ctx)

testBodyMultipart() continues to pass because BodyHandler also populates formAttributes().

NettyMultipartHelper fix

Vert.x's internal NettyFileUpload throws UnsupportedOperationException from getHttpDataType(). Added a try-catch to skip such items silently — they are handled by Vert.x's own instrumentation.

Motivation

solves APPSEC-61873 (partial — Vert.x coverage)

Additional Notes

Double-instrumentation in 5.0 tests: the vertx-web-5.0 test module includes vertx-web-4.0 as testImplementation, so both RoutingContextImplInstrumentation classes are applied in the same test run. Using Collection<FileUpload> in the 4.0 advice (instead of Set<FileUpload>) was necessary to avoid a ByteBuddy type-mismatch error when Vert.x 4.5.x+ changed the return type of fileUploads() from Set to List. Once the type error was gone, both advices coexist correctly via the shared CallDepthThreadLocalMap key.

JVM constraint: vertx-web-3.4 and vertx-web-4.0 default test set maxJavaVersion = VERSION_15; tests must be run with -PtestJvm=11.

Contributor Checklist

Jira ticket: APPSEC-61873

Note: Once your PR is ready to merge, add it to the merge queue by commenting /merge. /merge -c cancels 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.

…and 5.0

Instruments RoutingContextImpl.fileUploads() in vertx-web-3.4, vertx-web-4.0
and vertx-web-5.0 to fire EVENTS.requestFilesFilenames() with the list of
uploaded filenames, enabling WAF-level blocking on malicious file names.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant