From f06bb8820115996397a2e0010b003ecbf0570865 Mon Sep 17 00:00:00 2001 From: Kevin-CB Date: Wed, 6 Sep 2023 11:17:31 +0000 Subject: [PATCH] [SECURITY-3073] --- bom/pom.xml | 2 +- .../hudson/util/MultipartFormDataParser.java | 12 ++- .../FileParameterValueSecurity3073Test.java | 81 +++++++++++++++++++ .../model/JenkinsSecurity3073Test.java | 77 ++++++++++++++++++ 4 files changed, 170 insertions(+), 2 deletions(-) create mode 100644 test/src/test/java/hudson/model/FileParameterValueSecurity3073Test.java create mode 100644 test/src/test/java/jenkins/model/JenkinsSecurity3073Test.java diff --git a/bom/pom.xml b/bom/pom.xml index 7f0162f63c..ffbb673acf 100644 --- a/bom/pom.xml +++ b/bom/pom.xml @@ -40,7 +40,7 @@ THE SOFTWARE. 9.5 2.0.7 - 1802.v9e2750160d01 + 1802.1804.va_8d30483a_7f7 2.4.21 diff --git a/core/src/main/java/hudson/util/MultipartFormDataParser.java b/core/src/main/java/hudson/util/MultipartFormDataParser.java index 762c0c35b6..ecc4ad292d 100644 --- 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 @@ import org.kohsuke.accmod.restrictions.NoExternalUse; * @author Kohsuke Kawaguchi */ public class MultipartFormDataParser implements AutoCloseable { - private final ServletFileUpload upload = new ServletFileUpload(new DiskFileItemFactory()); private final Map 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 new file mode 100644 index 0000000000..bcbbd40231 --- /dev/null +++ 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[] filesPermission = new Set[]{new HashSet<>()}; + await().pollInterval(250, TimeUnit.MILLISECONDS) + .atMost(10, TimeUnit.SECONDS) + .until(() -> { + Optional 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 new file mode 100644 index 0000000000..96c3fb3c63 --- /dev/null +++ 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[] filesPermission = new Set[]{new HashSet<>()}; + await().pollInterval(250, TimeUnit.MILLISECONDS) + .atMost(10, TimeUnit.SECONDS) + .until(() -> { + Optional 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]); + } +}