Skip to content

Fix partial path traversal in UI ResourceServlet - #202

Open
vharseko wants to merge 3 commits into
OpenIdentityPlatform:masterfrom
vharseko:fix-resource-servlet-path-traversal
Open

vharseko wants to merge 3 commits into
OpenIdentityPlatform:masterfrom
vharseko:fix-resource-servlet-path-traversal

Conversation

@vharseko

@vharseko vharseko commented Sep 18, 2026

Copy link
Copy Markdown
Member

Summary

Fixes the seven critical CodeQL Code scanning alerts, all in ResourceServlet, plus the medium and the note-level ones in the same file (second and third commits):

  • Binary version and Docker #2, Fix build #3, OpenID 4.5 #8, maybe it is not a bug,but I build failed. #9java/partial-path-traversal(-from-remote): the containment check compared getCanonicalPath() results as string prefixes, so a sibling directory sharing the configured directory's name prefix (e.g. ui/admin/default-old next to ui/admin/default) was reachable via /../default-old/.... Practical impact is limited (needs such a sibling to exist; Jetty 12 normalises .. before getPathInfo()), but the check itself was wrong.
  • Feature/buildable master 2017 #5, CodeMirror 404 error fix #6, update submodule #7java/ssrf: false positives — the URL was always a file: URL produced by File.toURI(). Removing the URL detour eliminates the alerts rather than dismissing them.
  • FIX build, add travis + docker #4java/unvalidated-url-redirection (second commit): the bare-context-root redirect (/admin/admin/) built its target from req.getServletPath(). It now uses the configured urlContextRoot — the same value for the Pax Web alias registration, but nothing taken from the request. In practice this was a false positive (Jetty 12 rejects // as an ambiguous path segment); the replacement is simply cleaner than a dismissal.
  • #437 — java/inefficient-empty-string-test (third commit): "".equals(target)target.isEmpty() in the same doGet guard.

Changes

  • ResourceServlet: new locate(dir, target) resolves the request against the configured directory with java.nio.file.Pathnormalize(), containment via Path.startsWith (component-wise, not string prefix), then toRealPath() on both sides so symlinks are resolved exactly as getCanonicalPath() did before. Invalid paths and files that vanish between the check and the resolve yield 404 instead of 500.
  • handleIndexHtml / handle / getLastModified / copyResource take a Path and use Files; File, URL, URLConnection are gone from the class. Content-Length is now set before the body is written (previously after, which was ignored once the response buffer had flushed).
  • New ResourceServletTest (TestNG + Mockito, module previously had no tests): drives the real doGet against a temp directory tree — normal lookups (default/extension precedence, index.html injection, missing extension dir, 304), rejections (directory, unknown file, prefix-sibling traversal for both dirs, climbing above the UI root), and symlink policy (link escaping the dir → 404; the dir itself being a symlink → served).
  • openidm-servlet/pom.xml: test-scoped testng, mockito-all, slf4j-simple.

Test plan

The containment check compared canonical paths as string prefixes, so a
sibling directory sharing the configured directory's name prefix (e.g.
ui/admin/default-old next to ui/admin/default) was reachable via "..".
Resolve requests with java.nio.file.Path and check containment on path
components, resolving symlinks on both sides. Serve files through
Files/Path instead of file: URLs.

Resolves CodeQL alerts #2, #3, OpenIdentityPlatform#8, OpenIdentityPlatform#9 (java/partial-path-traversal) and
OpenIdentityPlatform#5, OpenIdentityPlatform#6, OpenIdentityPlatform#7 (java/ssrf) in openidm-servlet.
@vharseko vharseko added security Security fix / CVE remediation java Pull requests that update Java code test Tests and test infrastructure (unit, e2e, smoke) bug Something isn't working labels Sep 18, 2026
…let path

The bare-context-root redirect built its target from req.getServletPath();
use the configured urlContextRoot, which is the same value for the Pax Web
alias registration but does not depend on anything in the request.

Resolves CodeQL alert OpenIdentityPlatform#4 (java/unvalidated-url-redirection).
Resolves CodeQL alert #437 (java/inefficient-empty-string-test).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working java Pull requests that update Java code security Security fix / CVE remediation test Tests and test infrastructure (unit, e2e, smoke)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant