Skip to content

Commit 6b84024

Browse files
Andra Maria Puscasraul-arabaolaza
Andra Maria Puscas
authored and
raul-arabaolaza
committed
SECURITY-3303
1 parent 9c72463 commit 6b84024

File tree

3 files changed

+150
-5
lines changed

3 files changed

+150
-5
lines changed

‎pom.xml

+16
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,22 @@
9494
<artifactId>workflow-durable-task-step</artifactId>
9595
<scope>test</scope>
9696
</dependency>
97+
<dependency>
98+
<groupId>org.testcontainers</groupId>
99+
<artifactId>testcontainers</artifactId>
100+
<version>1.18.3</version>
101+
<scope>test</scope>
102+
</dependency>
103+
<dependency>
104+
<groupId>org.kohsuke</groupId>
105+
<artifactId>access-modifier-suppressions</artifactId>
106+
<version>${access-modifier-checker.version}</version>
107+
</dependency>
108+
<dependency>
109+
<groupId>org.kohsuke</groupId>
110+
<artifactId>access-modifier-annotation</artifactId>
111+
<version>1.33</version>
112+
</dependency>
97113
</dependencies>
98114
<dependencyManagement>
99115
<dependencies>

‎src/main/java/htmlpublisher/HtmlPublisher.java

+28-4
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@
3535
import java.io.Reader;
3636
import java.nio.charset.Charset;
3737
import java.nio.charset.StandardCharsets;
38+
import java.nio.file.LinkOption;
39+
import java.nio.file.OpenOption;
3840
import java.security.MessageDigest;
3941
import java.security.NoSuchAlgorithmException;
4042
import java.util.ArrayList;
@@ -44,8 +46,11 @@
4446
import java.util.List;
4547

4648
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
49+
import hudson.util.DirScanner;
50+
import jenkins.util.SystemProperties;
4751
import org.kohsuke.accmod.Restricted;
4852
import org.kohsuke.accmod.restrictions.NoExternalUse;
53+
import org.kohsuke.accmod.restrictions.suppressions.SuppressRestrictedWarnings;
4954
import org.kohsuke.stapler.AncestorInPath;
5055
import org.kohsuke.stapler.DataBoundConstructor;
5156
import org.kohsuke.stapler.QueryParameter;
@@ -84,6 +89,12 @@
8489
* @author Mike Rooney
8590
*/
8691
public class HtmlPublisher extends Recorder {
92+
93+
/**
94+
* Restores old behavior before SECURITY-3303
95+
*/
96+
@SuppressFBWarnings(value = "MS_SHOULD_BE_FINAL", justification = "Yes it should, but this allows the ability to change it via script in runtime.")
97+
static /*almost final*/ boolean FOLLOW_SYMLINKS = SystemProperties.getBoolean(HtmlPublisher.class.getName() + ".FOLLOW_SYMLINKS", false);
8798
private final List<HtmlPublisherTarget> reportTargets;
8899

89100
private static final String HEADER = "/htmlpublisher/HtmlPublisher/header.html";
@@ -238,8 +249,13 @@ public static boolean publishReports(Run<?, ?> build, FilePath workspace, TaskLi
238249
// We are only keeping one copy at the project level, so remove the old one.
239250
targetDir.deleteRecursive();
240251
}
241-
242-
if (archiveDir.copyRecursiveTo(reportTarget.getIncludes(), targetDir) == 0) {
252+
int copied = 0;
253+
if (FOLLOW_SYMLINKS) {
254+
copied = archiveDir.copyRecursiveTo(reportTarget.getIncludes(), targetDir);
255+
} else {
256+
copied = archiveDir.copyRecursiveTo(dirScannerGlob(reportTarget.getIncludes(), null, true, LinkOption.NOFOLLOW_LINKS), targetDir, reportTarget.getIncludes());
257+
}
258+
if (copied == 0) {
243259
if (!allowMissing) {
244260
listener.error("Directory '" + archiveDir + "' exists but failed copying to '" + targetDir + "'.");
245261
final Result buildResult = build.getResult();
@@ -252,8 +268,10 @@ public static boolean publishReports(Run<?, ?> build, FilePath workspace, TaskLi
252268
continue;
253269
}
254270
}
255-
} catch (IOException e) {
256-
Util.displayIOException(e, listener);
271+
} catch (Exception e) {
272+
if (e instanceof IOException) {
273+
Util.displayIOException((IOException) e, listener);
274+
}
257275
e.printStackTrace(listener.fatalError("HTML Publisher failure"));
258276
build.setResult(Result.FAILURE);
259277
return true;
@@ -367,6 +385,12 @@ public Collection<? extends Action> getProjectActions(AbstractProject<?, ?> proj
367385
}
368386
}
369387

388+
389+
@SuppressRestrictedWarnings(NoExternalUse.class)
390+
public static DirScanner dirScannerGlob(String includes, String excludes, boolean useDefaultExcludes, OpenOption... openOptions) throws Exception {
391+
return new DirScanner.Glob(includes, excludes, useDefaultExcludes, openOptions);
392+
}
393+
370394
@Extension
371395
public static class DescriptorImpl extends BuildStepDescriptor<Publisher> {
372396
@Override

‎src/test/java/htmlpublisher/HtmlPublisherIntegrationTest.java

+106-1
Original file line numberDiff line numberDiff line change
@@ -5,27 +5,63 @@
55
import hudson.Launcher;
66
import hudson.model.*;
77
import hudson.model.queue.QueueTaskFuture;
8+
import hudson.remoting.VirtualChannel;
9+
import hudson.slaves.DumbSlave;
810
import hudson.slaves.EnvironmentVariablesNodeProperty;
11+
import hudson.slaves.JNLPLauncher;
12+
import hudson.slaves.RetentionStrategy;
13+
import jenkins.MasterToSlaveFileCallable;
14+
import org.apache.commons.io.FileUtils;
15+
import org.junit.After;
916
import org.junit.Rule;
1017
import org.junit.Test;
18+
import org.jvnet.hudson.test.BuildWatcher;
19+
import org.jvnet.hudson.test.Issue;
1120
import org.jvnet.hudson.test.JenkinsRule;
21+
import org.jvnet.hudson.test.TemporaryDirectoryAllocator;
1222
import org.jvnet.hudson.test.TestBuilder;
23+
import org.testcontainers.DockerClientFactory;
24+
import org.testcontainers.containers.GenericContainer;
1325

1426
import java.io.File;
1527
import java.io.IOException;
28+
import java.nio.charset.StandardCharsets;
1629
import java.nio.file.Files;
30+
import java.nio.file.Paths;
1731
import java.util.ArrayList;
1832
import java.util.Arrays;
1933
import java.util.List;
34+
import java.util.Map;
2035

21-
import static org.junit.Assert.*;
36+
import static org.hamcrest.MatcherAssert.assertThat;
37+
import static org.hamcrest.collection.IsEmptyCollection.empty;
38+
import static org.hamcrest.core.IsNot.not;
39+
import static org.junit.Assert.assertEquals;
40+
import static org.junit.Assert.assertFalse;
41+
import static org.junit.Assert.assertTrue;
42+
import static org.junit.Assert.assertNotNull;
43+
import static org.junit.Assume.assumeTrue;
2244

2345
/**
2446
* @author Kohsuke Kawaguchi
2547
*/
2648
public class HtmlPublisherIntegrationTest {
2749
@Rule
2850
public JenkinsRule j = new JenkinsRule();
51+
@Rule
52+
public BuildWatcher buildWatcher = new BuildWatcher();
53+
54+
public TemporaryDirectoryAllocator tmp = new TemporaryDirectoryAllocator();
55+
private GenericContainer agentContainer;
56+
private DumbSlave agent;
57+
58+
@After
59+
public void dispose() throws IOException, InterruptedException {
60+
tmp.dispose();
61+
if (agentContainer != null) {
62+
agentContainer.stop();
63+
}
64+
}
2965

3066
/**
3167
* Makes sure that the configuration survives the round trip.
@@ -84,6 +120,39 @@ public boolean perform(AbstractBuild<?, ?> build, Launcher launcher,
84120
assertFalse(tab2Files.contains("dummy.html"));
85121
}
86122

123+
@Test @Issue("SECURITY-3303")
124+
public void testNotFollowingSymlinks() throws Exception {
125+
createDockerAgent();
126+
final File directoryOnController = tmp.allocate();
127+
FileUtils.write(new File(directoryOnController, "test.txt"), "test", StandardCharsets.UTF_8);
128+
final String directoryOnControllerPath = directoryOnController.getAbsolutePath();
129+
FreeStyleProject p = j.createFreeStyleProject();
130+
p.getBuildersList().add(new TestBuilder() {
131+
@Override
132+
public boolean perform(AbstractBuild<?, ?> build, Launcher launcher, BuildListener listener) throws InterruptedException, IOException {
133+
FilePath workspace = build.getWorkspace();
134+
workspace.act(new MakeSymlink(directoryOnControllerPath));
135+
workspace.child("test3.txt").write("Hello", "UTF-8");
136+
return true;
137+
}
138+
});
139+
HtmlPublisherTarget target1 = new HtmlPublisherTarget("tab1", "", "tab1/test.txt,tab1/test2.txt,**/test3.txt", true, false, true);
140+
p.getPublishersList().add(new HtmlPublisher(Arrays.asList(target1)));
141+
p.setAssignedLabel(Label.get("agent"));
142+
FreeStyleBuild build = j.buildAndAssertSuccess(p);
143+
File base = new File(build.getRootDir(), "htmlreports");
144+
String[] list = base.list();
145+
assertNotNull(list);
146+
assertThat(Arrays.asList(list), not(empty()));
147+
File tab1 = new File(base, "tab1");
148+
list = tab1.list();
149+
assertNotNull(list);
150+
assertThat(Arrays.asList(list), not(empty()));
151+
152+
File reports = new File(tab1, "tab1");
153+
assertFalse(reports.exists());
154+
}
155+
87156
@Test
88157
public void testVariableExpansion() throws Exception {
89158
FreeStyleProject p = j.createFreeStyleProject("variable_job");
@@ -246,5 +315,41 @@ private void addEnvironmentVariable(String key, String value) {
246315
j.jenkins.getGlobalNodeProperties().add(prop);
247316
}
248317

318+
private void createDockerAgent() throws Exception {
319+
assumeTrue("Needs Docker", DockerClientFactory.instance().isDockerAvailable());
320+
j.jenkins.setSlaveAgentPort(0);
321+
int port = j.jenkins.getTcpSlaveAgentListener().getAdvertisedPort();
322+
synchronized (j.jenkins) {
323+
agent = new DumbSlave("dockeragentOne", "/home/jenkins/work", new JNLPLauncher(true));
324+
agent.setLabelString("agent");
325+
agent.setRetentionStrategy(RetentionStrategy.NOOP);
326+
j.jenkins.addNode(agent);
327+
}
328+
Map<String, String> env = Map.of("JENKINS_URL", JNLPLauncher.getInboundAgentUrl(),
329+
"JENKINS_SECRET", agent.getComputer().getJnlpMac(),
330+
"JENKINS_AGENT_NAME", agent.getNodeName(),
331+
"JENKINS_AGENT_WORKDIR", agent.getRemoteFS());
332+
System.out.println(env);
249333

334+
agentContainer = new GenericContainer<>("jenkins/inbound-agent:jdk" + System.getProperty("java.specification.version"))
335+
.withEnv(env)
336+
.withNetworkMode("host").withLogConsumer(outputFrame -> System.out.print(outputFrame.getUtf8String()));
337+
//agentContainer.getHost()
338+
agentContainer.start();
339+
j.waitOnline(agent);
340+
}
341+
342+
static class MakeSymlink extends MasterToSlaveFileCallable<Void> {
343+
final String target;
344+
345+
MakeSymlink(String target) {
346+
this.target = target;
347+
}
348+
349+
@Override
350+
public Void invoke(File f, VirtualChannel channel) throws IOException, InterruptedException {
351+
Files.createSymbolicLink(Paths.get(f.getAbsolutePath(), "tab1"), Paths.get(target));
352+
return null;
353+
}
354+
}
250355
}

0 commit comments

Comments
 (0)