[SECURITY-2823]

core/src/main/java/hudson/PluginManager.java

@@ -28,6 +28,8 @@
 import static hudson.init.InitMilestone.PLUGINS_LISTED;
 import static hudson.init.InitMilestone.PLUGINS_PREPARED;
 import static hudson.init.InitMilestone.PLUGINS_STARTED;
+import static java.nio.file.attribute.PosixFilePermission.OWNER_READ;
+import static java.nio.file.attribute.PosixFilePermission.OWNER_WRITE;
 import static java.util.logging.Level.FINE;
 import static java.util.logging.Level.INFO;
 import static java.util.logging.Level.WARNING;
@@ -81,15 +83,18 @@
 import java.net.URLConnection;
 import java.net.http.HttpClient;
 import java.net.http.HttpRequest;
+import java.nio.file.FileSystems;
 import java.nio.file.Files;
 import java.nio.file.InvalidPathException;
 import java.nio.file.Paths;
 import java.nio.file.attribute.FileTime;
 import java.security.CodeSource;
 import java.time.Duration;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.EnumSet;
 import java.util.Enumeration;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -1860,7 +1865,8 @@ public HttpResponse doUploadPlugin(StaplerRequest req) throws IOException, Servl
 
             String fileName = "";
             PluginCopier copier;
-            ServletFileUpload upload = new ServletFileUpload(new DiskFileItemFactory());
+            File tmpDir = Files.createTempDirectory("uploadDir").toFile();
+            ServletFileUpload upload = new ServletFileUpload(new DiskFileItemFactory(DiskFileItemFactory.DEFAULT_SIZE_THRESHOLD, tmpDir));
             List<FileItem> items = upload.parseRequest(req);
             if (StringUtils.isNotBlank(items.get(1).getString())) {
                 // this is a URL deployment
@@ -1873,6 +1879,16 @@ public HttpResponse doUploadPlugin(StaplerRequest req) throws IOException, Servl
                 copier = new FileUploadPluginCopier(fileItem);
             }
 
+            if (FileSystems.getDefault().supportedFileAttributeViews().contains("posix")) {
+                Arrays.stream(Objects.requireNonNull(tmpDir.listFiles())).forEach((file -> {
+                    try {
+                        Files.setPosixFilePermissions(file.toPath(), EnumSet.of(OWNER_READ, OWNER_WRITE));
+                    } catch (IOException e) {
+                        throw new RuntimeException(e);
+                    }
+                }));
+            }
+
             if ("".equals(fileName)) {
                 return new HttpRedirect("advanced");
             }
@@ -1893,6 +1909,9 @@ public HttpResponse doUploadPlugin(StaplerRequest req) throws IOException, Servl
                 throw new ServletException(e);
             }
             copier.cleanup();
+            if (!tmpDir.delete()) {
+                System.err.println("Failed to delete temporary directory: " + tmpDir);
+            }
 
             final String baseName = identifyPluginShortName(t);
 

test/src/test/java/hudson/PluginManagerSecurity2823Test.java

@@ -0,0 +1,57 @@
+package hudson;
+
+import static java.nio.file.attribute.PosixFilePermission.OWNER_READ;
+import static java.nio.file.attribute.PosixFilePermission.OWNER_WRITE;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assume.assumeFalse;
+
+import com.gargoylesoftware.htmlunit.html.HtmlForm;
+import com.gargoylesoftware.htmlunit.html.HtmlPage;
+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.Objects;
+import java.util.Optional;
+import java.util.Set;
+import org.apache.commons.io.FileUtils;
+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 PluginManagerSecurity2823Test {
+
+    @Rule
+    public JenkinsRule r = PluginManagerUtil.newJenkinsRule();
+
+    @Rule
+    public TemporaryFolder tmp = new TemporaryFolder();
+
+    @Test
+    @Issue("SECURITY-2823")
+    public void verifyUploadedPluginPermission() throws Exception {
+        assumeFalse(Functions.isWindows());
+
+        HtmlPage page = r.createWebClient().goTo("pluginManager/advanced");
+        HtmlForm f = page.getFormByName("uploadPlugin");
+        File dir = tmp.newFolder();
+        File plugin = new File(dir, "htmlpublisher.jpi");
+        FileUtils.copyURLToFile(Objects.requireNonNull(getClass().getClassLoader().getResource("plugins/htmlpublisher.jpi")), plugin);
+        f.getInputByName("name").setValueAttribute(plugin.getAbsolutePath());
+        r.submit(f);
+
+        File tmpDir = Files.createTempFile("tmp", ".tmp").getParent().toFile();
+        tmpDir.deleteOnExit();
+
+        Optional<File> lastUploadedPlugin = Arrays.stream(Objects.requireNonNull(tmpDir.listFiles((file, fileName) -> fileName.startsWith("uploaded")))).max(Comparator.comparingLong(File::lastModified));
+        Set<PosixFilePermission> filesPermission = Files.getPosixFilePermissions(lastUploadedPlugin.get().toPath(), LinkOption.NOFOLLOW_LINKS);
+
+        assertEquals(EnumSet.of(OWNER_READ, OWNER_WRITE), filesPermission);
+    }
+
+}