[5.0.x] MYFACES-4761: Facelet path validation - #1044
Conversation
|
Im not sure If we should do this at this amount of detail. |
|
Hi, I removed some of the comments and simplified the implementation where possible. However, the fix ended up being more involved because there are several edge cases where a simpler approach would break existing behavior and tests. The main goal was to ensure that only valid Facelet resources within the application can be processed, while preventing access to other resources such as XML configuration files or arbitrary files. Simply blocking absolute paths is not sufficient, since behavior depends on how the resource is resolved, its location, and the URL scheme used by the container. On the surface this looks like a straightforward issue, but in practice there are a number of legitimate resource resolution scenarios that need to continue working. The implementation tries to strike a balance between addressing the security concern and preserving existing functionality. I will run TCK testing once more time, but this should be good unless you have any other concerns. |
44d1bea to
7cc65f7
Compare
Strengthen resolveURL() with four layered guards to prevent unsafe Facelet resource resolution: 1. Scheme check — rejects remote/network URI schemes (http, https, ftp, etc.) using a fast colon-index scan; OSGi/container schemes (jar, wsjar, file, zip) are intentionally allowed. 2. Traversal guard — for relative paths, verifies the resolved URL remains within the application base (WAR/EAR root) to prevent directory traversal attacks. 3. WEB-INF XML guard (scaffolded, currently disabled) — isWebInfConfigFile() is in place to block XML config descriptors under WEB-INF/ if .xml is ever added as a Facelet suffix. 4. Extension/suffix check — rejects paths whose extension is not in the configured Facelet suffix set (jakarta.faces.FACELETS_SUFFIX / jakarta.faces.FACELETS_VIEW_MAPPINGS), cached after the first call. Also adds FINE-level logging at each rejection point and a log of the computed allowed-suffix set on first initialisation. UnitTest project stage bypasses guards 2-4 to allow tests that use synthetic paths not backed by a real WAR layout. AI Assisted: Bob Version: 2.0.2
Add InvalidFileException (extends IOException) with a typed Reason enum to replace the generic MalformedURLException throws in resolveURL(). Reason values: DISALLOWED_SCHEME — blocked remote/network URI scheme PATH_TRAVERSAL — resolved URL escapes the application base INVALID_EXTENSION — extension not a configured Facelet suffix Callers can now catch InvalidFileException and inspect getReason() to programmatically distinguish between rejection causes without parsing exception messages. AI Assisted: Bob Version: 2.0.2
AI Assisted: Bob Version: 2.0.2
Fix up AI Assisted: Bob Version: 2.0.2
7cc65f7 to
9df06ed
Compare
|
Checks pass and unit tests pass. Merging. I'll work on other branches now. |
volosied
changed the title
3 participants
TODO -- Description will be updated later.