CVE-2024-28151

@@ -94,6 +94,22 @@
       <artifactId>workflow-durable-task-step</artifactId>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.testcontainers</groupId>
+      <artifactId>testcontainers</artifactId>
+      <version>1.18.3</version>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
+      <groupId>org.kohsuke</groupId>
+      <artifactId>access-modifier-suppressions</artifactId>
+      <version>${access-modifier-checker.version}</version>
+    </dependency>
+    <dependency>
+      <groupId>org.kohsuke</groupId>
+      <artifactId>access-modifier-annotation</artifactId>
+      <version>1.33</version>
+    </dependency>
   </dependencies>
   <dependencyManagement>
     <dependencies>

@@ -35,6 +35,8 @@
 import java.io.Reader;
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.LinkOption;
+import java.nio.file.OpenOption;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
 import java.util.ArrayList;
@@ -44,8 +46,11 @@
 import java.util.List;
 
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+import hudson.util.DirScanner;
+import jenkins.util.SystemProperties;
 import org.kohsuke.accmod.Restricted;
 import org.kohsuke.accmod.restrictions.NoExternalUse;
+import org.kohsuke.accmod.restrictions.suppressions.SuppressRestrictedWarnings;
 import org.kohsuke.stapler.AncestorInPath;
 import org.kohsuke.stapler.DataBoundConstructor;
 import org.kohsuke.stapler.QueryParameter;
@@ -84,6 +89,12 @@
  * @author Mike Rooney
  */
 public class HtmlPublisher extends Recorder {
+
+    /**
+     * Restores old behavior before SECURITY-3303
+     */
+    @SuppressFBWarnings(value = "MS_SHOULD_BE_FINAL", justification = "Yes it should, but this allows the ability to change it via script in runtime.")
+    static /*almost final*/ boolean FOLLOW_SYMLINKS = SystemProperties.getBoolean(HtmlPublisher.class.getName() + ".FOLLOW_SYMLINKS", false);
     private final List<HtmlPublisherTarget> reportTargets;
 
     private static final String HEADER = "/htmlpublisher/HtmlPublisher/header.html";
@@ -238,8 +249,13 @@ public static boolean publishReports(Run<?, ?> build, FilePath workspace, TaskLi
                     // We are only keeping one copy at the project level, so remove the old one.
                     targetDir.deleteRecursive();
                 }
-
-                if (archiveDir.copyRecursiveTo(reportTarget.getIncludes(), targetDir) == 0) {
+                int copied = 0;
+                if (FOLLOW_SYMLINKS) {
+                    copied = archiveDir.copyRecursiveTo(reportTarget.getIncludes(), targetDir);
+                } else {
+                    copied = archiveDir.copyRecursiveTo(dirScannerGlob(reportTarget.getIncludes(), null, true, LinkOption.NOFOLLOW_LINKS), targetDir, reportTarget.getIncludes());
+                }
+                if (copied == 0) {
                     if (!allowMissing) {
                         listener.error("Directory '" + archiveDir + "' exists but failed copying to '" + targetDir + "'.");
                         final Result buildResult = build.getResult();
@@ -252,8 +268,10 @@ public static boolean publishReports(Run<?, ?> build, FilePath workspace, TaskLi
                         continue;
                     }
                 }
-            } catch (IOException e) {
-                Util.displayIOException(e, listener);
+            } catch (Exception e) {
+                if (e instanceof IOException) {
+                    Util.displayIOException((IOException) e, listener);
+                }
                 e.printStackTrace(listener.fatalError("HTML Publisher failure"));
                 build.setResult(Result.FAILURE);
                 return true;
@@ -367,6 +385,12 @@ public Collection<? extends Action> getProjectActions(AbstractProject<?, ?> proj
         }
     }
 
+
+    @SuppressRestrictedWarnings(NoExternalUse.class)
+    public static DirScanner dirScannerGlob(String includes, String excludes, boolean useDefaultExcludes, OpenOption... openOptions) throws Exception {
+        return new DirScanner.Glob(includes, excludes, useDefaultExcludes, openOptions);
+    }
+
     @Extension
     public static class DescriptorImpl extends BuildStepDescriptor<Publisher> {
         @Override

@@ -5,27 +5,63 @@
 import hudson.Launcher;
 import hudson.model.*;
 import hudson.model.queue.QueueTaskFuture;
+import hudson.remoting.VirtualChannel;
+import hudson.slaves.DumbSlave;
 import hudson.slaves.EnvironmentVariablesNodeProperty;
+import hudson.slaves.JNLPLauncher;
+import hudson.slaves.RetentionStrategy;
+import jenkins.MasterToSlaveFileCallable;
+import org.apache.commons.io.FileUtils;
+import org.junit.After;
 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 org.jvnet.hudson.test.TemporaryDirectoryAllocator;
 import org.jvnet.hudson.test.TestBuilder;
+import org.testcontainers.DockerClientFactory;
+import org.testcontainers.containers.GenericContainer;
 
 import java.io.File;
 import java.io.IOException;
+import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
+import java.nio.file.Paths;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
+import java.util.Map;
 
-import static org.junit.Assert.*;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.collection.IsEmptyCollection.empty;
+import static org.hamcrest.core.IsNot.not;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assume.assumeTrue;
 
 /**
  * @author Kohsuke Kawaguchi
  */
 public class HtmlPublisherIntegrationTest {
     @Rule
     public JenkinsRule j = new JenkinsRule();
+    @Rule
+    public BuildWatcher buildWatcher = new BuildWatcher();
+
+    public TemporaryDirectoryAllocator tmp = new TemporaryDirectoryAllocator();
+    private GenericContainer agentContainer;
+    private DumbSlave agent;
+
+    @After
+    public void dispose() throws IOException, InterruptedException {
+        tmp.dispose();
+        if (agentContainer != null) {
+            agentContainer.stop();
+        }
+    }
 
     /**
      * Makes sure that the configuration survives the round trip.
@@ -84,6 +120,39 @@ public boolean perform(AbstractBuild<?, ?> build, Launcher launcher,
         assertFalse(tab2Files.contains("dummy.html"));
     }
 
+    @Test @Issue("SECURITY-3303")
+    public void testNotFollowingSymlinks() throws Exception {
+        createDockerAgent();
+        final File directoryOnController = tmp.allocate();
+        FileUtils.write(new File(directoryOnController, "test.txt"), "test", StandardCharsets.UTF_8);
+        final String directoryOnControllerPath = directoryOnController.getAbsolutePath();
+        FreeStyleProject p = j.createFreeStyleProject();
+        p.getBuildersList().add(new TestBuilder() {
+            @Override
+            public boolean perform(AbstractBuild<?, ?> build, Launcher launcher, BuildListener listener) throws InterruptedException, IOException {
+                FilePath workspace = build.getWorkspace();
+                workspace.act(new MakeSymlink(directoryOnControllerPath));
+                workspace.child("test3.txt").write("Hello", "UTF-8");
+                return true;
+            }
+        });
+        HtmlPublisherTarget target1 = new HtmlPublisherTarget("tab1", "", "tab1/test.txt,tab1/test2.txt,**/test3.txt", true, false, true);
+        p.getPublishersList().add(new HtmlPublisher(Arrays.asList(target1)));
+        p.setAssignedLabel(Label.get("agent"));
+        FreeStyleBuild build = j.buildAndAssertSuccess(p);
+        File base = new File(build.getRootDir(), "htmlreports");
+        String[] list = base.list();
+        assertNotNull(list);
+        assertThat(Arrays.asList(list), not(empty()));
+        File tab1 = new File(base, "tab1");
+        list = tab1.list();
+        assertNotNull(list);
+        assertThat(Arrays.asList(list), not(empty()));
+
+        File reports = new File(tab1, "tab1");
+        assertFalse(reports.exists());
+    }
+
     @Test
     public void testVariableExpansion() throws Exception {
         FreeStyleProject p = j.createFreeStyleProject("variable_job");
@@ -246,5 +315,41 @@ private void addEnvironmentVariable(String key, String value) {
         j.jenkins.getGlobalNodeProperties().add(prop);
     }
 
+    private void createDockerAgent() throws Exception {
+        assumeTrue("Needs Docker", DockerClientFactory.instance().isDockerAvailable());
+        j.jenkins.setSlaveAgentPort(0);
+        int port = j.jenkins.getTcpSlaveAgentListener().getAdvertisedPort();
+        synchronized (j.jenkins) {
+            agent = new DumbSlave("dockeragentOne", "/home/jenkins/work", new JNLPLauncher(true));
+            agent.setLabelString("agent");
+            agent.setRetentionStrategy(RetentionStrategy.NOOP);
+            j.jenkins.addNode(agent);
+        }
+        Map<String, String> env = Map.of("JENKINS_URL", JNLPLauncher.getInboundAgentUrl(),
+                "JENKINS_SECRET", agent.getComputer().getJnlpMac(),
+                "JENKINS_AGENT_NAME", agent.getNodeName(),
+                "JENKINS_AGENT_WORKDIR", agent.getRemoteFS());
+        System.out.println(env);
 
+        agentContainer = new GenericContainer<>("jenkins/inbound-agent:jdk" + System.getProperty("java.specification.version"))
+                .withEnv(env)
+                .withNetworkMode("host").withLogConsumer(outputFrame -> System.out.print(outputFrame.getUtf8String()));
+        //agentContainer.getHost()
+        agentContainer.start();
+        j.waitOnline(agent);
+    }
+
+    static class MakeSymlink extends MasterToSlaveFileCallable<Void> {
+        final String target;
+
+        MakeSymlink(String target) {
+            this.target = target;
+        }
+
+        @Override
+        public Void invoke(File f, VirtualChannel channel) throws IOException, InterruptedException {
+            Files.createSymbolicLink(Paths.get(f.getAbsolutePath(), "tab1"), Paths.get(target));
+            return null;
+        }
+    }
 }