[
https://issues.apache.org/jira/browse/WW-5820?focusedWorklogId=1052077&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1052077
]
ASF GitHub Bot logged work on WW-5820:
--------------------------------------
Author: ASF GitHub Bot
Created on: 03/Nov/26 07:34
Start Date: 03/Nov/26 07:34
Worklog Time Spent: 10m
Work Description: emilhuseyn commented on PR #2073:
URL: https://github.com/apache/struts/pull/2073#issuecomment-6598027741
@lukaszlenart a process question before this goes further. I wrote these PRs
with Claude Code, as the note at the end of each description says. AGENTS.md
tells AI agents not to open public PRs for suspected vulnerabilities and lists
parameter filtering, and SECURITY.md says a fix is a disclosure even when it is
hardening. You filed WW-5820 in public, so I took this route, but I would
rather ask than assume. Do you classify this as hardening that can stay here,
or should it move to the private process? I will follow what you decide,
including closing it. Separately, I would like to take WW-5787 once this and
the WW-5761 questions are settled, if nobody else has it.
Issue Time Tracking
-------------------
Worklog Id: (was: 1052077)
Time Spent: 1h (was: 50m)
> StrutsParameterAuthorizer's declared-on-neither fallback lets a parameter
> bind to an object below the action in the value stack
> -------------------------------------------------------------------------------------------------------------------------------
>
> Key: WW-5820
> URL: https://issues.apache.org/jira/browse/WW-5820
> Project: Struts 2
> Issue Type: Task
> Components: Core
> Reporter: Lukasz Lenart
> Priority: Major
> Fix For: 7.5.0
>
> Time Spent: 1h
> Remaining Estimate: 0h
>
> {{StrutsParameterAuthorizer.isAuthorizedOnModelDrivenAction}} applies a
> three-way rule since WW-5698: a property declared on the model is exempt from
> {{@StrutsParameter}}, a property declared on the action needs the annotation,
> and a property declared on neither is allowed so that Map-backed and
> custom-accessor models keep working.
> The decision is made about two objects, the model and the action, while the
> write that follows is made against the whole stack.
> {{OgnlValueStack.setParameter}} hands the name to OGNL, and
> {{CompoundRootAccessor.setProperty}} walks every object in the
> {{CompoundRoot}} and invokes the first matching setter. When the root holds
> more than the model and the action, a name that takes the "declared on
> neither" fallback can land on an object the authorizer did not look at.
> The ordinary way the root holds a third object is a chain result:
> {{ActionChainResult.execute}} runs the chained action on the same value stack
> with the same parameters, so the previous action stays in the root below the
> chained action and its model. The convention plugin's
> {{ConventionUnknownHandler}} builds the same chain result implicitly when an
> action {{foo}} returning {{bar}} has a sibling action {{foo-bar}}, so the
> shape also arises without a chain result declared in struts.xml.
> h2. Observed
> Three actions, {{struts.parameters.requireAnnotations=true}}, default stack.
> {{First}} declares an unannotated public setter {{setFoo}}. {{Second}} is
> {{ModelDriven}}, and neither it nor its model declares {{foo}}. {{Plain}} is
> not {{ModelDriven}} and does not declare {{foo}}.
> {code}First directly foo=x -> not bound (rejected,
> no annotation)
> First chained into Plain foo=x -> not bound (rejected, no
> annotation)
> First chained into Second (ModelDriven) foo=x -> bound on First.setFoo
> {code}
> In the third row the authorizer takes the "declared on neither" fallback for
> {{Second}} and its model, and {{CompoundRootAccessor}} finds the setter on
> {{First}}. The model property and the annotated members of {{Second}} bind as
> expected in the same request, so the first two rows are real rejections
> rather than a harness that binds nothing.
> The property copy done by {{ChainingInterceptor}} is not involved. {{chain}}
> runs before {{params}} in {{defaultStack}}, and {{foo}} on {{First}} is still
> null when the copy runs because the parameter phase of {{First}} already
> rejected the name. {{struts.chaining.requireAnnotations}} governs only that
> copy.
> h2. Proposed change
> Make the authorizer's decision and the landing object agree. Two shapes are
> open:
> * Extend the "declared on neither" branch: before allowing the name, look at
> the remaining objects in the {{CompoundRoot}}. Where one of them declares the
> property, apply the annotation requirement to that object instead of taking
> the fallback.
> * Apply the accepted parameter to the object the decision was made about, the
> model or the action, rather than to the whole root. The fallback then keeps
> serving Map-backed models because the model itself absorbs the name. This
> changes how {{ParametersInterceptor}} writes and needs a compatibility look
> before it is chosen.
> h2. Compatibility
> Either shape affects only a request whose parameter name is declared on
> neither the current model nor the current action and is declared on another
> object in the stack. Model binding, annotated action members and Map-backed
> models are unchanged. An application that relies on a chained-into
> {{ModelDriven}} action binding a request parameter onto the previous action
> will stop binding it and needs {{@StrutsParameter}} on that setter, which is
> the requirement the direct request already has.
> h2. Tests
> * Chain into a {{ModelDriven}} action: a name declared only on the previous
> action is rejected.
> * Chain into a {{ModelDriven}} action: the model property, an annotated
> action property and a Map-backed model key still bind.
> * Direct binding and a chain into a plain action keep their current behaviour
> (control cases).
> * The convention plugin's implicit chain reproduces the same case (plugin
> module, optional).
> Reported by Sungho Park (@gigio1023).
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
