Security context
High· 7.0GHSA-55wp-3pq4-w8p9 CVE-2023-43496CWE-276Published Sep 20, 2023

Jenkins temporary plugin file created with insecure permissions

Research this vulnerability

Research is free — Hunters explains how the bug works, the root-cause code pattern, how the fix addresses it, and how to test whether a target is affected, in chat. Investigate & write exploit is a paid run — the engine reads the advisory and fix commits, then builds and validates a working proof-of-concept exploit with reproduction steps.

Affected versions

2.50 → fixed in 2.414.22.415 → fixed in 2.424

Details

Jenkins creates a temporary file when a plugin is deployed directly from a URL. Jenkins 2.423 and earlier, LTS 2.414.1 and earlier creates this temporary file in the system temporary directory with the default permissions for newly created files. If these permissions are overly permissive, they may allow attackers with access to the Jenkins controller file system to read and write the file before it is installed in Jenkins, potentially resulting in arbitrary code execution. This vulnerability only affects operating systems using a shared temporary directory for all users (typically Linux). Additionally, the default permissions for newly created files generally only allow attackers to read the temporary file, but not write to it. This issue complements SECURITY-2823, which affected plugins uploaded from an administrator’s computer. Jenkins 2.424, LTS 2.414.2 creates the temporary file in a subdirectory with more restrictive permissions. As a workaround, you can change your default temporary-file directory using the Java system property java.io.tmpdir, if you’re concerned about this issue but unable to immediately update Jenkins.

The fix

[SECURITY-3072]

Kevin-CB· Sep 6, 2023, 11:17 AM+12827df7c4ccda8
core/src/main/java/hudson/PluginManager.java+2 19
@@ -28,8 +28,6 @@
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;
@@ -83,7 +81,6 @@
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;
@@ -91,10 +88,8 @@
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;
@@ -1883,16 +1878,6 @@ 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");
}
@@ -1902,7 +1887,8 @@ public HttpResponse doUploadPlugin(StaplerRequest req) throws IOException, Servl
}
// first copy into a temporary file name
- File t = File.createTempFile("uploaded", ".jpi");
+ File t = File.createTempFile("uploaded", ".jpi", tmpDir);
+ tmpDir.deleteOnExit();
t.deleteOnExit();
// TODO Remove this workaround after FILEUPLOAD-293 is resolved.
Files.delete(Util.fileToPath(t));
@@ -1913,9 +1899,6 @@ 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/PluginManagerSecurity3072Test.java+107 0
@@ -0,0 +1,107 @@
+package hudson;
+
+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.Assert.assertTrue;
+import static org.junit.Assume.assumeFalse;
+
+import hudson.model.RootAction;
+import java.io.File;
+import java.io.IOException;
+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 javax.servlet.ServletException;
+import jenkins.model.Jenkins;
+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;
+import org.jvnet.hudson.test.TestExtension;
+import org.kohsuke.stapler.StaplerRequest;
+import org.kohsuke.stapler.StaplerResponse;
+
+public class PluginManagerSecurity3072Test {
+
+ @Rule
+ public JenkinsRule r = PluginManagerUtil.newJenkinsRule();
+
+ @Rule
+ public TemporaryFolder tmp = new TemporaryFolder();
+
+ @Test
+ @Issue("SECURITY-3072")
+ public void verifyUploadedPluginFromURLPermission() throws Exception {
+ assumeFalse(Functions.isWindows());
+
+ HtmlPage page = r.createWebClient().goTo("pluginManager/advanced");
+ HtmlForm f = page.getFormByName("uploadPlugin");
+ f.getInputByName("pluginUrl").setValue(Jenkins.get().getRootUrl() + "pluginManagerGetPlugin/htmlpublisher.jpi");
+ r.submit(f);
+
+ 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> lastUploadedPluginDir = Arrays.stream(Objects.requireNonNull(
+ filesTmpDir.listFiles((file, fileName) ->
+ fileName.startsWith("uploadDir")))).
+ max(Comparator.comparingLong(File::lastModified));
+ if (lastUploadedPluginDir.isPresent()) {
+ filesPermission[0] = Files.getPosixFilePermissions(lastUploadedPluginDir.get().toPath(), LinkOption.NOFOLLOW_LINKS);
+ Optional<File> pluginFile = Arrays.stream(Objects.requireNonNull(
+ lastUploadedPluginDir.get().listFiles((file, fileName) ->
+ fileName.startsWith("uploaded")))).
+ max(Comparator.comparingLong(File::lastModified));
+ assertTrue(pluginFile.isPresent());
+ return true;
+ } else {
+ return false;
+ }
+ });
+ assertEquals(EnumSet.of(OWNER_EXECUTE, OWNER_READ, OWNER_WRITE), filesPermission[0]);
+ }
+
+ @TestExtension("verifyUploadedPluginFromURLPermission")
+ public static final class ReturnPluginJpiAction implements RootAction {
+
+ @Override
+ public String getIconFileName() {
+ return "gear2.png";
+ }
+
+ @Override
+ public String getDisplayName() {
+ return "URL to retrieve a plugin jpi";
+ }
+
+ @Override
+ public String getUrlName() {
+ return "pluginManagerGetPlugin";
+ }
+
+ public void doDynamic(StaplerRequest staplerRequest, StaplerResponse staplerResponse) throws ServletException, IOException {
+ staplerResponse.setContentType("application/octet-stream");
+ staplerResponse.setStatus(200);
+ staplerResponse.serveFile(staplerRequest, PluginManagerTest.class.getClassLoader().getResource("plugins/htmlpublisher.jpi"));
+ }
+ }
+}
test/src/test/java/hudson/PluginManagerTest.java+19 8
@@ -24,6 +24,7 @@
package hudson;
+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;
@@ -173,7 +174,7 @@ public String getUrlName() {
}
public void doDynamic(StaplerRequest staplerRequest, StaplerResponse staplerResponse) throws ServletException, IOException {
- staplerResponse.setContentType("application/octet");
+ staplerResponse.setContentType("application/octet-stream");
staplerResponse.setStatus(200);
staplerResponse.serveFile(staplerRequest, PluginManagerTest.class.getClassLoader().getResource("plugins/htmlpublisher.jpi"));
}
@@ -743,24 +744,34 @@ public void verifyUploadedPluginPermission() throws Exception {
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").setValue(plugin.getAbsolutePath());
+ f.getInputByName("name").setValueAttribute(plugin.getAbsolutePath());
r.submit(f);
- File tmpDir = new File(File.createTempFile("tmp", ".tmp").getParent());
- tmpDir.deleteOnExit();
+ 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(tmpDir.listFiles((file, fileName) -> fileName.startsWith("uploaded")))).max(Comparator.comparingLong(File::lastModified));
- if (lastUploadedPlugin.isPresent()) {
- filesPermission[0] = Files.getPosixFilePermissions(lastUploadedPlugin.get().toPath(), LinkOption.NOFOLLOW_LINKS);
+ Optional<File> lastUploadedPluginDir = Arrays.stream(Objects.requireNonNull(
+ filesTmpDir.listFiles((file, fileName) ->
+ fileName.startsWith("uploadDir")))).
+ max(Comparator.comparingLong(File::lastModified));
+ if (lastUploadedPluginDir.isPresent()) {
+ filesPermission[0] = Files.getPosixFilePermissions(lastUploadedPluginDir.get().toPath(), LinkOption.NOFOLLOW_LINKS);
+ Optional<File> pluginFile = Arrays.stream(Objects.requireNonNull(
+ lastUploadedPluginDir.get().listFiles((file, fileName) ->
+ fileName.startsWith("uploaded")))).
+ max(Comparator.comparingLong(File::lastModified));
+ assertTrue(pluginFile.isPresent());
return true;
} else {
return false;
}
});
- assertEquals(EnumSet.of(OWNER_READ, OWNER_WRITE), filesPermission[0]);
+ assertEquals(EnumSet.of(OWNER_EXECUTE, OWNER_READ, OWNER_WRITE), filesPermission[0]);
}
@Test

References