CVE-2023-32981

@@ -27,8 +27,12 @@
 import hudson.FilePath;

 import jenkins.MasterToSlaveFileCallable;

 

+import java.io.File;

+import java.io.IOException;

+

 public abstract class AbstractFileCallable<T> extends MasterToSlaveFileCallable<T> {

     private FilePath destination;

+    private boolean allowExtractionOutsideDestination = false;

 

     public FilePath getDestination() {

         return destination;

@@ -37,4 +41,31 @@ public FilePath getDestination() {
     public void setDestination(FilePath destination) {

         this.destination = destination;

     }

+

+    /**

+     * SECURITY-2169 escape hatch.

+     * Controlled by {@link DecompressStepExecution#ALLOW_EXTRACTION_OUTSIDE_DESTINATION}.

+     *

+     * @return true if so.

+     */

+    public boolean isAllowExtractionOutsideDestination() {

+        return allowExtractionOutsideDestination;

+    }

+

+    public void setAllowExtractionOutsideDestination(boolean allowExtractionOutsideDestination) {

+        this.allowExtractionOutsideDestination = allowExtractionOutsideDestination;

+    }

+

+    protected boolean isDescendantOfDestination(FilePath f) throws IOException {

+        if (allowExtractionOutsideDestination) {

+            return true;

+        }

+        //Assumes destination and f is on the local host

+        if (destination == null) {

+            return false;

+        }

+        File dst = new File(destination.getRemote()).getCanonicalFile();

+        File child = new File(f.getRemote()).getCanonicalFile();

+        return child.toPath().startsWith(dst.toPath());

+    }

 }
\ No newline at end of file

@@ -25,8 +25,10 @@
 package org.jenkinsci.plugins.pipeline.utility.steps;

 

 import edu.umd.cs.findbugs.annotations.NonNull;

+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;

 import hudson.FilePath;

 import hudson.model.TaskListener;

+import jenkins.util.SystemProperties;

 import org.apache.commons.lang.StringUtils;

 import org.jenkinsci.plugins.workflow.steps.StepContext;

 import org.jenkinsci.plugins.workflow.steps.SynchronousNonBlockingStepExecution;

@@ -39,6 +41,13 @@
  * @author Robert Sandell <rsandell@cloudbees.com>.

  */

 public abstract class DecompressStepExecution extends SynchronousNonBlockingStepExecution<Object> {

+

+    /**

+     * SECURITY-2169 escape hatch.

+     */

+    @SuppressFBWarnings(value={"MS_SHOULD_BE_FINAL"}, justification="Non final so that an admin can adjust the value through the groovy script console without restarting the instance.")

+    public static /*almost final*/ boolean ALLOW_EXTRACTION_OUTSIDE_DESTINATION = SystemProperties.getBoolean(DecompressStepExecution.class.getName() + ".ALLOW_EXTRACTION_OUTSIDE_DESTINATION", false);

+

     private transient AbstractFileCallable<? extends Object> callable;

     private transient final AbstractFileDecompressStep step;

 

@@ -49,6 +58,9 @@ protected DecompressStepExecution(@NonNull AbstractFileDecompressStep step, @Non
 

     protected void setCallable(final AbstractFileCallable<? extends Object> callable) {

         this.callable = callable;

+        if (callable != null) {

+            callable.setAllowExtractionOutsideDestination(ALLOW_EXTRACTION_OUTSIDE_DESTINATION);

+        }

     }

 

     @Override

@@ -87,6 +99,12 @@ private Object test(TaskListener listener, FilePath workspace) throws IOExceptio
             listener.error(source.getRemote() + " is a directory.");

             return Boolean.FALSE;

         }

+        FilePath destination = workspace;

+        if (!StringUtils.isBlank(step.getDir())) {

+            destination = workspace.child(step.getDir());

+        }

+

+        callable.setDestination(destination);

         return source.act(callable);

     }

 }

@@ -40,6 +40,7 @@
 
 import java.io.File;
 import java.io.FileInputStream;
+import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
@@ -95,17 +96,36 @@ public Void invoke(File tarFile, VirtualChannel channel) throws IOException, Int
             PrintStream logger = listener.getLogger();
             boolean doGlob = !StringUtils.isBlank(glob);
 
-            InputStream fileStream = new FileInputStream(tarFile);
+            FileInputStream fileStream = new FileInputStream(tarFile);
 
+            FileChannel fileChannel = fileStream.getChannel();
+
+            byte[] signature = new byte[2];
             try {
-                //check if matches standard gzip magic number
-                fileStream = new GzipCompressorInputStream(fileStream);
+                int read = fileStream.read(signature);
+                fileChannel.position(0);
+                if (read <= 0) {
+                    logger.println("File is empty.");
+                }
             } catch (IOException exception) {
-                // Eat exception, may be not compressed file
+                fileStream.close();
+                throw new IOException("Error reading tar/tgz file: " + exception.getMessage(), exception);
+            } finally {
+                logger.flush();
+            }
+
+            InputStream inputStream = fileStream;
+            if(GzipCompressorInputStream.matches(signature, signature.length)) {
+                try {
+                    //check if matches standard gzip magic number
+                    inputStream = new GzipCompressorInputStream(fileStream);
+                } catch (IOException exception) {
+                    // Eat exception, may be not compressed file
+                }
             }
 
             getDestination().mkdirs();
-            try (TarArchiveInputStream tarStream = new TarArchiveInputStream(fileStream)) {
+            try (TarArchiveInputStream tarStream = new TarArchiveInputStream(inputStream)) {
                 logger.println("Extracting from " + tarFile.getAbsolutePath());
                 TarArchiveEntry entry;
                 Integer fileCount = 0;
@@ -115,6 +135,9 @@ public Void invoke(File tarFile, VirtualChannel channel) throws IOException, Int
                     }
 
                     FilePath f = getDestination().child(entry.getName());
+                    if (!isDescendantOfDestination(f)) {
+                        throw new FileNotFoundException(f.getRemote() + " is out of bounds!");
+                    }
                     if (entry.isDirectory()) {
                         f.mkdirs();
                     } else {
@@ -151,7 +174,7 @@ boolean matches(String path, String glob) {
     }
 
     /**
-     * Performs a test of a tar file on the slave where the file is.
+     * Performs a test of a tar file on the agent where the file is.
      */
     static class TestTarFileCallable extends AbstractFileCallable<Boolean> {
         private TaskListener listener;
@@ -205,6 +228,14 @@ public Boolean invoke(File f, VirtualChannel channel) throws IOException, Interr
                     if (!entry.isCheckSumOK()) {
                         throw new IOException("Not a tar archive");
                     }
+                    FilePath destination = getDestination();
+                    if (destination != null) {
+                        FilePath ef = destination.child(entry.getName());
+                        if (!isDescendantOfDestination(ef)) {
+                            listener.error(ef.getRemote() + " is out of bounds!");
+                            return false;
+                        }
+                    }
                 }
             } catch (IOException exception) {
                 listener.error("Error validating tar file: " + exception.getMessage());

@@ -36,6 +36,7 @@
 import org.jenkinsci.plugins.workflow.steps.StepContext;
 
 import java.io.File;
+import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
@@ -114,6 +115,9 @@ public Map<String, String> invoke(File zipFile, VirtualChannel channel) throws I
                         continue;
                     }
                     FilePath f = getDestination().child(entry.getName());
+                    if (!isDescendantOfDestination(f)) {
+                        throw new FileNotFoundException(f.getRemote() + " is out of bounds!");
+                    }
                     if (entry.isDirectory()) {
                         if (!read) {
                             f.mkdirs();
@@ -191,6 +195,14 @@ public Boolean invoke(File f, VirtualChannel channel) throws IOException, Interr
 
                     ZipEntry entry = entries.nextElement();
                     if (!entry.isDirectory()) {
+                        FilePath destination = getDestination();
+                        if (destination != null) {
+                            FilePath ef = destination.child(entry.getName());
+                            if (!isDescendantOfDestination(ef)) {
+                                listener.error(ef.getRemote() + " is out of bounds!");
+                                return false;
+                            }
+                        }
                         try (InputStream inputStream = zip.getInputStream(entry)) {
                             int length;
                             while ((length = IOUtils.read(inputStream, buffer)) > 0) {

@@ -25,6 +25,8 @@
 package org.jenkinsci.plugins.pipeline.utility.steps.tar;
 
 import hudson.model.Label;
+import hudson.model.Result;
+import org.jenkinsci.plugins.pipeline.utility.steps.DecompressStepExecution;
 import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
 import org.jenkinsci.plugins.workflow.job.WorkflowJob;
 import org.jenkinsci.plugins.workflow.job.WorkflowRun;
@@ -33,14 +35,18 @@
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
+import org.jvnet.hudson.test.BuildWatcher;
+import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRule;
 
 import java.io.File;
 import java.net.URL;
 import java.net.URLDecoder;
+import java.nio.charset.StandardCharsets;
 
 import static org.jenkinsci.plugins.pipeline.utility.steps.FilenameTestsUtils.separatorsToSystemEscaped;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assume.assumeTrue;
 
 /**
  * Tests for {@link UnTarStep}.
@@ -51,6 +57,8 @@ public class UnTarStepTest {
 
     @Rule
     public JenkinsRule j = new JenkinsRule();
+    @Rule
+    public BuildWatcher watcher = new BuildWatcher();
 
     @Before
     public void setup() throws Exception {
@@ -273,4 +281,77 @@ public void untarKeepPermissions() throws Exception {
         WorkflowRun run = j.assertBuildStatusSuccess(p.scheduleBuild2(0));
         j.assertLogContains("Hello World!", run);
     }
+
+    @Test @Issue("SECURITY-2196")
+    public void testingAbsolutePathsShouldFail() throws Exception {
+        assumeTrue("Can only run in a gnu unix environment", File.pathSeparatorChar == ':');
+        WorkflowJob p = j.jenkins.createProject(WorkflowJob.class, "p");
+        URL resource = getClass().getResource("absolute.tar");
+        String tgz = new File(URLDecoder.decode(resource.getPath(), StandardCharsets.UTF_8)).getAbsolutePath().replace('\\', '/');
+        p.setDefinition(new CpsFlowDefinition(
+                "node {\n" +
+                        "  def result = untar file: '" + separatorsToSystemEscaped(tgz) + "', test: true\n" +
+                        "  if (result)\n" +
+                        "      error('Should be fail!')\n" +
+                        "}", true));
+        WorkflowRun run = j.buildAndAssertSuccess(p);
+        j.assertLogContains("is out of bounds!", run);
+    }
+
+    @Test @Issue("SECURITY-2196")
+    public void testingAbsolutePathsShouldNotFailWithEscapeHatch() throws Exception {
+        assumeTrue("Can only run in a gnu unix environment", File.pathSeparatorChar == ':');
+        try {
+            DecompressStepExecution.ALLOW_EXTRACTION_OUTSIDE_DESTINATION = true;
+            j.createOnlineSlave(Label.get("bbb"));
+            WorkflowJob p = j.jenkins.createProject(WorkflowJob.class, "p");
+            URL resource = getClass().getResource("absolute.tar");
+            String tgz = new File(URLDecoder.decode(resource.getPath(), StandardCharsets.UTF_8)).getAbsolutePath().replace('\\', '/');
+            p.setDefinition(new CpsFlowDefinition(
+                    "node('bbb') {\n" +
+                            "  def result = untar file: '" + separatorsToSystemEscaped(tgz) + "', test: true\n" +
+                            "  if (!result)\n" +
+                            "      error('Should not be fail!')\n" +
+                            "}", true));
+            WorkflowRun run = j.buildAndAssertSuccess(p);
+            j.assertLogNotContains("is out of bounds!", run);
+        } finally {
+            DecompressStepExecution.ALLOW_EXTRACTION_OUTSIDE_DESTINATION = false;
+        }
+    }
+
+    @Test @Issue("SECURITY-2196")
+    public void absolutePathsShouldFailBuild() throws Exception {
+        assumeTrue("Can only run in a gnu unix environment", File.pathSeparatorChar == ':');
+        WorkflowJob p = j.jenkins.createProject(WorkflowJob.class, "p");
+        URL resource = getClass().getResource("absolute.tar");
+        String tgz = new File(URLDecoder.decode(resource.getPath(), StandardCharsets.UTF_8)).getAbsolutePath().replace('\\', '/');
+        p.setDefinition(new CpsFlowDefinition(
+                "node {\n" +
+                        "  untar file: '" + separatorsToSystemEscaped(tgz) + "'\n" +
+                        "}", true));
+        WorkflowRun run = j.buildAndAssertStatus(Result.FAILURE, p);
+        j.assertLogContains("is out of bounds!", run);
+    }
+
+    @Test
+    @Issue("SECURITY-2196")
+    public void absolutePathsShouldNotFailBuildWithEscapeHatch() throws Exception {
+        assumeTrue("Can only run in a gnu unix environment", File.pathSeparatorChar == ':');
+        try {
+            DecompressStepExecution.ALLOW_EXTRACTION_OUTSIDE_DESTINATION = true;
+
+            WorkflowJob p = j.jenkins.createProject(WorkflowJob.class, "p");
+            URL resource = getClass().getResource("absolute.tar");
+            String tgz = new File(URLDecoder.decode(resource.getPath(), StandardCharsets.UTF_8)).getAbsolutePath().replace('\\', '/');
+            p.setDefinition(new CpsFlowDefinition(
+                    "node {\n" +
+                            "  untar file: '" + separatorsToSystemEscaped(tgz) + "'\n" +
+                            "}", true));
+            WorkflowRun run = j.buildAndAssertStatus(Result.SUCCESS, p);
+            j.assertLogNotContains("is out of bounds!", run);
+        } finally {
+            DecompressStepExecution.ALLOW_EXTRACTION_OUTSIDE_DESTINATION = false;
+        }
+    }
 }

@@ -24,7 +24,11 @@
 
 package org.jenkinsci.plugins.pipeline.utility.steps.zip;
 
+import hudson.Functions;
 import hudson.model.Label;
+import hudson.model.Result;
+import hudson.model.queue.QueueTaskFuture;
+import org.jenkinsci.plugins.pipeline.utility.steps.DecompressStepExecution;
 import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
 import org.jenkinsci.plugins.workflow.job.WorkflowJob;
 import org.jenkinsci.plugins.workflow.job.WorkflowRun;
@@ -33,14 +37,17 @@
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
+import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRule;
 
 import java.io.File;
+import java.io.IOException;
 import java.net.URL;
 import java.net.URLDecoder;
 
 import static org.jenkinsci.plugins.pipeline.utility.steps.FilenameTestsUtils.separatorsToSystemEscaped;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assume.assumeTrue;
 
 /**
  * Tests for {@link UnZipStep}.
@@ -172,7 +179,7 @@ public void globReadingMore() throws Exception {
 
     @Test
     public void zipTest() throws Exception {
-        Assume.assumeTrue("Can only run in a gnu unix environment", File.pathSeparatorChar == ':');
+        assumeTrue("Can only run in a gnu unix environment", File.pathSeparatorChar == ':');
         WorkflowJob p = j.jenkins.createProject(WorkflowJob.class, "p");
         p.setDefinition(new CpsFlowDefinition(
                 "node('slaves') {\n" +
@@ -279,4 +286,65 @@ public void unzipQuietReading() throws Exception {
         j.assertLogContains("Read: 2 files", run);
         j.assertLogContains("Text: Hello World!", run);
     }
+
+    @Test @Issue("SECURITY-2196")
+    public void unZipMaliciousFailsTheTest() throws Exception {
+        assumeTrue("Can only run in a gnu unix environment", File.pathSeparatorChar == ':');
+        /*
+         This test uses a prepared zip file with a malicious payload.
+         */
+        WorkflowJob p = j.jenkins.createProject(WorkflowJob.class, "p");
+        URL resource = getClass().getResource("malicious.zip");
+        String zip = new File(URLDecoder.decode(resource.getPath(), "UTF-8")).getAbsolutePath().replace('\\', '/');
+        p.setDefinition(new CpsFlowDefinition(
+                "node {\n" +
+                        "  def result = unzip zipFile: '" + separatorsToSystemEscaped(zip) + "', test: true\n" +
+                        "  if (result)\n" +
+                        "      error('Should be failed!')\n" +
+                        "}", true));
+        WorkflowRun run = j.assertBuildStatusSuccess(p.scheduleBuild2(0));
+        j.assertLogContains("is out of bounds!", run);
+    }
+
+    @Test @Issue("SECURITY-2196")
+    public void unZipMaliciousDoesNotFailTheTestWithEscapeHath() throws Exception {
+        assumeTrue("Can only run in a gnu unix environment", File.pathSeparatorChar == ':');
+        try {
+            DecompressStepExecution.ALLOW_EXTRACTION_OUTSIDE_DESTINATION = true;
+        /*
+         This test uses a prepared zip file with a malicious payload.
+         */
+            j.createOnlineSlave(Label.get("bbb"));
+            WorkflowJob p = j.jenkins.createProject(WorkflowJob.class, "p");
+            URL resource = getClass().getResource("malicious.zip");
+            String zip = new File(URLDecoder.decode(resource.getPath(), "UTF-8")).getAbsolutePath().replace('\\', '/');
+            p.setDefinition(new CpsFlowDefinition(
+                    "node('bbb') {\n" +
+                            "  def result = unzip zipFile: '" + separatorsToSystemEscaped(zip) + "', test: true\n" +
+                            "  if (!result)\n" +
+                            "      error('Should not fail!')\n" +
+                            "}", true));
+            WorkflowRun run = j.assertBuildStatusSuccess(p.scheduleBuild2(0));
+            j.assertLogNotContains("is out of bounds!", run);
+        } finally {
+            DecompressStepExecution.ALLOW_EXTRACTION_OUTSIDE_DESTINATION = false;
+        }
+    }
+
+    @Test @Issue("SECURITY-2196")
+    public void unZipMaliciousFailsTheBuild() throws Exception {
+        assumeTrue("Can only run in a gnu unix environment", File.pathSeparatorChar == ':');
+        /*
+         This test uses a prepared zip file with a malicious payload.
+         */
+        WorkflowJob p = j.jenkins.createProject(WorkflowJob.class, "p");
+        URL resource = getClass().getResource("malicious.zip");
+        String zip = new File(URLDecoder.decode(resource.getPath(), "UTF-8")).getAbsolutePath().replace('\\', '/');
+        p.setDefinition(new CpsFlowDefinition(
+                "node {\n" +
+                        "  unzip zipFile: '" + separatorsToSystemEscaped(zip) + "'\n" +
+                        "}", true));
+        WorkflowRun run = j.buildAndAssertStatus(Result.FAILURE, p);
+        j.assertLogContains("is out of bounds!", run);
+    }
 }