[
https://issues.apache.org/jira/browse/WW-5666?focusedWorklogId=1033024&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1033024
]
ASF GitHub Bot logged work on WW-5666:
--------------------------------------
Author: ASF GitHub Bot
Created on: 30/Jul/26 11:54
Start Date: 30/Jul/26 11:54
Worklog Time Spent: 10m
Work Description: Copilot commented on code in PR #1819:
URL: https://github.com/apache/struts/pull/1819#discussion_r3682435194
##########
core/src/main/java/org/apache/struts2/action/CspReportAction.java:
##########
@@ -51,7 +53,27 @@
* @see DefaultCspReportAction
*/
public abstract class CspReportAction extends ActionSupport implements
ServletRequestAware, ServletResponseAware {
+
+ private static final Logger LOG =
LogManager.getLogger(CspReportAction.class);
+
+ /**
+ * Default upper bound, in characters, on the report body accepted by
{@link #withServletRequest}.
+ * CSP violation reports are small JSON documents; anything larger is not
treated as a report.
+ */
+ public static final int DEFAULT_MAX_REPORT_SIZE = 8192;
+
private HttpServletRequest request;
+ private int maxReportSize = DEFAULT_MAX_REPORT_SIZE;
+
+ /**
+ * Sets the upper bound, in characters, on an accepted report body. A body
exceeding this size is
+ * discarded and not passed to {@link #processReport(String)}.
+ *
+ * @param maxReportSize maximum accepted report size in characters
+ */
+ public void setMaxReportSize(int maxReportSize) {
+ this.maxReportSize = maxReportSize;
+ }
Review Comment:
`setMaxReportSize` assigns the value directly, but `readReport` allocates
`new char[maxReportSize]`. A negative value will throw
`NegativeArraySizeException`, and an excessively large value can cause large
per-request allocations (and can also be influenced by action parameter binding
in typical stacks). Consider sanitizing and clamping the value to a reasonable
hard upper bound to avoid accidental/attacker-controlled memory pressure.
Issue Time Tracking
-------------------
Worklog Id: (was: 1033024)
Time Spent: 40m (was: 0.5h)
> Apply input length limits consistently when reading request bodies
> ------------------------------------------------------------------
>
> Key: WW-5666
> URL: https://issues.apache.org/jira/browse/WW-5666
> Project: Struts 2
> Issue Type: Improvement
> Reporter: Lukasz Lenart
> Assignee: Lukasz Lenart
> Priority: Major
> Fix For: 6.11.0, 7.3.0
>
> Time Spent: 40m
> Remaining Estimate: 0h
>
> h2. Summary
> Two request-body reading paths apply input limits less consistently than the
> rest of the framework. This issue makes the JSON input length limit apply
> uniformly while reading, and gives the CSP reporting path a configurable
> limit of its own.
> h2. Current behaviour
> *JSON plugin*
> {{JSONUtil.deserializeInput(Reader, int)}} accumulates input a line at a time
> and compares the accumulated length against {{struts.json.maxLength}} between
> lines. The limit is therefore applied after input has been accumulated rather
> than while it is being read, which makes enforcement less predictable than
> the other configured limits in the plugin ({{struts.json.maxDepth}},
> {{struts.json.maxElements}}, {{struts.json.maxStringLength}},
> {{struts.json.maxKeyLength}}).
> *CSP reporting*
> {{CspReportAction}} reads the submitted report body with a single
> {{readLine()}} call and has no limit of its own. Unlike the JSON path, there
> is no way for an application to declare how large a report it is prepared to
> accept, and no limit is applied by default.
> h2. Proposed change
> * Evaluate the JSON input length limit as input is read, in fixed-size
> chunks, so that enforcement does not vary with the structure of the input.
> * Read the CSP report body up to a limit, defaulting to 8192 characters,
> exposed as a {{maxReportSize}} property on {{CspReportAction}} so
> applications can tune it. A report above the limit is discarded with a
> warning rather than processed.
> h2. Compatibility notes
> *JSON plugin*
> * Line terminators are no longer stripped while reading. They are
> insignificant whitespace between tokens, so parsing is unaffected.
> * An unescaped control character appearing inside a JSON string value is now
> preserved in the parsed value rather than silently removed. Such input is not
> valid JSON; applications relying on the previous silent removal may observe
> different values.
> *CSP reporting*
> * {{processReport}} now receives the whole body up to the limit rather than
> only its first line.
> * An empty body is passed as an empty string rather than {{null}}.
> h2. Notes
> {{JSONUtil}} is already marked for removal in 8.0.0 under WW-5619. This
> change targets the existing class and does not affect that plan.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
