Skip to content

Prevent path traversal in the asset serving REST resources (7.0) - #26987

Draft
graylog-internal-actions-access[bot] wants to merge 1 commit into
7.0from
backport-7.0/fix/asset-path-traversal
Draft

Prevent path traversal in the asset serving REST resources (7.0)#26987
graylog-internal-actions-access[bot] wants to merge 1 commit into
7.0from
backport-7.0/fix/asset-path-traversal

Conversation

@graylog-internal-actions-access

Copy link
Copy Markdown

Note: This is a backport of #26942 to 7.0.

Description

Validates the user-supplied filename in the asset serving REST resources before it reaches a classpath resource lookup.

ResourceFileReader.resolveResourceName resolves the filename against its base directory, normalizes it, and requires the result to be a direct child of that base. A direct child rather than mere containment below the base, because Path#normalize collapses leading .. on an absolute path — which makes a containment check meaningless for plugin assets, since those are packaged at the root of the plugin JAR and / contains every path.

That alone still lets a traversal land on an unrelated JAR's root-level resource (/../../log4j2.xml normalizes to the perfectly root-level /log4j2.xml), so plugin assets additionally have to originate from the code source of the plugin being asked for them.

PreflightAssetsResource duplicated ResourceFileReader almost verbatim — same Guava FileSystem cache, same file/jar switch. It now injects it instead, so the check lives in exactly one place for all three call sites. Neither resource reflects the requested file name back to the client anymore; it goes to a debug log.

Fixes https://github.com/Graylog2/graylog-plugin-enterprise/issues/15006.

Motivation and Context

PreflightAssetsResource and ResourceFileReader concatenated a user-supplied filename into getResource(...) with no validation. The JAX-RS path templates match against the encoded path segment, so a literal ../ never matches — but a percent-encoded one does, and Jersey decodes @PathParam values before injection.

Of the two, WebInterfaceAssetsResource matters more: it runs on the normal server and its asset routes are not permission-annotated. PreflightAssetsResource is only reachable before a node is configured and sits behind BasicAuthFilter.

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactoring (non-breaking change)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have requested a documentation update.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.

* Prevent path traversal in the asset serving REST resources

`PreflightAssetsResource` and `ResourceFileReader` concatenated a
user-supplied filename into a classpath resource lookup without validating
it. The JAX-RS path templates match against the encoded path segment, so a
literal `../` never matches, but a percent-encoded one does and is decoded
before it reaches the resource method.

Resource names are now resolved against their base directory, normalized,
and required to address a direct child of it. A direct child rather than
mere containment, because `Path#normalize` collapses leading `..` on an
absolute path, which makes containment meaningless for plugin assets: those
are packaged at the root of the plugin JAR, and `/` contains every path.

That alone still lets a traversal land on an unrelated JAR's root-level
resource, so plugin assets additionally have to originate from the code
source of the plugin that is being asked for them.

`PreflightAssetsResource` duplicated `ResourceFileReader` almost verbatim.
It now injects it instead, so the check lives in a single place. Neither
resource reflects the requested file name back to the client anymore; it
goes to a debug log.

See GHSA-7v9x-j5xj-57rq for details.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Add changelog snippet for #26942

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Improving changelog snippet.

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Tomas Dvorak <tomas.dvorak@graylog.com>
(cherry picked from commit 4654f70)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant