[
https://issues.apache.org/jira/browse/WW-5839?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Gouri Sankar A updated WW-5839:
-------------------------------
Description:
AbstractMatcher/ActionConfigMatcher: 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.
Tests: WildcardClassNameHardeningTest (new, 2 tests). Full core suite: 3742/0
failures.
Affected versions: present, same code pattern, by source inspection, in the
6.12.0, 7.0.0, 7.3.0 and 7.4.0 tags.
Pull request: https://github.com/apache/struts/pull/2115 (already open,
narrowed)
was:
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.
> Reject unconfined wildcard substitution into class= attributes
> --------------------------------------------------------------
>
> 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: 0.5h
> Remaining Estimate: 0h
>
> AbstractMatcher/ActionConfigMatcher: 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.
> Tests: WildcardClassNameHardeningTest (new, 2 tests). Full core suite: 3742/0
> failures.
> Affected versions: present, same code pattern, by source inspection, in the
> 6.12.0, 7.0.0, 7.3.0 and 7.4.0 tags.
> Pull request: https://github.com/apache/struts/pull/2115 (already open,
> narrowed)
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
