[
https://issues.apache.org/jira/browse/WW-5839?focusedWorklogId=1053198&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1053198
]
ASF GitHub Bot logged work on WW-5839:
--------------------------------------
Author: ASF GitHub Bot
Created on: 07/Nov/26 09:02
Start Date: 07/Nov/26 09:02
Worklog Time Spent: 10m
Work Description: lukaszlenart commented on PR #2115:
URL: https://github.com/apache/struts/pull/2115#issuecomment-6704807561
Thanks for this. The three changes are unrelated, touch different modules
and will need separate review rounds and separate release notes, so please
split this PR into three, each with its own Jira ticket:
1. **Wildcard substitution into `class=`**: keep it on WW-5839, and narrow
that ticket's summary and description to this item.
2. **Path-stripped original upload file name**: use
[WW-5840](https://issues.apache.org/jira/browse/WW-5840), which already
describes this change. Please title the PR `WW-5840 ...`.
3. **Struts Boot session tracking (cookie-only tracking, `SameSite`)**:
please open a new ticket for it (component Boot, if available).
Review comments will follow on each PR separately. Thanks!
Issue Time Tracking
-------------------
Worklog Id: (was: 1053198)
Time Spent: 20m (was: 10m)
> Harden three input-handling gaps: wildcard class-name substitution, raw
> upload file names, Boot session tracking
> ----------------------------------------------------------------------------------------------------------------
>
> Key: WW-5839
> URL: https://issues.apache.org/jira/browse/WW-5839
> Project: Struts 2
> Issue Type: Improvement
> Components: Core
> Affects Versions: 6.12.0, 7.0.0, 7.3.0, 7.4.0
> Reporter: Gouri Sankar A
> Priority: Minor
> Fix For: 7.5.0
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Three small, independent hardening items found in the same review pass. None
> is a documented Struts control that fails to hold (SECURITY.md's "no control
> claimed" case), so filing as public hardening rather than privately.
> 1) AbstractMatcher/ActionConfigMatcher (core): a wildcard "\{n}" substituted
> into an action/result/exception-mapping class= attribute was unconfined --
> any matched path segment, including one containing "." or "/", landed
> verbatim in the resulting class name. The sibling method-name slot is already
> gated (ActionConfig#isAllowedMethod); there was no equivalent gate for the
> class-name slot. Fix rejects a substituted value outside [A-Za-z0-9_$]* when
> filling a class-name slot, instead of using it verbatim. Reachable only if a
> developer writes a wildcard mapping that substitutes into class= --
> application-responsibility, not a default-config issue.
> 2) UploadedFile/ActionFileUploadInterceptor (core): the client-supplied
> original file name was handed to actions unsanitised via getOriginalName(), a
> regression from the pre-7.0
> MultiPartRequest#getFileNames()/getCanonicalName() path-stripping pair, which
> has no caller in the current action-based upload flow. Struts itself never
> builds a filesystem path from the raw name (the on-disk temp file is
> UUID-named via getName()); this only matters if application code does. Fix
> adds a default method UploadedFile#getSanitizedOriginalName() (basename only,
> via FilenameUtils.getName) alongside the existing raw accessor, with javadoc
> on both making the raw/sanitised distinction explicit.
> 3) StrutsBootServer (boot, 7.5.0, unreleased -- no production deployments
> yet): without a web.xml, the embedded Tomcat context defaulted to the
> servlet-spec COOKIE+URL session tracking modes, so a session id supplied only
> via ";jsessionid=" in the URL (no cookie at all) was accepted and kept alive
> with no fresh Set-Cookie -- a session-fixation vector. Live-reproduced on a
> throwaway Boot app before the fix: a session established by cookie was
> resumed by replaying the id via the URL parameter with no Cookie header sent
> at all. Fix restricts session tracking to COOKIE only, and the session cookie
> now carries SameSite=Lax (previously unset). Secure is deliberately not
> forced: Boot opens a plain HTTP listener by default, and an operator
> terminating TLS in front of it sets Secure themselves, as with any other
> servlet container.
> Tests: WildcardClassNameHardeningTest (new, 2 tests);
> AbstractMultiPartRequestTest gains a benign-marker test proving the raw
> accessor is unsanitised and the new sanitised one is not;
> StrutsBootTomcatTest gains testSessionCookieHasSameSiteLax and a
> URL-supplied-session-id-is-not-honoured test, with a
> cookie-based-session-still-works control. Full core suite: 3629 run, 0
> failures, 2 skipped. Full boot suite: 52 run, 0 failures. Existing
> ActionConfigMatcherTest (9), AbstractMatcherTest (1),
> JakartaMultiPartRequestTest (42) and JakartaStreamMultiPartRequestTest (43)
> all pass unaffected.
> Affected versions: items 1 and 2 verified present (same code pattern, by
> source inspection) in the 6.12.0, 7.0.0, 7.3.0 and 7.4.0 tags. Item 3 is
> boot/, new in 7.5.0 (unreleased), so no released version is affected by it.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
