[SECURITY-3073]

jenkinsci · Sep 6, 2023 · f06bb88 · f06bb88
1 parent df7c4cc
commit f06bb88
Show file tree

Hide file tree

Showing 4 changed files with 170 additions and 2 deletions.
diff --git a/bom/pom.xml b/bom/pom.xml
@@ -40,7 +40,7 @@ THE SOFTWARE.
   <properties>
     <asm.version>9.5</asm.version>
     <slf4jVersion>2.0.7</slf4jVersion>
-    <stapler.version>1802.v9e2750160d01</stapler.version>
+    <stapler.version>1802.1804.va_8d30483a_7f7</stapler.version>
     <groovy.version>2.4.21</groovy.version>
   </properties>
 

diff --git a/core/src/main/java/hudson/util/MultipartFormDataParser.java b/core/src/main/java/hudson/util/MultipartFormDataParser.java
@@ -25,6 +25,9 @@
 package hudson.util;
 
 import edu.umd.cs.findbugs.annotations.CheckForNull;
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
 import java.util.HashMap;
 import java.util.Map;
 import javax.servlet.ServletException;
@@ -46,7 +49,6 @@
  * @author Kohsuke Kawaguchi
  */
 public class MultipartFormDataParser implements AutoCloseable {
-    private final ServletFileUpload upload = new ServletFileUpload(new DiskFileItemFactory());
     private final Map<String, FileItem> byName = new HashMap<>();
 
     /**
@@ -74,6 +76,14 @@ public class MultipartFormDataParser implements AutoCloseable {
 
     @Restricted(NoExternalUse.class)
     public MultipartFormDataParser(HttpServletRequest request, int maxParts, long maxPartSize, long maxSize) throws ServletException {
+        File tmpDir;
+        try {
+            tmpDir = Files.createTempDirectory("jenkins-multipart-uploads").toFile();
+        } catch (IOException e) {
+            throw new ServletException("Error creating temporary directory", e);
+        }
+        tmpDir.deleteOnExit();
+        ServletFileUpload upload = new ServletFileUpload(new DiskFileItemFactory(DiskFileItemFactory.DEFAULT_SIZE_THRESHOLD, tmpDir));
         upload.setFileCountMax(maxParts);
         upload.setFileSizeMax(maxPartSize);
         upload.setSizeMax(maxSize);

diff --git a/test/src/test/java/hudson/model/FileParameterValueSecurity3073Test.java b/test/src/test/java/hudson/model/FileParameterValueSecurity3073Test.java
@@ -0,0 +1,81 @@
+package hudson.model;
+
+import static java.nio.file.attribute.PosixFilePermission.OWNER_EXECUTE;
+import static java.nio.file.attribute.PosixFilePermission.OWNER_READ;
+import static java.nio.file.attribute.PosixFilePermission.OWNER_WRITE;
+import static org.awaitility.Awaitility.await;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assume.assumeFalse;
+
+import hudson.Functions;
+import java.io.File;
+import java.nio.file.Files;
+import java.nio.file.LinkOption;
+import java.nio.file.attribute.PosixFilePermission;
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.EnumSet;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.Set;
+import java.util.concurrent.TimeUnit;
+import org.apache.commons.io.FileUtils;
+import org.htmlunit.html.HtmlForm;
+import org.htmlunit.html.HtmlPage;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.jvnet.hudson.test.Issue;
+import org.jvnet.hudson.test.JenkinsRule;
+
+public class FileParameterValueSecurity3073Test {
+
+    @Rule
+    public JenkinsRule j = new JenkinsRule();
+
+    @Rule
+    public TemporaryFolder tmp = new TemporaryFolder();
+
+    @Test
+    @Issue("SECURITY-3073")
+    public void verifyUploadedFileParameterPermission() throws Exception {
+        assumeFalse(Functions.isWindows());
+
+        FreeStyleProject project = j.createFreeStyleProject();
+        project.addProperty(new ParametersDefinitionProperty(List.of(
+                new FileParameterDefinition("filePermission", null)
+        )));
+        File dir = tmp.newFolder();
+        File plugin = new File(dir, "htmlpublisher.jpi");
+        // We're using a plugin to have a file above DiskFileItemFactory.DEFAULT_SIZE_THRESHOLD
+        FileUtils.copyURLToFile(Objects.requireNonNull(getClass().getClassLoader().getResource("plugins/htmlpublisher.jpi")), plugin);
+
+        HtmlPage page = j.createWebClient().withThrowExceptionOnFailingStatusCode(false).goTo(project.getUrl() + "/build?delay=0sec");
+        HtmlForm form = page.getFormByName("parameters");
+        form.getInputByName("file").setValueAttribute(plugin.getAbsolutePath());
+        j.submit(form);
+
+        File filesRef = Files.createTempFile("tmp", ".tmp").toFile();
+        File filesTmpDir = filesRef.getParentFile();
+        filesRef.deleteOnExit();
+
+        final Set<PosixFilePermission>[] filesPermission = new Set[]{new HashSet<>()};
+        await().pollInterval(250, TimeUnit.MILLISECONDS)
+                .atMost(10, TimeUnit.SECONDS)
+                .until(() -> {
+                    Optional<File> lastUploadedPlugin = Arrays.stream(Objects.requireNonNull(
+                                    filesTmpDir.listFiles((file, fileName) ->
+                                            fileName.startsWith("jenkins-stapler-uploads")))).
+                                            max(Comparator.comparingLong(File::lastModified));
+                    if (lastUploadedPlugin.isPresent()) {
+                        filesPermission[0] = Files.getPosixFilePermissions(lastUploadedPlugin.get().toPath(), LinkOption.NOFOLLOW_LINKS);
+                        return true;
+                    } else {
+                        return false;
+                    }
+                });
+        assertEquals(EnumSet.of(OWNER_EXECUTE, OWNER_READ, OWNER_WRITE), filesPermission[0]);
+    }
+}
diff --git a/test/src/test/java/jenkins/model/JenkinsSecurity3073Test.java b/test/src/test/java/jenkins/model/JenkinsSecurity3073Test.java
@@ -0,0 +1,77 @@
+package jenkins.model;
+
+import static java.nio.file.attribute.PosixFilePermission.OWNER_EXECUTE;
+import static java.nio.file.attribute.PosixFilePermission.OWNER_READ;
+import static java.nio.file.attribute.PosixFilePermission.OWNER_WRITE;
+import static org.awaitility.Awaitility.await;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assume.assumeFalse;
+
+import hudson.Functions;
+import java.io.File;
+import java.nio.file.Files;
+import java.nio.file.LinkOption;
+import java.nio.file.attribute.PosixFilePermission;
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.EnumSet;
+import java.util.HashSet;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.Set;
+import java.util.concurrent.TimeUnit;
+import org.apache.commons.io.FileUtils;
+import org.htmlunit.html.HtmlForm;
+import org.htmlunit.html.HtmlPage;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.jvnet.hudson.test.Issue;
+import org.jvnet.hudson.test.JenkinsRule;
+
+public class JenkinsSecurity3073Test {
+
+
+    @Rule
+    public JenkinsRule j = new JenkinsRule();
+
+    @Rule
+    public TemporaryFolder tmp = new TemporaryFolder();
+
+    @Test
+    @Issue("SECURITY-3073")
+    public void verifyUploadedFingerprintFilePermission() throws Exception {
+        assumeFalse(Functions.isWindows());
+
+        HtmlPage page = j.createWebClient().goTo("fingerprintCheck");
+        // The form doesn't have a name, the page contain the search form and the one we're interested in
+        HtmlForm form = page.getForms().get(1);
+        File dir = tmp.newFolder();
+        File plugin = new File(dir, "htmlpublisher.jpi");
+        // We're using a plugin to have a file above DiskFileItemFactory.DEFAULT_SIZE_THRESHOLD
+        FileUtils.copyURLToFile(Objects.requireNonNull(getClass().getClassLoader().getResource("plugins/htmlpublisher.jpi")), plugin);
+        form.getInputByName("name").setValueAttribute(plugin.getAbsolutePath());
+        j.submit(form);
+
+        File filesRef = Files.createTempFile("tmp", ".tmp").toFile();
+        File filesTmpDir = filesRef.getParentFile();
+        filesRef.deleteOnExit();
+
+        final Set<PosixFilePermission>[] filesPermission = new Set[]{new HashSet<>()};
+        await().pollInterval(250, TimeUnit.MILLISECONDS)
+                .atMost(10, TimeUnit.SECONDS)
+                .until(() -> {
+                    Optional<File> lastUploadedPlugin = Arrays.stream(Objects.requireNonNull(
+                                    filesTmpDir.listFiles((file, fileName) ->
+                                            fileName.startsWith("jenkins-multipart-uploads")))).
+                                            max(Comparator.comparingLong(File::lastModified));
+                    if (lastUploadedPlugin.isPresent()) {
+                        filesPermission[0] = Files.getPosixFilePermissions(lastUploadedPlugin.get().toPath(), LinkOption.NOFOLLOW_LINKS);
+                        return true;
+                    } else {
+                        return false;
+                    }
+                });
+        assertEquals(EnumSet.of(OWNER_EXECUTE, OWNER_READ, OWNER_WRITE), filesPermission[0]);
+    }
+}