Denial of service in Jenkins Core

@@ -40,7 +40,7 @@ THE SOFTWARE.
   <properties>
     <asm.version>9.4</asm.version>
     <slf4jVersion>2.0.6</slf4jVersion>
-    <stapler.version>1770.v0c3dc82ee103</stapler.version>
+    <stapler.version>1775.v3181508ea_21e</stapler.version>
     <groovy.version>2.4.21</groovy.version>
   </properties>
  

@@ -29,11 +29,15 @@
 import java.util.Map;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
+import org.apache.commons.fileupload.FileCountLimitExceededException;
 import org.apache.commons.fileupload.FileItem;
+import org.apache.commons.fileupload.FileUploadBase;
 import org.apache.commons.fileupload.FileUploadException;
 import org.apache.commons.fileupload.disk.DiskFileItemFactory;
 import org.apache.commons.fileupload.servlet.ServletFileUpload;
 import org.apache.commons.lang.ArrayUtils;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
  
 /**
  * Wraps commons file-upload and handles a "multipart/form-data" form submission
@@ -45,15 +49,60 @@ public class MultipartFormDataParser implements AutoCloseable {
     private final ServletFileUpload upload = new ServletFileUpload(new DiskFileItemFactory());
     private final Map<String, FileItem> byName = new HashMap<>();
  
-    public MultipartFormDataParser(HttpServletRequest request) throws ServletException {
+    /**
+     * Limits the number of form fields that can be processed in one multipart/form-data request.
+     * Used to set {@link org.apache.commons.fileupload.servlet.ServletFileUpload#setFileCountMax(long)}.
+     * Despite the name, this applies to all form fields, not just actual file attachments.
+     * Set to {@code -1} to disable limits.
+     */
+    private static /* nonfinal for Jenkins script console */ int FILEUPLOAD_MAX_FILES = Integer.getInteger(MultipartFormDataParser.class.getName() + ".FILEUPLOAD_MAX_FILES", 1000);
+ 
+    /**
+     * Limits the size (in bytes) of individual fields that can be processed in one multipart/form-data request.
+     * Used to set {@link org.apache.commons.fileupload.servlet.ServletFileUpload#setFileSizeMax(long)}.
+     * Despite the name, this applies to all form fields, not just actual file attachments.
+     * Set to {@code -1} to disable limits.
+     */
+    private static /* nonfinal for Jenkins script console */ long FILEUPLOAD_MAX_FILE_SIZE = Long.getLong(MultipartFormDataParser.class.getName() + ".FILEUPLOAD_MAX_FILE_SIZE", -1);
+ 
+    /**
+     * Limits the total request size (in bytes) that can be processed in one multipart/form-data request.
+     * Used to set {@link org.apache.commons.fileupload.servlet.ServletFileUpload#setSizeMax(long)}.
+     * Set to {@code -1} to disable limits.
+     */
+    private static /* nonfinal for Jenkins script console */ long FILEUPLOAD_MAX_SIZE = Long.getLong(MultipartFormDataParser.class.getName() + ".FILEUPLOAD_MAX_SIZE", -1);
+ 
+    @Restricted(NoExternalUse.class)
+    public MultipartFormDataParser(HttpServletRequest request, int maxParts, long maxPartSize, long maxSize) throws ServletException {
+        upload.setFileCountMax(maxParts);
+        upload.setFileSizeMax(maxPartSize);
+        upload.setSizeMax(maxSize);
         try {
             for (FileItem fi : upload.parseRequest(request))
                 byName.put(fi.getFieldName(), fi);
+        } catch (FileCountLimitExceededException e) {
+            throw new ServletException("File upload field count limit exceeded. Consider setting the Java system property "
+                    + MultipartFormDataParser.class.getName() + ".FILEUPLOAD_MAX_FILES to a value greater than " + FILEUPLOAD_MAX_FILES + ", or to -1 to disable this limit.", e);
+        } catch (FileUploadBase.FileSizeLimitExceededException e) {
+            throw new ServletException("File upload field size limit exceeded. Consider setting the Java system property "
+                    + MultipartFormDataParser.class.getName() + ".FILEUPLOAD_MAX_FILE_SIZE to a value greater than " + FILEUPLOAD_MAX_FILE_SIZE + ", or to -1 to disable this limit.", e);
+        } catch (FileUploadBase.SizeLimitExceededException e) {
+            throw new ServletException("File upload total size limit exceeded. Consider setting the Java system property "
+                    + MultipartFormDataParser.class.getName() + ".FILEUPLOAD_MAX_SIZE to a value greater than " + FILEUPLOAD_MAX_SIZE + ", or to -1 to disable this limit.", e);
         } catch (FileUploadException e) {
             throw new ServletException(e);
         }
     }
  
+    @Restricted(NoExternalUse.class)
+    public MultipartFormDataParser(HttpServletRequest request, int maxParts) throws ServletException {
+        this(request, maxParts, FILEUPLOAD_MAX_FILE_SIZE, FILEUPLOAD_MAX_SIZE);
+    }
+ 
+    public MultipartFormDataParser(HttpServletRequest request) throws ServletException {
+        this(request, FILEUPLOAD_MAX_FILES, FILEUPLOAD_MAX_FILE_SIZE, FILEUPLOAD_MAX_SIZE);
+    }
+ 
     public String get(String key) {
         FileItem fi = byName.get(key);
         if (fi == null)    return null;

@@ -4398,7 +4398,7 @@ public void reload() throws IOException, InterruptedException, ReactorException
     @RequirePOST
     public void doDoFingerprintCheck(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException {
         // Parse the request
-        try (MultipartFormDataParser p = new MultipartFormDataParser(req)) {
+        try (MultipartFormDataParser p = new MultipartFormDataParser(req, 10)) {
             if (isUseCrumbs() && !getCrumbIssuer().validateCrumb(req, p)) {
                 // TODO investigate whether this check can be removed
                 rsp.sendError(HttpServletResponse.SC_FORBIDDEN, "No crumb found");

@@ -0,0 +1,330 @@
+/*
+ * The MIT License
+ *
+ * Copyright (c) 2023, CloudBees, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+ 
+package jenkins.security;
+ 
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+ 
+import com.gargoylesoftware.htmlunit.HttpMethod;
+import com.gargoylesoftware.htmlunit.WebRequest;
+import hudson.ExtensionList;
+import hudson.model.RootAction;
+import hudson.util.HttpResponses;
+import hudson.util.MultipartFormDataParser;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.io.OutputStreamWriter;
+import java.io.PrintWriter;
+import java.lang.reflect.Field;
+import java.net.URL;
+import java.nio.charset.StandardCharsets;
+import java.util.Random;
+import javax.servlet.ServletException;
+import org.apache.commons.fileupload.FileCountLimitExceededException;
+import org.apache.commons.fileupload.FileUploadBase;
+import org.apache.commons.fileupload.FileUploadException;
+import org.junit.Assert;
+import org.junit.Rule;
+import org.junit.Test;
+import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.TestExtension;
+import org.kohsuke.stapler.HttpResponse;
+import org.kohsuke.stapler.RequestImpl;
+import org.kohsuke.stapler.StaplerRequest;
+import org.kohsuke.stapler.verb.POST;
+ 
+public class Security3030Test {
+    // TODO Consider parameterizing with Stapler (RequestImpl/StaplerRequestFormAction) + Jenkins (MultipartFormDataParser/MultipartFormDataParserAction)
+    @Rule
+    public JenkinsRule j = new JenkinsRule();
+ 
+    @Test
+    public void fewFilesStapler() throws IOException {
+        assertSubmissionOK(StaplerRequestFormAction.instance(), 20, 10, 1024 * 1024);
+        assertSubmissionOK(StaplerRequestFormAction.instance(), 10, 41, 10);
+    }
+ 
+    @Test
+    public void tooManyFilesStapler() throws Exception {
+        ServletException ex = assertSubmissionThrows(StaplerRequestFormAction.instance(), 10, 1000, 20, FileCountLimitExceededException.class);
+        assertThat(ex.getMessage(), containsString(RequestImpl.class.getName() + ".FILEUPLOAD_MAX_FILES"));
+        ex = assertSubmissionThrows(StaplerRequestFormAction.instance(), 1000, 10, 10, FileCountLimitExceededException.class);
+        assertThat(ex.getMessage(), containsString(RequestImpl.class.getName() + ".FILEUPLOAD_MAX_FILES"));
+        try (FieldValue v = withStaticField(RequestImpl.class, "FILEUPLOAD_MAX_FILES", 10_000)) {
+            assertSubmissionOK(StaplerRequestFormAction.instance(), 1000, 10, 10);
+            ex = assertSubmissionThrows(StaplerRequestFormAction.instance(), 10_000, 10, 10, FileCountLimitExceededException.class);
+            assertThat(ex.getMessage(), containsString(RequestImpl.class.getName() + ".FILEUPLOAD_MAX_FILES"));
+        }
+        ex = assertSubmissionThrows(StaplerRequestFormAction.instance(), 10, 1000, 20, FileCountLimitExceededException.class);
+        assertThat(ex.getMessage(), containsString(RequestImpl.class.getName() + ".FILEUPLOAD_MAX_FILES"));
+        ex = assertSubmissionThrows(StaplerRequestFormAction.instance(), 1000, 10, 10, FileCountLimitExceededException.class);
+        assertThat(ex.getMessage(), containsString(RequestImpl.class.getName() + ".FILEUPLOAD_MAX_FILES"));
+    }
+ 
+    @Test
+    public void tooLargeFilesStapler() throws Exception {
+        assertSubmissionOK(StaplerRequestFormAction.instance(), 1, 50, 10 * 1024 * 1024);
+        try (FieldValue v = withStaticField(RequestImpl.class, "FILEUPLOAD_MAX_FILE_SIZE", 1024 * 1024)) {
+            assertSubmissionOK(StaplerRequestFormAction.instance(), 200, 100, 1024);
+            ServletException ex = assertSubmissionThrows(StaplerRequestFormAction.instance(), 1, 50, 10 * 1024 * 1024, FileUploadBase.FileSizeLimitExceededException.class);
+            assertThat(ex.getMessage(), containsString(RequestImpl.class.getName() + ".FILEUPLOAD_MAX_FILE_SIZE"));
+        }
+        assertSubmissionOK(StaplerRequestFormAction.instance(), 1, 50, 10 * 1024 * 1024);
+    }
+ 
+    @Test
+    public void tooLargeSubmissionStapler() throws Exception {
+        assertSubmissionOK(StaplerRequestFormAction.instance(), 1, 50, 10 * 1024 * 1024);
+        try (FieldValue v = withStaticField(RequestImpl.class, "FILEUPLOAD_MAX_SIZE", 1024 * 1024)) {
+            assertSubmissionOK(StaplerRequestFormAction.instance(), 200, 100, 1024);
+            ServletException ex = assertSubmissionThrows(StaplerRequestFormAction.instance(), 1, 50, 10 * 1024 * 1024, FileUploadBase.SizeLimitExceededException.class);
+            assertThat(ex.getMessage(), containsString(RequestImpl.class.getName() + ".FILEUPLOAD_MAX_SIZE"));
+        }
+        assertSubmissionOK(StaplerRequestFormAction.instance(), 1, 50, 10 * 1024 * 1024);
+    }
+ 
+    @Test
+    public void fewFilesParser() throws IOException {
+        assertSubmissionOK(MultipartFormDataParserAction.instance(), 20, 10, 1024 * 1024);
+        assertSubmissionOK(MultipartFormDataParserAction.instance(), 200, 100, 1024);
+    }
+ 
+    @Test
+    public void tooManyFilesParser() throws Exception {
+        ServletException ex = assertSubmissionThrows(MultipartFormDataParserAction.instance(), 10, 1000, 20, FileCountLimitExceededException.class);
+        assertThat(ex.getMessage(), containsString(MultipartFormDataParser.class.getName() + ".FILEUPLOAD_MAX_FILES"));
+        ex = assertSubmissionThrows(MultipartFormDataParserAction.instance(), 1000, 10, 10, FileCountLimitExceededException.class);
+        assertThat(ex.getMessage(), containsString(MultipartFormDataParser.class.getName() + ".FILEUPLOAD_MAX_FILES"));
+        try (FieldValue v = withStaticField(MultipartFormDataParser.class, "FILEUPLOAD_MAX_FILES", 10_000)) {
+            assertSubmissionOK(MultipartFormDataParserAction.instance(), 1000, 10, 10);
+            ex = assertSubmissionThrows(MultipartFormDataParserAction.instance(), 10_000, 10, 10, FileCountLimitExceededException.class);
+            assertThat(ex.getMessage(), containsString(MultipartFormDataParser.class.getName() + ".FILEUPLOAD_MAX_FILES"));
+        }
+        ex = assertSubmissionThrows(MultipartFormDataParserAction.instance(), 10, 1000, 20, FileCountLimitExceededException.class);
+        assertThat(ex.getMessage(), containsString(MultipartFormDataParser.class.getName() + ".FILEUPLOAD_MAX_FILES"));
+        ex = assertSubmissionThrows(MultipartFormDataParserAction.instance(), 1000, 10, 10, FileCountLimitExceededException.class);
+        assertThat(ex.getMessage(), containsString(MultipartFormDataParser.class.getName() + ".FILEUPLOAD_MAX_FILES"));
+    }
+ 
+    @Test
+    public void tooLargeFilesParser() throws Exception {
+        assertSubmissionOK(MultipartFormDataParserAction.instance(), 1, 50, 10 * 1024 * 1024);
+        try (FieldValue v = withStaticField(MultipartFormDataParser.class, "FILEUPLOAD_MAX_FILE_SIZE", 1024 * 1024)) {
+            assertSubmissionOK(MultipartFormDataParserAction.instance(), 200, 100, 1024);
+            ServletException ex = assertSubmissionThrows(MultipartFormDataParserAction.instance(), 1, 50, 10 * 1024 * 1024, FileUploadBase.FileSizeLimitExceededException.class);
+            assertThat(ex.getMessage(), containsString(MultipartFormDataParser.class.getName() + ".FILEUPLOAD_MAX_FILE_SIZE"));
+        }
+        assertSubmissionOK(MultipartFormDataParserAction.instance(), 1, 50, 10 * 1024 * 1024);
+    }
+ 
+    @Test
+    public void tooLargeSubmissionParser() throws Exception {
+        assertSubmissionOK(MultipartFormDataParserAction.instance(), 1, 50, 10 * 1024 * 1024);
+        try (FieldValue v = withStaticField(MultipartFormDataParser.class, "FILEUPLOAD_MAX_SIZE", 1024 * 1024)) {
+            assertSubmissionOK(MultipartFormDataParserAction.instance(), 200, 100, 1024);
+            ServletException ex = assertSubmissionThrows(MultipartFormDataParserAction.instance(), 1, 50, 10 * 1024 * 1024, FileUploadBase.SizeLimitExceededException.class);
+            assertThat(ex.getMessage(), containsString(MultipartFormDataParser.class.getName() + ".FILEUPLOAD_MAX_SIZE"));
+        }
+        assertSubmissionOK(MultipartFormDataParserAction.instance(), 1, 50, 10 * 1024 * 1024);
+    }
+ 
+    // HTTP needs CRLF
+    private static void println(PrintWriter pw, String s) {
+        pw.print(s + "\r\n");
+    }
+ 
+    private static Object getStaticFieldValue(Class<?> clazz, String field) throws IllegalAccessException, NoSuchFieldException {
+        final Field declaredField = clazz.getDeclaredField(field);
+        declaredField.setAccessible(true);
+        return declaredField.get(null);
+    }
+ 
+    private static void setStaticFieldValue(Class<?> clazz, String field, Object value) throws IllegalAccessException, NoSuchFieldException {
+        final Field declaredField = clazz.getDeclaredField(field);
+        declaredField.setAccessible(true);
+        declaredField.set(null, value);
+    }
+ 
+    private static FieldValue withStaticField(Class<?> clazz, String field, Object value) throws NoSuchFieldException, IllegalAccessException {
+        return new FieldValue(clazz, field, value);
+    }
+ 
+    private static class FieldValue implements AutoCloseable {
+        private final Class<?> clazz;
+        private final String field;
+        private final Object oldValue;
+ 
+        private FieldValue(Class<?> clazz, String field, Object value) throws NoSuchFieldException, IllegalAccessException {
+            this.clazz = clazz;
+            this.field = field;
+            oldValue = getStaticFieldValue(clazz, field);
+            setStaticFieldValue(clazz, field, value);
+        }
+ 
+        @Override
+        public void close() throws Exception {
+            setStaticFieldValue(clazz, field, oldValue);
+        }
+    }
+ 
+    private <T extends Exception> ServletException assertSubmissionThrows(FileUploadAction<T> endpoint, int files, int other, int fileSize, Class<? extends T> expected) throws IOException {
+        final ServletException actual = testSubmission(endpoint, files, other, fileSize, expected);
+        assertEquals(actual.getCause().getClass(), expected);
+        return actual;
+    }
+ 
+    private <T extends Exception> void assertSubmissionOK(FileUploadAction<T> endpoint, int files, int other, int fileSize) throws IOException {
+        assertNull(testSubmission(endpoint, files, other, fileSize, null));
+    }
+ 
+    private <T extends Exception> ServletException testSubmission(FileUploadAction<T> endpoint, int files, int other, int fileSize, Class<? extends T> expected) throws IOException {
+        endpoint.setExpectedWrapped(expected);
+        final JenkinsRule.WebClient wc = j.createWebClient();
+        final URL url = wc.createCrumbedUrl(endpoint.getUrlName() + "/submitMultipart");
+        WebRequest request = new WebRequest(url, HttpMethod.POST);
+        final String boundary = "---------------------------" + System.nanoTime();
+        request.setAdditionalHeader("Content-Type", "multipart/form-data; boundary=" + boundary);
+        ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        writeMultipartFormDataBody(baos, boundary, files, other, fileSize);
+        request.setRequestBody(baos.toString());
+        wc.getPage(request);
+        return endpoint.getActual();
+    }
+ 
+    private static void writeMultipartFormDataBody(OutputStream os, String boundary, int files, int other, int fileSize) throws IOException {
+        try (OutputStreamWriter osw = new OutputStreamWriter(os, StandardCharsets.UTF_8); PrintWriter pw = new PrintWriter(osw)) {
+            final Random random = new Random();
+            while (files + other > 0) {
+                println(pw, "--" + boundary);
+                if (files > other) {
+                    println(pw, "Content-Disposition: form-data; name=\"file" + files + "\"; filename=\"file" + files + "\"");
+                    println(pw, "Content-Type: application/octet-stream");
+                    println(pw, "");
+                    pw.flush();
+                    byte[] content = new byte[fileSize];
+                    random.nextBytes(content);
+                    os.write(content);
+                    os.flush();
+                    println(pw, "");
+                    files--;
+                } else {
+                    println(pw, "Content-Disposition: form-data; name=\"field" + other + "\"");
+                    println(pw, "");
+                    println(pw, "Value " + random.nextLong());
+                    other--;
+                }
+            }
+            println(pw, "--" + boundary + "--");
+        }
+    }
+ 
+    public abstract static class FileUploadAction<T extends Exception> implements RootAction {
+        protected Class<? extends T> expectedWrapped;
+        protected Throwable actualWrapped;
+        protected ServletException actual;
+ 
+        @Override
+        public String getIconFileName() {
+            return null;
+        }
+ 
+        @Override
+        public String getDisplayName() {
+            return null;
+        }
+ 
+        @Override
+        public String getUrlName() {
+            return getClass().getSimpleName();
+        }
+ 
+        @POST
+        public HttpResponse doSubmitMultipart(StaplerRequest req) throws FileUploadException, ServletException, IOException {
+            if (expectedWrapped == null) {
+                actualWrapped = null;
+                actual = null;
+                return processMultipartAndUnwrap(req);
+            } else {
+                actualWrapped = Assert.assertThrows(expectedWrapped, () -> processMultipartAndUnwrap(req));
+                return HttpResponses.ok();
+            }
+        }
+ 
+        private HttpResponse processMultipartAndUnwrap(StaplerRequest req) throws FileUploadException, ServletException, IOException {
+            try {
+                return processMultipart(req);
+            } catch (ServletException ex) {
+                // unwrap
+                actual = ex;
+                final Throwable cause = ex.getCause();
+                if (cause instanceof FileUploadException) {
+                    throw (FileUploadException) cause;
+                }
+                throw ex;
+            }
+        }
+ 
+        protected abstract HttpResponse processMultipart(StaplerRequest req) throws ServletException, IOException;
+ 
+        public void setExpectedWrapped(Class<? extends T> expectedWrapped) {
+            this.expectedWrapped = expectedWrapped;
+        }
+ 
+        public Throwable getActualWrapped() {
+            return actualWrapped;
+        }
+ 
+        public ServletException getActual() {
+            return actual;
+        }
+    }
+ 
+    @TestExtension
+    public static class StaplerRequestFormAction extends FileUploadAction<FileUploadException> {
+        public static StaplerRequestFormAction instance() {
+            return ExtensionList.lookupSingleton(StaplerRequestFormAction.class);
+        }
+ 
+        protected HttpResponse processMultipart(StaplerRequest req) throws ServletException, IOException {
+            req.getFileItem("any-name");
+            return HttpResponses.ok();
+        }
+    }
+ 
+    @TestExtension
+    public static class MultipartFormDataParserAction extends FileUploadAction<FileUploadException>  {
+        public static MultipartFormDataParserAction instance() {
+            return ExtensionList.lookupSingleton(MultipartFormDataParserAction.class);
+        }
+ 
+        protected HttpResponse processMultipart(StaplerRequest req) throws ServletException {
+            new MultipartFormDataParser(req);
+            return HttpResponses.ok();
+        }
+    }
+}