[
https://issues.apache.org/jira/browse/WW-5713?focusedWorklogId=1040838&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1040838
]
ASF GitHub Bot logged work on WW-5713:
--------------------------------------
Author: ASF GitHub Bot
Created on: 11/Sep/26 06:44
Start Date: 11/Sep/26 06:44
Worklog Time Spent: 10m
Work Description: lukaszlenart commented on PR #1890:
URL: https://github.com/apache/struts/pull/1890#issuecomment-5630574434
Sorry for the delay — yes, that direction addresses all of it. Please go
ahead.
Resolving the Dispatcher from the current request's `ServletContext` is the
right key: `Dispatcher.getInstance(ServletContext)` (`Dispatcher.java:280`)
reads the `SERVLET_DISPATCHER` attribute, which is per-webapp and cleared on
cleanup, so no static state is involved and a neighbouring application can't
opt this one in. Constructing the raw evaluator lazily and only after that
check keeps the `OgnlRuntime.setPropertyAccessor(Request.class, ...)` mutation
off the default path, which was the main thing. Public constructors for the
explicit opt-in cover the custom-initializer case.
Two small things while you're in there:
- Resolve the flag once per evaluator instance and hold the result, rather
than looking it up from the container on every evaluation. The constant can't
change at runtime, and there is one evaluator per Tiles container, so caching
the decision is safe and avoids a container lookup per rendered attribute.
- `ServletUtil.getServletRequest(request)` throws
`NotAServletEnvironmentException` for a non-servlet `Request`. Treat that the
same as "no Dispatcher": fail closed with the migration message, not a stack
trace from the lookup.
A separate PR for `support/struts-6-x-x` afterwards is fine — plain
`@Deprecated` there, as discussed.
Issue Time Tracking
-------------------
Worklog Id: (was: 1040838)
Time Spent: 40m (was: 0.5h)
> Fail closed for legacy Tiles OGNL evaluation
> --------------------------------------------
>
> Key: WW-5713
> URL: https://issues.apache.org/jira/browse/WW-5713
> Project: Struts 2
> Issue Type: Improvement
> Components: Plugin - Tiles
> Reporter: Karan Kurani
> Priority: Major
> Fix For: 7.4.0
>
> Time Spent: 40m
> Remaining Estimate: 0h
>
> The Struts Tiles plugin registers separate {{S2:}} and legacy {{OGNL:}}
> attribute-expression evaluators.
> The {{S2:}} evaluator processes ValueStack expressions through the Struts
> OGNL facilities. The legacy {{OGNL:}} evaluator evaluates against the Tiles
> {{Request}} and does not use the Struts OGNL security controls used by
> {{{}S2:{}}}.
> This hardening change keeps {{OGNL:}} registered but makes it fail closed by
> default. Evaluation throws Tiles {{EvaluationException}} with migration
> guidance directing users to {{{}S2:{}}}.
> Applications that temporarily require the existing raw behavior may
> explicitly set:
> {{struts.tiles.ognl.legacy.enabled=true}}
> The compatibility flag defaults to {{false}} and is targeted for removal in
> Struts 8.0.0.
> The raw evaluator and its global {{OgnlRuntime}} {{Request}}
> property-accessor setup are constructed only when legacy mode is explicitly
> enabled. This avoids mutating the shared OGNL runtime for applications using
> the secure default.
> When legacy mode is enabled, the existing raw behavior is preserved and a
> clear startup migration warning is emitted.
> {{{}S2:{}}}, {{{}I18N:{}}}, and {{EL:}} remain unchanged.
> Tests cover:
> * default fail-closed behavior;
> * exact migration guidance;
> * non-evaluation of expression markers;
> * construction gating;
> * absence of global accessor initialization under the default;
> * explicit legacy compatibility behavior;
> * startup warning behavior; and
> * real servlet-backed {{S2:}} evaluation.
> This is defense-in-depth hardening. No attacker-controlled expression source
> or concrete vulnerability impact has been demonstrated, and no security
> advisory is proposed.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
