On Thu, Nov 20, 2025 at 4:30 PM Dimitris Soumis <[email protected]> wrote: > > On Thu, Nov 20, 2025 at 4:57 PM Rémy Maucherat <[email protected]> wrote: > > > On Thu, Nov 20, 2025 at 3:39 PM <[email protected]> wrote: > > > > > > This is an automated email from the ASF dual-hosted git repository. > > > > > > dsoumis pushed a commit to branch main > > > in repository https://gitbox.apache.org/repos/asf/tomcat.git > > > > > > > > > The following commit(s) were added to refs/heads/main by this push: > > > new 799b6f314d Add a unit test for CVE-2023-24998 and CVE-2023-28709 > > > 799b6f314d is described below > > > > > > commit 799b6f314d84ccabf52bd6b384e8cb3f2c4d7521 > > > Author: Dimitris Soumis <[email protected]> > > > AuthorDate: Wed Nov 19 16:33:27 2025 +0200 > > > > > > Add a unit test for CVE-2023-24998 and CVE-2023-28709 > > > > It is probably good to have an explicit named test as much as > > possible, but out of curiosity I looked if the test coverage improved > > and it seems not (in particular Request.parseParts looks the same). So > > leave this test there, but it is something to keep in mind when adding > > tests. > > Also: it is not because code is run over that some useful assertions > > are tested. So it's complicated ;) > > > I agree that aggregating CVE tests can lead to duplication without moving > meaningful coverage. However, how do we proceed with these? > If I implement extra assertions (the useful ones) we drift away from > replicating the CVE.
Most of the time the test should be added. Maybe sometimes it is possible to skip a test, especially if it is complex and already tested elsewhere. Rémy > > > > Rémy > > > > > --- > > > .../apache/tomcat/security/TestSecurity2023.java | 143 > > +++++++++++++++++++++ > > > 1 file changed, 143 insertions(+) > > > > > > diff --git a/test/org/apache/tomcat/security/TestSecurity2023.java > > b/test/org/apache/tomcat/security/TestSecurity2023.java > > > new file mode 100644 > > > index 0000000000..34f6cff0c5 > > > --- /dev/null > > > +++ b/test/org/apache/tomcat/security/TestSecurity2023.java > > > @@ -0,0 +1,143 @@ > > > +/* > > > + * 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.tomcat.security; > > > + > > > +import java.io.IOException; > > > +import java.io.InputStream; > > > +import java.io.OutputStream; > > > +import java.io.Serial; > > > +import java.net.HttpURLConnection; > > > +import java.net.URI; > > > +import java.net.URISyntaxException; > > > + > > > +import jakarta.servlet.MultipartConfigElement; > > > +import jakarta.servlet.ServletException; > > > +import jakarta.servlet.http.HttpServlet; > > > +import jakarta.servlet.http.HttpServletRequest; > > > +import jakarta.servlet.http.HttpServletResponse; > > > + > > > +import org.junit.Assert; > > > +import org.junit.Test; > > > + > > > +import static org.apache.catalina.startup.SimpleHttpClient.CRLF; > > > +import org.apache.catalina.Context; > > > +import org.apache.catalina.Wrapper; > > > +import org.apache.catalina.startup.Tomcat; > > > +import org.apache.catalina.startup.TomcatBaseTest; > > > + > > > +public class TestSecurity2023 extends TomcatBaseTest { > > > + /* > > > + * https://www.cve.org/CVERecord?id=CVE-2023-24998 > > > + * > > > + * Fixed in > > > + * 11.0.0-M3 > > https://github.com/apache/tomcat/commit/063e2e81ede50c287f737cc8e2915ce7217e886e > > > + * 10.1.5 > > https://github.com/apache/tomcat/commit/8a2285f13affa961cc65595aad999db5efae45ce > > > + * 9.0.71 > > https://github.com/apache/tomcat/commit/cf77cc545de0488fb89e24294151504a7432df74 > > > + * > > > + * https://www.cve.org/CVERecord?id=CVE-2023-28709 > > > + * > > > + * Fixed in > > > + * 11.0.0-M5 > > https://github.com/apache/tomcat/commit/d53d8e7f77042cc32a3b98f589496a1ef5088e38 > > > + * 10.1.8 > > https://github.com/apache/tomcat/commit/ba848da71c523d94950d3c53c19ea155189df9dc > > > + * 9.0.74 > > https://github.com/apache/tomcat/commit/fbd81421629afe8b8a3922d59020cde81caea861 > > > + */ > > > + @Test > > > + public void testCVE_2023_24998_CVE_2023_28709() throws Exception { > > > + Tomcat tomcat = getTomcatInstance(); > > > + tomcat.getConnector().setMaxParameterCount(2); > > > + Context context = tomcat.addContext("", null); > > > + Wrapper wrapper = Tomcat.addServlet(context, "test", new > > TestServlet()); > > > + wrapper.setMultipartConfigElement(new > > MultipartConfigElement("")); > > > + context.addServletMappingDecoded("/", "test"); > > > + > > > + tomcat.start(); > > > + > > > + final String path = "http://localhost:"; + getPort() + "/"; > > > + int status = postMultipart(path, null, 1); > > > + Assert.assertEquals(HttpServletResponse.SC_OK, status); > > > + > > > + status = postMultipart(path, null, 3); > > > + > > Assert.assertEquals(HttpServletResponse.SC_REQUEST_ENTITY_TOO_LARGE, > > status); > > > + > > > + status = postMultipart(path,"x=1", 1); > > > + Assert.assertEquals(HttpServletResponse.SC_OK, status); > > > + > > > + status = postMultipart(path, "x=1&y=2", 1); > > > + > > Assert.assertEquals(HttpServletResponse.SC_REQUEST_ENTITY_TOO_LARGE, > > status); > > > + } > > > + > > > + @SuppressWarnings("StatementWithEmptyBody") > > > + private static int postMultipart(String path, String > > queryStringParams, int parts) throws IOException, URISyntaxException { > > > + String urlStr = path + (queryStringParams == null || > > queryStringParams.isEmpty() ? "" : "?" + queryStringParams); > > > + String boundary = "--simpleboundary"; > > > + > > > + byte[] body = buildMultipartBody(boundary, parts); > > > + HttpURLConnection conn = (HttpURLConnection) new > > URI(urlStr).toURL().openConnection(); > > > + conn.setDoOutput(true); > > > + conn.setRequestProperty("Content-Type", "multipart/form-data; > > boundary=" + boundary); > > > + conn.setRequestProperty("Content-Length", > > String.valueOf(body.length)); > > > + conn.setReadTimeout(1000000); > > > + > > > + try (OutputStream os = conn.getOutputStream()) { > > > + os.write(body); > > > + } > > > + > > > + int rc = conn.getResponseCode(); > > > + InputStream inputStream = null; > > > + if (rc == HttpServletResponse.SC_OK) { > > > + inputStream = conn.getInputStream(); > > > + } else if (rc == > > HttpServletResponse.SC_REQUEST_ENTITY_TOO_LARGE) { > > > + inputStream = conn.getErrorStream(); > > > + } > > > + if (inputStream != null) { > > > + while (inputStream.read() != -1) {} > > > + inputStream.close(); > > > + } > > > + > > > + return rc; > > > + } > > > + > > > + private static byte[] buildMultipartBody(String boundary, int > > parts) { > > > + StringBuilder sb = new StringBuilder(); > > > + for (int i = 0; i < parts; i++) { > > > + sb.append("--"); > > > + sb.append(boundary); > > > + sb.append(CRLF); > > > + sb.append("Content-Disposition: form-data; name=\"part\""); > > > + sb.append(CRLF); > > > + sb.append(CRLF); > > > + sb.append("bodyOfPart"); > > > + sb.append(CRLF); > > > + } > > > + sb.append("--"); > > > + sb.append(boundary); > > > + sb.append("--"); > > > + sb.append(CRLF); > > > + return sb.toString().getBytes(); > > > + } > > > + > > > + public static class TestServlet extends HttpServlet { > > > + @Serial > > > + private static final long serialVersionUID = 1L; > > > + > > > + @Override > > > + protected void doPost(HttpServletRequest req, > > HttpServletResponse resp) throws IOException, ServletException { > > > + req.getParameterMap(); > > > + } > > > + } > > > +} > > > > > > > > > --------------------------------------------------------------------- > > > To unsubscribe, e-mail: [email protected] > > > For additional commands, e-mail: [email protected] > > > > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: [email protected] > > For additional commands, e-mail: [email protected] > > > > --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
