diff --git a/core/src/main/java/org/apache/struts2/action/CspReportAction.java b/core/src/main/java/org/apache/struts2/action/CspReportAction.java index c8b6b7bc51..158cfd3bcc 100644 --- a/core/src/main/java/org/apache/struts2/action/CspReportAction.java +++ b/core/src/main/java/org/apache/struts2/action/CspReportAction.java @@ -18,12 +18,14 @@ */ package org.apache.struts2.action; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.apache.struts2.ActionSupport; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; -import java.io.BufferedReader; import java.io.IOException; +import java.io.Reader; import static org.apache.struts2.interceptor.csp.CspSettings.CSP_REPORT_TYPE; @@ -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; + } @Override public void withServletRequest(HttpServletRequest request) { @@ -60,13 +82,35 @@ public void withServletRequest(HttpServletRequest request) { } try { - BufferedReader reader = request.getReader(); - String cspReport = reader.readLine(); + String cspReport = readReport(request.getReader()); + if (cspReport == null) { + LOG.warn("Discarding CSP report larger than the configured limit of {} characters", maxReportSize); + return; + } processReport(cspReport); } catch (IOException ignored) { } } + /** + * Reads at most {@link #maxReportSize} characters from the report body. + * + * @param reader reader over the report body + * @return the report body, or {@code null} if it exceeds the configured limit + */ + private String readReport(Reader reader) throws IOException { + char[] buffer = new char[maxReportSize]; + int total = 0; + int read; + while (total < buffer.length && (read = reader.read(buffer, total, buffer.length - total)) != -1) { + total += read; + } + if (total == buffer.length && reader.read() != -1) { + return null; + } + return new String(buffer, 0, total); + } + private boolean isCspReportRequest(HttpServletRequest request) { if (!"POST".equals(request.getMethod()) || request.getContentLength() <= 0){ return false; diff --git a/core/src/test/java/org/apache/struts2/action/CspReportActionReportSizeTest.java b/core/src/test/java/org/apache/struts2/action/CspReportActionReportSizeTest.java new file mode 100644 index 0000000000..46f54aeaee --- /dev/null +++ b/core/src/test/java/org/apache/struts2/action/CspReportActionReportSizeTest.java @@ -0,0 +1,145 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.struts2.action; + +import org.apache.struts2.XWorkTestCase; +import org.apache.struts2.interceptor.csp.CspSettings; +import org.springframework.mock.web.MockHttpServletRequest; + +import java.io.BufferedReader; +import java.io.Reader; +import java.util.concurrent.atomic.AtomicLong; + +/** + * Verifies that {@link CspReportAction} applies an upper bound to the report body it accepts, and + * that the bound is configurable. + */ +public class CspReportActionReportSizeTest extends XWorkTestCase { + + /** + * The reader supplied by the container buffers ahead, so consumption is bounded by the limit + * plus one buffer rather than by the limit exactly. That overshoot is fixed, not proportional + * to the size of the body. + */ + private static final long READ_AHEAD_ALLOWANCE = 8192L; + + /** + * Produces {@code total} characters without buffering them, and records how many the caller + * actually consumed. + */ + private static final class CountingReader extends Reader { + private final long total; + private final AtomicLong consumed; + private long produced = 0; + + CountingReader(long total, AtomicLong consumed) { + this.total = total; + this.consumed = consumed; + } + + @Override + public int read(char[] cbuf, int off, int len) { + if (produced >= total) { + return -1; + } + int count = (int) Math.min(len, total - produced); + for (int i = 0; i < count; i++) { + cbuf[off + i] = 'a'; + } + produced += count; + consumed.addAndGet(count); + return count; + } + + @Override + public void close() { + // characters are generated on demand, so there is nothing to release + } + } + + private static final class CapturingCspReportAction extends CspReportAction { + String captured; + int reports; + + @Override + void processReport(String jsonCspReport) { + captured = jsonCspReport; + reports++; + } + } + + /** + * A request that both declares and delivers {@code size} characters, matching what a client can + * actually send: the declared length and the delivered body agree. + */ + private MockHttpServletRequest requestOfSize(final long size, final AtomicLong consumed) { + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/csp-reports") { + @Override + public int getContentLength() { + return (int) Math.min(size, Integer.MAX_VALUE); + } + + @Override + public BufferedReader getReader() { + return new BufferedReader(new CountingReader(size, consumed)); + } + }; + request.setContentType(CspSettings.CSP_REPORT_TYPE); + return request; + } + + public void testReportAboveLimitIsNotProcessed() { + AtomicLong consumed = new AtomicLong(); + MockHttpServletRequest request = requestOfSize(64L * 1024 * 1024, consumed); + + CapturingCspReportAction action = new CapturingCspReportAction(); + action.withServletRequest(request); + + assertEquals("A report above the limit should not be processed", 0, action.reports); + assertTrue("Consumed " + consumed.get() + " characters for a limit of " + + CspReportAction.DEFAULT_MAX_REPORT_SIZE, + consumed.get() <= CspReportAction.DEFAULT_MAX_REPORT_SIZE + READ_AHEAD_ALLOWANCE); + } + + public void testReportWithinLimitIsProcessed() { + String sampleReport = "{\"csp-report\":{\"document-uri\":\"https://example.test/\"}}"; + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/csp-reports"); + request.setContent(sampleReport.getBytes()); + request.setContentType(CspSettings.CSP_REPORT_TYPE); + + CapturingCspReportAction action = new CapturingCspReportAction(); + action.withServletRequest(request); + + assertEquals("A report within the limit should be processed", 1, action.reports); + assertEquals("The report should be passed through unchanged", sampleReport, action.captured); + } + + public void testConfiguredLimitIsApplied() { + AtomicLong consumed = new AtomicLong(); + MockHttpServletRequest request = requestOfSize(4096, consumed); + + CapturingCspReportAction action = new CapturingCspReportAction(); + action.setMaxReportSize(1024); + action.withServletRequest(request); + + assertEquals("A report above the configured limit should not be processed", 0, action.reports); + assertTrue("Consumed " + consumed.get() + " characters for a configured limit of 1024", + consumed.get() <= 1024L + READ_AHEAD_ALLOWANCE); + } +} diff --git a/plugins/json/src/main/java/org/apache/struts2/json/JSONUtil.java b/plugins/json/src/main/java/org/apache/struts2/json/JSONUtil.java index 78bceaacf3..e1c20f7b1c 100644 --- a/plugins/json/src/main/java/org/apache/struts2/json/JSONUtil.java +++ b/plugins/json/src/main/java/org/apache/struts2/json/JSONUtil.java @@ -59,6 +59,9 @@ public class JSONUtil { private static final Logger LOG = LogManager.getLogger(JSONUtil.class); + /** Chunk size used to read input incrementally while applying the length limit. */ + private static final int READ_CHUNK_SIZE = 8192; + private JSONReader reader; private JSONWriter writer; @@ -297,13 +300,15 @@ public void serialize(Writer writer, Object object, Collection excludeP * @throws JSONException when IOException happens or limits are exceeded */ public Object deserializeInput(Reader reader, int maxLength) throws JSONException { - BufferedReader bufferReader = new BufferedReader(reader); - String line; StringBuilder buffer = new StringBuilder(); + char[] chunk = new char[READ_CHUNK_SIZE]; try { - while ((line = bufferReader.readLine()) != null) { - buffer.append(line); + int read; + // Apply the limit while reading rather than afterwards, so input that contains no + // line terminator is not accumulated in full before the limit can be evaluated. + while ((read = reader.read(chunk)) != -1) { + buffer.append(chunk, 0, read); if (buffer.length() > maxLength) { throw new JSONException("JSON input exceeds maximum allowed length (" + maxLength + "). Use " + JSONConstants.JSON_MAX_LENGTH + " to increase the limit."); diff --git a/plugins/json/src/test/java/org/apache/struts2/json/JSONUtilInputLimitTest.java b/plugins/json/src/test/java/org/apache/struts2/json/JSONUtilInputLimitTest.java new file mode 100644 index 0000000000..4dbe70b251 --- /dev/null +++ b/plugins/json/src/test/java/org/apache/struts2/json/JSONUtilInputLimitTest.java @@ -0,0 +1,114 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.struts2.json; + +import org.junit.Test; + +import java.io.Reader; +import java.io.StringReader; +import java.util.Map; +import java.util.concurrent.atomic.AtomicLong; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; + +/** + * Verifies that {@link JSONUtil#deserializeInput(Reader, int)} applies the configured input length + * limit while reading, bounding how much input is consumed before the limit takes effect, and that + * input within the limit still parses. + */ +public class JSONUtilInputLimitTest { + + /** + * Emits {@code total} characters with no line terminator anywhere, and records how many + * characters the caller actually consumed. + */ + private static final class UnterminatedReader extends Reader { + private final long total; + private final AtomicLong consumed; + private long produced = 0; + + UnterminatedReader(long total, AtomicLong consumed) { + this.total = total; + this.consumed = consumed; + } + + @Override + public int read(char[] cbuf, int off, int len) { + if (produced >= total) { + return -1; + } + int count = (int) Math.min(len, total - produced); + for (int i = 0; i < count; i++) { + cbuf[off + i] = 'a'; + } + produced += count; + consumed.addAndGet(count); + return count; + } + + @Override + public void close() { + // characters are generated on demand, so there is nothing to release + } + } + + @Test + public void inputWithoutLineTerminatorIsLimitedWhileReading() { + int maxLength = 1024; + long inputSize = 64L * 1024 * 1024; + AtomicLong consumed = new AtomicLong(); + + JSONUtil util = new JSONUtil(); + Reader input = new UnterminatedReader(inputSize, consumed); + + assertThrows(JSONException.class, () -> util.deserializeInput(input, maxLength)); + + long read = consumed.get(); + // Reading proceeds in chunks, so a single chunk of overshoot beyond the limit is expected. + assertTrue("Consumed " + read + " characters for a limit of " + maxLength, + read < maxLength + 65_536L); + } + + @Test + public void inputWithinLimitIsParsed() throws JSONException { + JSONUtil util = new JSONUtil(); + util.setReader(new StrutsJSONReader()); + + Object result = util.deserializeInput(new StringReader("{\"a\":1, \"b\":\"hello\"}"), 1024); + + assertTrue("Expected a parsed JSON object", result instanceof Map); + assertEquals(1L, ((Map) result).get("a")); + assertEquals("hello", ((Map) result).get("b")); + } + + @Test + public void inputSpanningMultipleLinesIsParsed() throws JSONException { + JSONUtil util = new JSONUtil(); + util.setReader(new StrutsJSONReader()); + + // Line terminators between tokens are insignificant whitespace to the reader. + Object result = util.deserializeInput(new StringReader("{\n\"a\":1,\n\"b\":2\n}"), 1024); + + assertTrue("Expected a parsed JSON object", result instanceof Map); + assertEquals(1L, ((Map) result).get("a")); + assertEquals(2L, ((Map) result).get("b")); + } +}