From acc2da063e8510719950c694ae6490c201f1318b Mon Sep 17 00:00:00 2001 From: Josh Soref Date: Thu, 21 Mar 2019 11:29:20 -0400 Subject: [PATCH] [SECURITY-904] Fold tests back into normal files --- .../test/java/hudson/FilePathSEC904Test.java | 322 ---------- core/src/test/java/hudson/FilePathTest.java | 284 +++++++- core/src/test/java/hudson/UtilSEC904Test.java | 84 --- core/src/test/java/hudson/UtilTest.java | 39 ++ .../jenkins/util/VirtualFileSEC904Test.java | 281 -------- .../java/jenkins/util/VirtualFileTest.java | 233 +++++++ .../DirectoryBrowserSupportSEC904Test.java | 606 ------------------ .../model/DirectoryBrowserSupportTest.java | 541 +++++++++++++++- .../insideWorkspaceStructure.bat | 0 .../insideWorkspaceStructure.sh | 0 .../outsideWorkspaceStructure.bat | 0 .../outsideWorkspaceStructure.sh | 0 ...outsideWorkspaceStructureWithJunctions.bat | 0 13 files changed, 1088 insertions(+), 1302 deletions(-) delete mode 100644 core/src/test/java/hudson/FilePathSEC904Test.java delete mode 100644 core/src/test/java/hudson/UtilSEC904Test.java delete mode 100644 core/src/test/java/jenkins/util/VirtualFileSEC904Test.java delete mode 100644 test/src/test/java/hudson/model/DirectoryBrowserSupportSEC904Test.java rename test/src/test/resources/hudson/model/{DirectoryBrowserSupportSEC904Test => DirectoryBrowserSupportTest}/insideWorkspaceStructure.bat (100%) rename test/src/test/resources/hudson/model/{DirectoryBrowserSupportSEC904Test => DirectoryBrowserSupportTest}/insideWorkspaceStructure.sh (100%) rename test/src/test/resources/hudson/model/{DirectoryBrowserSupportSEC904Test => DirectoryBrowserSupportTest}/outsideWorkspaceStructure.bat (100%) rename test/src/test/resources/hudson/model/{DirectoryBrowserSupportSEC904Test => DirectoryBrowserSupportTest}/outsideWorkspaceStructure.sh (100%) rename test/src/test/resources/hudson/model/{DirectoryBrowserSupportSEC904Test => DirectoryBrowserSupportTest}/outsideWorkspaceStructureWithJunctions.bat (100%) diff --git a/core/src/test/java/hudson/FilePathSEC904Test.java b/core/src/test/java/hudson/FilePathSEC904Test.java deleted file mode 100644 index 9f50c6bb50..0000000000 --- a/core/src/test/java/hudson/FilePathSEC904Test.java +++ /dev/null @@ -1,322 +0,0 @@ -/* - * The MIT License - * - * Copyright (c) 2004-2010, Sun Microsystems, Inc., Kohsuke Kawaguchi, Alan Harder - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN - * THE SOFTWARE. - */ -package hudson; - -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.TemporaryFolder; -import org.jvnet.hudson.test.Issue; - -import java.io.File; -import java.io.IOException; -import java.nio.charset.StandardCharsets; -import java.util.concurrent.TimeUnit; - -import static org.hamcrest.CoreMatchers.is; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertThat; -import static org.junit.Assert.assertTrue; -import static org.junit.Assume.assumeTrue; - -//TODO to be merged back in FilePathTest after security release -public class FilePathSEC904Test { - - @Rule - public TemporaryFolder temp = new TemporaryFolder(); - - @Test - @Issue("SECURITY-904") - public void isDescendant_regularFiles() throws IOException, InterruptedException { - // root - // /workspace - // /sub - // sub-regular.txt - // regular.txt - // /protected - // secret.txt - FilePath rootFolder = new FilePath(temp.newFolder("root")); - FilePath workspaceFolder = rootFolder.child("workspace"); - FilePath subFolder = workspaceFolder.child("sub"); - FilePath protectedFolder = rootFolder.child("protected"); - - FilePath regularFile = workspaceFolder.child("regular.txt"); - regularFile.write("regular-file", StandardCharsets.UTF_8.name()); - FilePath subRegularFile = subFolder.child("sub-regular.txt"); - subRegularFile.write("sub-regular-file", StandardCharsets.UTF_8.name()); - - FilePath secretFile = protectedFolder.child("secret.txt"); - secretFile.write("secrets", StandardCharsets.UTF_8.name()); - - assertTrue(workspaceFolder.isDescendant(".")); - assertTrue(workspaceFolder.isDescendant("regular.txt")); - assertTrue(workspaceFolder.isDescendant("./regular.txt")); - assertTrue(workspaceFolder.isDescendant("sub/sub-regular.txt")); - assertTrue(workspaceFolder.isDescendant("sub//sub-regular.txt")); - assertTrue(workspaceFolder.isDescendant("sub/../sub/sub-regular.txt")); - assertTrue(workspaceFolder.isDescendant("./sub/../sub/sub-regular.txt")); - - // nonexistent files - assertTrue(workspaceFolder.isDescendant("nonexistent.txt")); - assertTrue(workspaceFolder.isDescendant("sub/nonexistent.txt")); - assertTrue(workspaceFolder.isDescendant("nonexistent/nonexistent.txt")); - assertFalse(workspaceFolder.isDescendant("../protected/nonexistent.txt")); - assertFalse(workspaceFolder.isDescendant("../nonexistent/nonexistent.txt")); - - // the intermediate path "./.." goes out of workspace and so is refused - assertFalse(workspaceFolder.isDescendant("./../workspace")); - assertFalse(workspaceFolder.isDescendant("./../workspace/")); - assertFalse(workspaceFolder.isDescendant("./../workspace/regular.txt")); - assertFalse(workspaceFolder.isDescendant("../workspace/regular.txt")); - assertFalse(workspaceFolder.isDescendant("./../../root/workspace/regular.txt")); - - // attempt to reach other folder - assertFalse(workspaceFolder.isDescendant("../protected/secret.txt")); - assertFalse(workspaceFolder.isDescendant("./../protected/secret.txt")); - } - - @Test - @Issue("SECURITY-904") - public void isDescendant_regularSymlinks() throws IOException, InterruptedException { - // root - // /workspace - // /a - // a.txt - // /b - // _a => symlink to ../a - // _atxt => symlink to ../a/a.txt - // regular.txt - // _nonexistent => symlink to nonexistent (nonexistent folder) - // _nonexistentUp => symlink to ../nonexistent (nonexistent folder + illegal) - // _protected => symlink to ../protected (illegal) - // _secrettxt => symlink to ../protected/secret.txt (illegal) - // /protected - // secret.txt - FilePath rootFolder = new FilePath(temp.newFolder("root")); - FilePath workspaceFolder = rootFolder.child("workspace"); - FilePath aFolder = workspaceFolder.child("a"); - FilePath bFolder = workspaceFolder.child("b"); - FilePath protectedFolder = rootFolder.child("protected"); - - FilePath regularFile = workspaceFolder.child("regular.txt"); - regularFile.write("regular-file", StandardCharsets.UTF_8.name()); - FilePath aFile = aFolder.child("a.txt"); - aFile.write("a-file", StandardCharsets.UTF_8.name()); - FilePath bFile = bFolder.child("a.txt"); - bFile.write("b-file", StandardCharsets.UTF_8.name()); - bFolder.child("_a").symlinkTo("../a", null); - bFolder.child("_atxt").symlinkTo("../a/a.txt", null); - // illegal symlinks - workspaceFolder.child("_protected").symlinkTo("../protected", null); - workspaceFolder.child("_nonexistent").symlinkTo("nonexistent", null); - workspaceFolder.child("_nonexistentUp").symlinkTo("../nonexistent", null); - workspaceFolder.child("_secrettxt").symlinkTo("../protected/secret.txt", null); - - FilePath secretFile = protectedFolder.child("secret.txt"); - secretFile.write("secrets", StandardCharsets.UTF_8.name()); - - assertTrue(workspaceFolder.isDescendant("regular.txt")); - assertTrue(workspaceFolder.isDescendant("_nonexistent")); - assertTrue(workspaceFolder.isDescendant("a")); - assertTrue(workspaceFolder.isDescendant("a/a.txt")); - assertTrue(workspaceFolder.isDescendant("a/../a/a.txt")); - assertTrue(workspaceFolder.isDescendant("b/../a/a.txt")); - assertTrue(workspaceFolder.isDescendant("b")); - assertTrue(workspaceFolder.isDescendant("./b")); - assertTrue(workspaceFolder.isDescendant("b/_a/a.txt")); - assertTrue(workspaceFolder.isDescendant("b/_a/../a/a.txt")); - assertTrue(workspaceFolder.isDescendant("b/_atxt")); - - // nonexistent but illegal - assertFalse(workspaceFolder.isDescendant("_nonexistentUp")); - // illegal symlinks - assertFalse(workspaceFolder.isDescendant("_protected")); - assertFalse(workspaceFolder.isDescendant("_protected/")); - assertFalse(workspaceFolder.isDescendant("_protected/secret.txt")); - assertFalse(workspaceFolder.isDescendant("./_protected/secret.txt")); - assertFalse(workspaceFolder.isDescendant("_secrettxt")); - assertFalse(workspaceFolder.isDescendant("./_secrettxt")); - } - - @Test - @Issue("SECURITY-904") - public void isDescendant_windowsSpecificSymlinks() throws Exception { - assumeTrue(Functions.isWindows()); - // root - // /workspace - // /a - // a.txt - // /b - // b.txt - // _a => junction to ../a - // regular.txt - // _nonexistent => junction to nonexistent (nonexistent folder) - // _nonexistentUp => junction to ../nonexistent (nonexistent and illegal) - // _protected => junction to ../protected (illegal) - // /protected - // secret.txt - File root = temp.newFolder("root"); - FilePath rootFolder = new FilePath(root); - FilePath workspaceFolder = rootFolder.child("workspace"); - FilePath aFolder = workspaceFolder.child("a"); - FilePath bFolder = workspaceFolder.child("b"); - FilePath protectedFolder = rootFolder.child("protected"); - - FilePath regularFile = workspaceFolder.child("regular.txt"); - regularFile.write("regular-file", StandardCharsets.UTF_8.name()); - FilePath aFile = aFolder.child("a.txt"); - aFile.write("a-file", StandardCharsets.UTF_8.name()); - FilePath bFile = bFolder.child("a.txt"); - bFile.write("b-file", StandardCharsets.UTF_8.name()); - - createJunction(new File(root, "/workspace/b/_a"), new File(root, "/workspace/a")); - createJunction(new File(root, "/workspace/_nonexistent"), new File(root, "/workspace/nonexistent")); - createJunction(new File(root, "/workspace/_nonexistentUp"), new File(root, "/nonexistent")); - createJunction(new File(root, "/workspace/_protected"), new File(root, "/protected")); - - FilePath secretFile = protectedFolder.child("secret.txt"); - secretFile.write("secrets", StandardCharsets.UTF_8.name()); - - assertTrue(workspaceFolder.isDescendant("b")); - assertTrue(workspaceFolder.isDescendant("b/_a/a.txt")); - assertTrue(workspaceFolder.isDescendant("b\\_a\\a.txt")); - assertTrue(workspaceFolder.isDescendant("b\\_a\\../a/a.txt")); - assertTrue(workspaceFolder.isDescendant("b\\_a\\..\\a\\a.txt")); - assertTrue(workspaceFolder.isDescendant(".\\b\\_a\\..\\a\\a.txt")); - assertTrue(workspaceFolder.isDescendant("b/_a/../a/a.txt")); - assertTrue(workspaceFolder.isDescendant("./b/_a/../a/a.txt")); - - // nonexistent and not proven illegal, the junction links are not resolved - // by Util.resolveSymlinkToFile / neither Path.toRealPath under Windows - assertTrue(workspaceFolder.isDescendant("_nonexistent")); - assertTrue(workspaceFolder.isDescendant("_nonexistent/")); - assertTrue(workspaceFolder.isDescendant("_nonexistent/..")); - assertTrue(workspaceFolder.isDescendant("_nonexistentUp")); - - // illegal symlinks - assertFalse(workspaceFolder.isDescendant("_protected")); - assertFalse(workspaceFolder.isDescendant("_protected/../a")); - } - - private void createJunction(File from, File to) throws Exception { - Process p = Runtime.getRuntime().exec(new String[]{"cmd", "/c", "mklink", "/J", from.getAbsolutePath(), to.getAbsolutePath()}); - p.waitFor(2, TimeUnit.SECONDS); - } - - @Issue("SECURITY-904") - public void isDescendant_throwIfParentDoesNotExist_symlink() throws Exception { - FilePath rootFolder = new FilePath(temp.newFolder("root")); - FilePath aFolder = rootFolder.child("a"); - aFolder.mkdirs(); - FilePath linkToNonexistent = aFolder.child("linkToNonexistent"); - linkToNonexistent.symlinkTo("__nonexistent__", null); - - assertThat(linkToNonexistent.isDescendant("."), is(false)); - } - - @Issue("SECURITY-904") - public void isDescendant_throwIfParentDoesNotExist_directNonexistent() throws Exception { - FilePath rootFolder = new FilePath(temp.newFolder("root")); - FilePath nonexistent = rootFolder.child("nonexistent"); - assertThat(nonexistent.isDescendant("."), is(false)); - } - - @Test(expected = IllegalArgumentException.class) - @Issue("SECURITY-904") - public void isDescendant_throwIfAbsolutePathGiven() throws Exception { - FilePath rootFolder = new FilePath(temp.newFolder("root")); - rootFolder.mkdirs(); - rootFolder.isDescendant(temp.newFile().getAbsolutePath()); - } - - @Test - @Issue("SECURITY-904") - public void isDescendant_worksEvenInSymbolicWorkspace() throws Exception { - // root - // /w - // /_workspace => symlink to ../workspace - // /workspace - // /a - // a.txt - // /b - // _a => symlink to ../a - // _atxt => symlink to ../a/a.txt - // regular.txt - // _nonexistent => symlink to nonexistent (nonexistent folder) - // _nonexistentUp => symlink to ../nonexistent (nonexistent folder + illegal) - // _protected => symlink to ../protected (illegal) - // _secrettxt => symlink to ../protected/secret.txt (illegal) - // /protected - // secret.txt - FilePath rootFolder = new FilePath(temp.newFolder("root")); - FilePath wFolder = rootFolder.child("w"); - FilePath workspaceFolder = rootFolder.child("workspace"); - FilePath aFolder = workspaceFolder.child("a"); - FilePath bFolder = workspaceFolder.child("b"); - FilePath protectedFolder = rootFolder.child("protected"); - - FilePath regularFile = workspaceFolder.child("regular.txt"); - regularFile.write("regular-file", StandardCharsets.UTF_8.name()); - FilePath aFile = aFolder.child("a.txt"); - aFile.write("a-file", StandardCharsets.UTF_8.name()); - FilePath bFile = bFolder.child("a.txt"); - bFile.write("b-file", StandardCharsets.UTF_8.name()); - bFolder.child("_a").symlinkTo("../a", null); - bFolder.child("_atxt").symlinkTo("../a/a.txt", null); - // illegal symlinks - workspaceFolder.child("_protected").symlinkTo("../protected", null); - workspaceFolder.child("_protected2").symlinkTo("../../protected", null); - workspaceFolder.child("_nonexistent").symlinkTo("nonexistent", null); - workspaceFolder.child("_nonexistentUp").symlinkTo("../nonexistent", null); - workspaceFolder.child("_secrettxt").symlinkTo("../protected/secret.txt", null); - workspaceFolder.child("_secrettxt2").symlinkTo("../../protected/secret.txt", null); - - wFolder.mkdirs(); - FilePath symbolicWorkspace = wFolder.child("_w"); - symbolicWorkspace.symlinkTo("../workspace", null); - - FilePath secretFile = protectedFolder.child("secret.txt"); - secretFile.write("secrets", StandardCharsets.UTF_8.name()); - - assertTrue(symbolicWorkspace.isDescendant("regular.txt")); - assertTrue(symbolicWorkspace.isDescendant("_nonexistent")); - assertTrue(symbolicWorkspace.isDescendant("a")); - assertTrue(symbolicWorkspace.isDescendant("a/a.txt")); - assertTrue(symbolicWorkspace.isDescendant("b")); - assertTrue(symbolicWorkspace.isDescendant("b/_a/a.txt")); - assertTrue(symbolicWorkspace.isDescendant("b/_atxt")); - - // nonexistent but illegal - assertFalse(symbolicWorkspace.isDescendant("_nonexistentUp")); - // illegal symlinks - assertFalse(symbolicWorkspace.isDescendant("_protected")); - assertFalse(symbolicWorkspace.isDescendant("_protected/")); - assertFalse(symbolicWorkspace.isDescendant("_protected/secret.txt")); - assertFalse(symbolicWorkspace.isDescendant("./_protected/secret.txt")); - assertFalse(symbolicWorkspace.isDescendant("_protected2")); - assertFalse(symbolicWorkspace.isDescendant("_protected2/secret.txt")); - assertFalse(symbolicWorkspace.isDescendant("_secrettxt")); - assertFalse(symbolicWorkspace.isDescendant("./_secrettxt")); - assertFalse(symbolicWorkspace.isDescendant("_secrettxt2")); - } -} diff --git a/core/src/test/java/hudson/FilePathTest.java b/core/src/test/java/hudson/FilePathTest.java index b40d0a37d4..0043abbfa3 100644 --- a/core/src/test/java/hudson/FilePathTest.java +++ b/core/src/test/java/hudson/FilePathTest.java @@ -42,6 +42,7 @@ import java.net.HttpURLConnection; import java.net.URL; import java.net.URLConnection; import java.net.URLStreamHandler; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.attribute.FileTime; @@ -64,12 +65,8 @@ import org.apache.commons.io.output.NullOutputStream; import org.apache.tools.ant.Project; import org.apache.tools.ant.taskdefs.Chmod; import static org.hamcrest.CoreMatchers.is; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; +import static org.junit.Assert.*; +import static org.junit.Assert.assertThat; import static org.junit.Assume.assumeThat; import static org.junit.Assume.assumeTrue; import static org.junit.Assume.assumeFalse; @@ -877,4 +874,279 @@ public class FilePathTest { assertTrue("file was not copied", Files.exists(destinationFile)); assertEquals("file mtime was not preserved", mtime, Files.getLastModifiedTime(destinationFile)); } + + @Test + @Issue("SECURITY-904") + public void isDescendant_regularFiles() throws IOException, InterruptedException { + // root + // /workspace + // /sub + // sub-regular.txt + // regular.txt + // /protected + // secret.txt + FilePath rootFolder = new FilePath(temp.newFolder("root")); + FilePath workspaceFolder = rootFolder.child("workspace"); + FilePath subFolder = workspaceFolder.child("sub"); + FilePath protectedFolder = rootFolder.child("protected"); + + FilePath regularFile = workspaceFolder.child("regular.txt"); + regularFile.write("regular-file", StandardCharsets.UTF_8.name()); + FilePath subRegularFile = subFolder.child("sub-regular.txt"); + subRegularFile.write("sub-regular-file", StandardCharsets.UTF_8.name()); + + FilePath secretFile = protectedFolder.child("secret.txt"); + secretFile.write("secrets", StandardCharsets.UTF_8.name()); + + assertTrue(workspaceFolder.isDescendant(".")); + assertTrue(workspaceFolder.isDescendant("regular.txt")); + assertTrue(workspaceFolder.isDescendant("./regular.txt")); + assertTrue(workspaceFolder.isDescendant("sub/sub-regular.txt")); + assertTrue(workspaceFolder.isDescendant("sub//sub-regular.txt")); + assertTrue(workspaceFolder.isDescendant("sub/../sub/sub-regular.txt")); + assertTrue(workspaceFolder.isDescendant("./sub/../sub/sub-regular.txt")); + + // nonexistent files + assertTrue(workspaceFolder.isDescendant("nonexistent.txt")); + assertTrue(workspaceFolder.isDescendant("sub/nonexistent.txt")); + assertTrue(workspaceFolder.isDescendant("nonexistent/nonexistent.txt")); + assertFalse(workspaceFolder.isDescendant("../protected/nonexistent.txt")); + assertFalse(workspaceFolder.isDescendant("../nonexistent/nonexistent.txt")); + + // the intermediate path "./.." goes out of workspace and so is refused + assertFalse(workspaceFolder.isDescendant("./../workspace")); + assertFalse(workspaceFolder.isDescendant("./../workspace/")); + assertFalse(workspaceFolder.isDescendant("./../workspace/regular.txt")); + assertFalse(workspaceFolder.isDescendant("../workspace/regular.txt")); + assertFalse(workspaceFolder.isDescendant("./../../root/workspace/regular.txt")); + + // attempt to reach other folder + assertFalse(workspaceFolder.isDescendant("../protected/secret.txt")); + assertFalse(workspaceFolder.isDescendant("./../protected/secret.txt")); + } + + @Test + @Issue("SECURITY-904") + public void isDescendant_regularSymlinks() throws IOException, InterruptedException { + // root + // /workspace + // /a + // a.txt + // /b + // _a => symlink to ../a + // _atxt => symlink to ../a/a.txt + // regular.txt + // _nonexistent => symlink to nonexistent (nonexistent folder) + // _nonexistentUp => symlink to ../nonexistent (nonexistent folder + illegal) + // _protected => symlink to ../protected (illegal) + // _secrettxt => symlink to ../protected/secret.txt (illegal) + // /protected + // secret.txt + FilePath rootFolder = new FilePath(temp.newFolder("root")); + FilePath workspaceFolder = rootFolder.child("workspace"); + FilePath aFolder = workspaceFolder.child("a"); + FilePath bFolder = workspaceFolder.child("b"); + FilePath protectedFolder = rootFolder.child("protected"); + + FilePath regularFile = workspaceFolder.child("regular.txt"); + regularFile.write("regular-file", StandardCharsets.UTF_8.name()); + FilePath aFile = aFolder.child("a.txt"); + aFile.write("a-file", StandardCharsets.UTF_8.name()); + FilePath bFile = bFolder.child("a.txt"); + bFile.write("b-file", StandardCharsets.UTF_8.name()); + bFolder.child("_a").symlinkTo("../a", null); + bFolder.child("_atxt").symlinkTo("../a/a.txt", null); + // illegal symlinks + workspaceFolder.child("_protected").symlinkTo("../protected", null); + workspaceFolder.child("_nonexistent").symlinkTo("nonexistent", null); + workspaceFolder.child("_nonexistentUp").symlinkTo("../nonexistent", null); + workspaceFolder.child("_secrettxt").symlinkTo("../protected/secret.txt", null); + + FilePath secretFile = protectedFolder.child("secret.txt"); + secretFile.write("secrets", StandardCharsets.UTF_8.name()); + + assertTrue(workspaceFolder.isDescendant("regular.txt")); + assertTrue(workspaceFolder.isDescendant("_nonexistent")); + assertTrue(workspaceFolder.isDescendant("a")); + assertTrue(workspaceFolder.isDescendant("a/a.txt")); + assertTrue(workspaceFolder.isDescendant("a/../a/a.txt")); + assertTrue(workspaceFolder.isDescendant("b/../a/a.txt")); + assertTrue(workspaceFolder.isDescendant("b")); + assertTrue(workspaceFolder.isDescendant("./b")); + assertTrue(workspaceFolder.isDescendant("b/_a/a.txt")); + assertTrue(workspaceFolder.isDescendant("b/_a/../a/a.txt")); + assertTrue(workspaceFolder.isDescendant("b/_atxt")); + + // nonexistent but illegal + assertFalse(workspaceFolder.isDescendant("_nonexistentUp")); + // illegal symlinks + assertFalse(workspaceFolder.isDescendant("_protected")); + assertFalse(workspaceFolder.isDescendant("_protected/")); + assertFalse(workspaceFolder.isDescendant("_protected/secret.txt")); + assertFalse(workspaceFolder.isDescendant("./_protected/secret.txt")); + assertFalse(workspaceFolder.isDescendant("_secrettxt")); + assertFalse(workspaceFolder.isDescendant("./_secrettxt")); + } + + @Test + @Issue("SECURITY-904") + public void isDescendant_windowsSpecificSymlinks() throws Exception { + assumeTrue(Functions.isWindows()); + // root + // /workspace + // /a + // a.txt + // /b + // b.txt + // _a => junction to ../a + // regular.txt + // _nonexistent => junction to nonexistent (nonexistent folder) + // _nonexistentUp => junction to ../nonexistent (nonexistent and illegal) + // _protected => junction to ../protected (illegal) + // /protected + // secret.txt + File root = temp.newFolder("root"); + FilePath rootFolder = new FilePath(root); + FilePath workspaceFolder = rootFolder.child("workspace"); + FilePath aFolder = workspaceFolder.child("a"); + FilePath bFolder = workspaceFolder.child("b"); + FilePath protectedFolder = rootFolder.child("protected"); + + FilePath regularFile = workspaceFolder.child("regular.txt"); + regularFile.write("regular-file", StandardCharsets.UTF_8.name()); + FilePath aFile = aFolder.child("a.txt"); + aFile.write("a-file", StandardCharsets.UTF_8.name()); + FilePath bFile = bFolder.child("a.txt"); + bFile.write("b-file", StandardCharsets.UTF_8.name()); + + createJunction(new File(root, "/workspace/b/_a"), new File(root, "/workspace/a")); + createJunction(new File(root, "/workspace/_nonexistent"), new File(root, "/workspace/nonexistent")); + createJunction(new File(root, "/workspace/_nonexistentUp"), new File(root, "/nonexistent")); + createJunction(new File(root, "/workspace/_protected"), new File(root, "/protected")); + + FilePath secretFile = protectedFolder.child("secret.txt"); + secretFile.write("secrets", StandardCharsets.UTF_8.name()); + + assertTrue(workspaceFolder.isDescendant("b")); + assertTrue(workspaceFolder.isDescendant("b/_a/a.txt")); + assertTrue(workspaceFolder.isDescendant("b\\_a\\a.txt")); + assertTrue(workspaceFolder.isDescendant("b\\_a\\../a/a.txt")); + assertTrue(workspaceFolder.isDescendant("b\\_a\\..\\a\\a.txt")); + assertTrue(workspaceFolder.isDescendant(".\\b\\_a\\..\\a\\a.txt")); + assertTrue(workspaceFolder.isDescendant("b/_a/../a/a.txt")); + assertTrue(workspaceFolder.isDescendant("./b/_a/../a/a.txt")); + + // nonexistent and not proven illegal, the junction links are not resolved + // by Util.resolveSymlinkToFile / neither Path.toRealPath under Windows + assertTrue(workspaceFolder.isDescendant("_nonexistent")); + assertTrue(workspaceFolder.isDescendant("_nonexistent/")); + assertTrue(workspaceFolder.isDescendant("_nonexistent/..")); + assertTrue(workspaceFolder.isDescendant("_nonexistentUp")); + + // illegal symlinks + assertFalse(workspaceFolder.isDescendant("_protected")); + assertFalse(workspaceFolder.isDescendant("_protected/../a")); + } + + private void createJunction(File from, File to) throws Exception { + Process p = Runtime.getRuntime().exec(new String[]{"cmd", "/c", "mklink", "/J", from.getAbsolutePath(), to.getAbsolutePath()}); + p.waitFor(2, TimeUnit.SECONDS); + } + + @Issue("SECURITY-904") + public void isDescendant_throwIfParentDoesNotExist_symlink() throws Exception { + FilePath rootFolder = new FilePath(temp.newFolder("root")); + FilePath aFolder = rootFolder.child("a"); + aFolder.mkdirs(); + FilePath linkToNonexistent = aFolder.child("linkToNonexistent"); + linkToNonexistent.symlinkTo("__nonexistent__", null); + + assertThat(linkToNonexistent.isDescendant("."), is(false)); + } + + @Issue("SECURITY-904") + public void isDescendant_throwIfParentDoesNotExist_directNonexistent() throws Exception { + FilePath rootFolder = new FilePath(temp.newFolder("root")); + FilePath nonexistent = rootFolder.child("nonexistent"); + assertThat(nonexistent.isDescendant("."), is(false)); + } + + @Test(expected = IllegalArgumentException.class) + @Issue("SECURITY-904") + public void isDescendant_throwIfAbsolutePathGiven() throws Exception { + FilePath rootFolder = new FilePath(temp.newFolder("root")); + rootFolder.mkdirs(); + rootFolder.isDescendant(temp.newFile().getAbsolutePath()); + } + + @Test + @Issue("SECURITY-904") + public void isDescendant_worksEvenInSymbolicWorkspace() throws Exception { + // root + // /w + // /_workspace => symlink to ../workspace + // /workspace + // /a + // a.txt + // /b + // _a => symlink to ../a + // _atxt => symlink to ../a/a.txt + // regular.txt + // _nonexistent => symlink to nonexistent (nonexistent folder) + // _nonexistentUp => symlink to ../nonexistent (nonexistent folder + illegal) + // _protected => symlink to ../protected (illegal) + // _secrettxt => symlink to ../protected/secret.txt (illegal) + // /protected + // secret.txt + FilePath rootFolder = new FilePath(temp.newFolder("root")); + FilePath wFolder = rootFolder.child("w"); + FilePath workspaceFolder = rootFolder.child("workspace"); + FilePath aFolder = workspaceFolder.child("a"); + FilePath bFolder = workspaceFolder.child("b"); + FilePath protectedFolder = rootFolder.child("protected"); + + FilePath regularFile = workspaceFolder.child("regular.txt"); + regularFile.write("regular-file", StandardCharsets.UTF_8.name()); + FilePath aFile = aFolder.child("a.txt"); + aFile.write("a-file", StandardCharsets.UTF_8.name()); + FilePath bFile = bFolder.child("a.txt"); + bFile.write("b-file", StandardCharsets.UTF_8.name()); + bFolder.child("_a").symlinkTo("../a", null); + bFolder.child("_atxt").symlinkTo("../a/a.txt", null); + // illegal symlinks + workspaceFolder.child("_protected").symlinkTo("../protected", null); + workspaceFolder.child("_protected2").symlinkTo("../../protected", null); + workspaceFolder.child("_nonexistent").symlinkTo("nonexistent", null); + workspaceFolder.child("_nonexistentUp").symlinkTo("../nonexistent", null); + workspaceFolder.child("_secrettxt").symlinkTo("../protected/secret.txt", null); + workspaceFolder.child("_secrettxt2").symlinkTo("../../protected/secret.txt", null); + + wFolder.mkdirs(); + FilePath symbolicWorkspace = wFolder.child("_w"); + symbolicWorkspace.symlinkTo("../workspace", null); + + FilePath secretFile = protectedFolder.child("secret.txt"); + secretFile.write("secrets", StandardCharsets.UTF_8.name()); + + assertTrue(symbolicWorkspace.isDescendant("regular.txt")); + assertTrue(symbolicWorkspace.isDescendant("_nonexistent")); + assertTrue(symbolicWorkspace.isDescendant("a")); + assertTrue(symbolicWorkspace.isDescendant("a/a.txt")); + assertTrue(symbolicWorkspace.isDescendant("b")); + assertTrue(symbolicWorkspace.isDescendant("b/_a/a.txt")); + assertTrue(symbolicWorkspace.isDescendant("b/_atxt")); + + // nonexistent but illegal + assertFalse(symbolicWorkspace.isDescendant("_nonexistentUp")); + // illegal symlinks + assertFalse(symbolicWorkspace.isDescendant("_protected")); + assertFalse(symbolicWorkspace.isDescendant("_protected/")); + assertFalse(symbolicWorkspace.isDescendant("_protected/secret.txt")); + assertFalse(symbolicWorkspace.isDescendant("./_protected/secret.txt")); + assertFalse(symbolicWorkspace.isDescendant("_protected2")); + assertFalse(symbolicWorkspace.isDescendant("_protected2/secret.txt")); + assertFalse(symbolicWorkspace.isDescendant("_secrettxt")); + assertFalse(symbolicWorkspace.isDescendant("./_secrettxt")); + assertFalse(symbolicWorkspace.isDescendant("_secrettxt2")); + } } diff --git a/core/src/test/java/hudson/UtilSEC904Test.java b/core/src/test/java/hudson/UtilSEC904Test.java deleted file mode 100644 index 7bb7896395..0000000000 --- a/core/src/test/java/hudson/UtilSEC904Test.java +++ /dev/null @@ -1,84 +0,0 @@ -/* - * The MIT License - * - * Copyright (c) 2004-2010, Sun Microsystems, Inc., Kohsuke Kawaguchi, - * Daniel Dyer, Erik Ramfelt, Richard Bair, id:cactusman - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN - * THE SOFTWARE. - */ -package hudson; - -import hudson.model.TaskListener; -import org.apache.commons.io.FileUtils; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.TemporaryFolder; - -import java.io.File; -import java.nio.file.Files; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; - -/** - * @author Kohsuke Kawaguchi - */ -//TODO merge into UtilTest after the security release -public class UtilSEC904Test { - - @Rule public TemporaryFolder tmp = new TemporaryFolder(); - - @Test - public void resolveSymlinkToFile() throws Exception { - // root - // /a - // /aa - // aa.txt - // /_b => symlink to /root/b - // /b - // /_a => symlink to /root/a - File root = tmp.getRoot(); - File a = new File(root, "a"); - File aa = new File(a, "aa"); - aa.mkdirs(); - File aaTxt = new File(aa, "aa.txt"); - FileUtils.write(aaTxt, "aa"); - - File b = new File(root, "b"); - b.mkdir(); - - File _a = new File(b, "_a"); - Util.createSymlink(_a.getParentFile(), a.getAbsolutePath(), _a.getName(), TaskListener.NULL); - - File _b = new File(a, "_b"); - Util.createSymlink(_b.getParentFile(), b.getAbsolutePath(), _b.getName(), TaskListener.NULL); - - assertTrue(Files.isSymbolicLink(_a.toPath())); - assertTrue(Files.isSymbolicLink(_b.toPath())); - - // direct symlinks are resolved - assertEquals(Util.resolveSymlinkToFile(_a), a); - assertEquals(Util.resolveSymlinkToFile(_b), b); - - // intermediate symlinks are NOT resolved - assertNull(Util.resolveSymlinkToFile(new File(_a, "aa"))); - assertNull(Util.resolveSymlinkToFile(new File(_a, "aa/aa.txt"))); - } -} diff --git a/core/src/test/java/hudson/UtilTest.java b/core/src/test/java/hudson/UtilTest.java index 8662fc5631..e6c448f430 100644 --- a/core/src/test/java/hudson/UtilTest.java +++ b/core/src/test/java/hudson/UtilTest.java @@ -43,7 +43,9 @@ import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.CoreMatchers.startsWith; import static org.junit.Assert.*; +import hudson.model.TaskListener; import hudson.os.WindowsUtil; +import org.apache.commons.io.FileUtils; import org.hamcrest.BaseMatcher; import org.hamcrest.Description; import org.junit.Assume; @@ -545,4 +547,41 @@ public class UtilTest { private Date parseDate(String dateString) throws ParseException { return new SimpleDateFormat("yyyy-MM-dd hh:mm:ss").parse(dateString); } + + @Test + public void resolveSymlinkToFile() throws Exception { + // root + // /a + // /aa + // aa.txt + // /_b => symlink to /root/b + // /b + // /_a => symlink to /root/a + File root = tmp.getRoot(); + File a = new File(root, "a"); + File aa = new File(a, "aa"); + aa.mkdirs(); + File aaTxt = new File(aa, "aa.txt"); + FileUtils.write(aaTxt, "aa"); + + File b = new File(root, "b"); + b.mkdir(); + + File _a = new File(b, "_a"); + Util.createSymlink(_a.getParentFile(), a.getAbsolutePath(), _a.getName(), TaskListener.NULL); + + File _b = new File(a, "_b"); + Util.createSymlink(_b.getParentFile(), b.getAbsolutePath(), _b.getName(), TaskListener.NULL); + + assertTrue(Files.isSymbolicLink(_a.toPath())); + assertTrue(Files.isSymbolicLink(_b.toPath())); + + // direct symlinks are resolved + assertEquals(Util.resolveSymlinkToFile(_a), a); + assertEquals(Util.resolveSymlinkToFile(_b), b); + + // intermediate symlinks are NOT resolved + assertNull(Util.resolveSymlinkToFile(new File(_a, "aa"))); + assertNull(Util.resolveSymlinkToFile(new File(_a, "aa/aa.txt"))); + } } diff --git a/core/src/test/java/jenkins/util/VirtualFileSEC904Test.java b/core/src/test/java/jenkins/util/VirtualFileSEC904Test.java deleted file mode 100644 index 4a679eb21d..0000000000 --- a/core/src/test/java/jenkins/util/VirtualFileSEC904Test.java +++ /dev/null @@ -1,281 +0,0 @@ -/* - * The MIT License - * - * Copyright 2015 Jesse Glick. - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN - * THE SOFTWARE. - */ - -package jenkins.util; - -import hudson.FilePath; -import hudson.model.TaskListener; -import org.apache.commons.io.FileUtils; -import org.hamcrest.Description; -import org.hamcrest.TypeSafeMatcher; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.TemporaryFolder; -import org.jvnet.hudson.test.Issue; - -import java.io.File; - -import static org.hamcrest.Matchers.contains; -import static org.hamcrest.Matchers.containsInAnyOrder; -import static org.hamcrest.Matchers.hasSize; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertThat; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; - -//TODO merge into VirtualFileTest after the security release -public class VirtualFileSEC904Test { - - @Rule - public TemporaryFolder tmp = new TemporaryFolder(); - - // root - // /a - // /aa - // /aaa - // /_b2 => symlink to /root/b - // aa.txt - // /ab - // ab.txt - // /_b => symlink to /root/b - // /b - // /_a => symlink to /root/a - // /_aatxt => symlink to /root/a/aa/aa.txt - // /ba - // ba.txt - private void prepareFileStructureForIsDescendant() throws Exception { - File root = tmp.getRoot(); - File a = new File(root, "a"); - File aa = new File(a, "aa"); - File aaa = new File(aa, "aaa"); - aaa.mkdirs(); - File aaTxt = new File(aa, "aa.txt"); - FileUtils.write(aaTxt, "aa"); - - File ab = new File(a, "ab"); - ab.mkdirs(); - File abTxt = new File(ab, "ab.txt"); - FileUtils.write(abTxt, "ab"); - - File b = new File(root, "b"); - - File ba = new File(b, "ba"); - ba.mkdirs(); - File baTxt = new File(ba, "ba.txt"); - FileUtils.write(baTxt, "ba"); - - File _a = new File(b, "_a"); - new FilePath(_a).symlinkTo(a.getAbsolutePath(), TaskListener.NULL); - - File _aatxt = new File(b, "_aatxt"); - new FilePath(_aatxt).symlinkTo(aaTxt.getAbsolutePath(), TaskListener.NULL); - - File _b = new File(a, "_b"); - new FilePath(_b).symlinkTo(b.getAbsolutePath(), TaskListener.NULL); - File _b2 = new File(aaa, "_b2"); - new FilePath(_b2).symlinkTo(b.getAbsolutePath(), TaskListener.NULL); - } - - @Issue("SECURITY-904") - @Test public void forFile_isDescendant() throws Exception { - this.prepareFileStructureForIsDescendant(); - - File root = tmp.getRoot(); - File a = new File(root, "a"); - File aa = new File(a, "aa"); - VirtualFile virtualRoot = VirtualFile.forFile(root); - // keep the root information for isDescendant - VirtualFile virtualRootChildA = virtualRoot.child("a"); - VirtualFile virtualFromA = VirtualFile.forFile(a); - - checkCommonAssertionForIsDescendant(virtualRoot, virtualRootChildA, virtualFromA, aa.getAbsolutePath()); - } - - @Test - @Issue("SECURITY-904") - public void forFilePath_isDescendant() throws Exception { - this.prepareFileStructureForIsDescendant(); - - File root = tmp.getRoot(); - File a = new File(root, "a"); - File aa = new File(a, "aa"); - VirtualFile virtualRoot = VirtualFile.forFilePath(new FilePath(root)); - // keep the root information for isDescendant - VirtualFile virtualRootChildA = virtualRoot.child("a"); - VirtualFile virtualFromA = VirtualFile.forFilePath(new FilePath(a)); - - checkCommonAssertionForIsDescendant(virtualRoot, virtualRootChildA, virtualFromA, aa.getAbsolutePath()); - } - - private void checkCommonAssertionForIsDescendant(VirtualFile virtualRoot, VirtualFile virtualRootChildA, VirtualFile virtualFromA, String absolutePath) throws Exception { - try { - virtualRootChildA.isDescendant(absolutePath); - fail("isDescendant should have refused the absolute path"); - } catch (IllegalArgumentException e) {} - - assertTrue(virtualRootChildA.isDescendant("aa")); - assertTrue(virtualRootChildA.isDescendant("aa/aa.txt")); - assertTrue(virtualRootChildA.isDescendant("aa\\aa.txt")); - assertTrue(virtualRootChildA.isDescendant("ab")); - assertTrue(virtualRootChildA.isDescendant("ab/ab.txt")); - assertTrue(virtualRootChildA.isDescendant("ab//ab.txt")); - assertTrue(virtualRootChildA.isDescendant("ab/nonExistingFile.txt")); - assertTrue(virtualRootChildA.isDescendant("nonExistingFolder")); - assertTrue(virtualRootChildA.isDescendant("nonExistingFolder/nonExistingFile.txt")); - - assertTrue(virtualRootChildA.isDescendant("_b")); - assertTrue(virtualRootChildA.isDescendant("_b/ba")); - assertTrue(virtualRootChildA.isDescendant("_b/ba/ba.txt")); - assertTrue(virtualRootChildA.isDescendant("aa/aaa/_b2")); - assertTrue(virtualRootChildA.isDescendant("aa/aaa/_b2/ba")); - assertTrue(virtualRootChildA.isDescendant("aa/aaa/_b2/ba/ba.txt")); - - // such approach could be used to check existence of file inside symlink - assertTrue(virtualRootChildA.isDescendant("_b/ba/ba-unexistingFile.txt")); - - // we go outside, then inside = forbidden, could be used to check existence of symlinks - assertTrue(virtualRootChildA.isDescendant("_b/_a")); - assertTrue(virtualRootChildA.isDescendant("_b/_a/aa")); - assertTrue(virtualRootChildA.isDescendant("_b/_a/aa/aa.txt")); - - assertTrue(virtualFromA.isDescendant("aa")); - assertFalse(virtualFromA.isDescendant("_b")); - assertFalse(virtualFromA.isDescendant("_b/ba/ba-unexistingFile.txt")); - assertFalse(virtualFromA.isDescendant("_b/_a")); - assertFalse(virtualFromA.isDescendant("_b/_a/aa")); - assertFalse(virtualFromA.isDescendant("_b/_a/aa/aa.txt")); - assertFalse(virtualFromA.isDescendant("aa/aaa/_b2")); - assertFalse(virtualFromA.isDescendant("aa/aaa/_b2/ba")); - assertFalse(virtualFromA.isDescendant("aa/aaa/_b2/ba/ba.txt")); - - assertTrue(virtualRoot.isDescendant("aa")); - assertTrue(virtualRoot.isDescendant("aa/aa.txt")); - assertTrue(virtualRoot.isDescendant("ab")); - assertTrue(virtualRoot.isDescendant("ab/ab.txt")); - assertTrue(virtualRoot.isDescendant("ab/nonExistingFile.txt")); - assertTrue(virtualRoot.isDescendant("nonExistingFolder")); - assertTrue(virtualRoot.isDescendant("nonExistingFolder/nonExistingFile.txt")); - - assertTrue(virtualRoot.isDescendant("_b")); - assertTrue(virtualRoot.isDescendant("_b/ba")); - assertTrue(virtualRoot.isDescendant("_b/ba/ba.txt")); - assertTrue(virtualRoot.isDescendant("_b/_a")); - assertTrue(virtualRoot.isDescendant("_b/_a/aa")); - assertTrue(virtualRoot.isDescendant("_b/_a/aa/")); - assertTrue(virtualRoot.isDescendant("_b/_a/aa/../ab/ab.txt")); - assertTrue(virtualRoot.isDescendant("_b/_a/aa/aa.txt")); - } - - @Test - @Issue("JENKINS-55050") - public void forFile_listOnlyDescendants_withoutIllegal() throws Exception { - this.prepareFileStructureForIsDescendant(); - - File root = tmp.getRoot(); - File a = new File(root, "a"); - File b = new File(root, "b"); - VirtualFile virtualRoot = VirtualFile.forFile(root); - VirtualFile virtualFromA = VirtualFile.forFile(a); - VirtualFile virtualFromB = VirtualFile.forFile(b); - - checkCommonAssertionForList(virtualRoot, virtualFromA, virtualFromB); - } - - @Test - @Issue("SECURITY-904") - public void forFilePath_listOnlyDescendants_withoutIllegal() throws Exception { - this.prepareFileStructureForIsDescendant(); - - File root = tmp.getRoot(); - File a = new File(root, "a"); - File b = new File(root, "b"); - VirtualFile virtualRoot = VirtualFile.forFilePath(new FilePath(root)); - VirtualFile virtualFromA = VirtualFile.forFilePath(new FilePath(a)); - VirtualFile virtualFromB = VirtualFile.forFilePath(new FilePath(b)); - - checkCommonAssertionForList(virtualRoot, virtualFromA, virtualFromB); - } - - private void checkCommonAssertionForList(VirtualFile virtualRoot, VirtualFile virtualFromA, VirtualFile virtualFromB) throws Exception { - // outside link to folder is not returned - assertThat(virtualFromA.listOnlyDescendants(), containsInAnyOrder( - VFMatcher.hasName("aa"), - VFMatcher.hasName("ab") - )); - - // outside link to file is not returned - assertThat(virtualFromB.listOnlyDescendants(), contains( - VFMatcher.hasName("ba") - )); - - assertThat(virtualFromA.child("_b").listOnlyDescendants(), hasSize(0)); - - assertThat(virtualFromA.child("aa").listOnlyDescendants(), containsInAnyOrder( - VFMatcher.hasName("aaa"), - VFMatcher.hasName("aa.txt") - )); - - // only a outside link - assertThat(virtualFromA.child("aa").child("aaa").listOnlyDescendants(), hasSize(0)); - - // as we start from the root, the a/_b linking to root/b is legal - assertThat(virtualRoot.child("a").listOnlyDescendants(), containsInAnyOrder( - VFMatcher.hasName("aa"), - VFMatcher.hasName("ab"), - VFMatcher.hasName("_b") - )); - - assertThat(virtualRoot.child("a").child("_b").listOnlyDescendants(), containsInAnyOrder( - VFMatcher.hasName("_a"), - VFMatcher.hasName("_aatxt"), - VFMatcher.hasName("ba") - )); - - assertThat(virtualRoot.child("a").child("_b").child("_a").listOnlyDescendants(), containsInAnyOrder( - VFMatcher.hasName("aa"), - VFMatcher.hasName("ab"), - VFMatcher.hasName("_b") - )); - } - - private abstract static class VFMatcher extends TypeSafeMatcher { - private final String description; - - private VFMatcher(String description) { - this.description = description; - } - - public void describeTo(Description description) { - description.appendText(this.description); - } - - public static VFMatcher hasName(String expectedName) { - return new VFMatcher("Has name: " + expectedName) { - protected boolean matchesSafely(VirtualFile vf) { - return expectedName.equals(vf.getName()); - } - }; - } - } -} diff --git a/core/src/test/java/jenkins/util/VirtualFileTest.java b/core/src/test/java/jenkins/util/VirtualFileTest.java index 8bab746557..9ee3c40cc1 100644 --- a/core/src/test/java/jenkins/util/VirtualFileTest.java +++ b/core/src/test/java/jenkins/util/VirtualFileTest.java @@ -42,7 +42,12 @@ import java.util.stream.Collectors; import org.apache.commons.io.FileUtils; import org.apache.commons.io.IOUtils; import org.apache.commons.io.input.NullInputStream; +import org.hamcrest.Description; +import org.hamcrest.TypeSafeMatcher; import org.junit.Test; + +import static org.hamcrest.Matchers.*; +import static org.hamcrest.Matchers.containsInAnyOrder; import static org.junit.Assert.*; import static org.junit.Assume.assumeFalse; @@ -195,4 +200,232 @@ public class VirtualFileTest { } } + // root + // /a + // /aa + // /aaa + // /_b2 => symlink to /root/b + // aa.txt + // /ab + // ab.txt + // /_b => symlink to /root/b + // /b + // /_a => symlink to /root/a + // /_aatxt => symlink to /root/a/aa/aa.txt + // /ba + // ba.txt + private void prepareFileStructureForIsDescendant() throws Exception { + File root = tmp.getRoot(); + File a = new File(root, "a"); + File aa = new File(a, "aa"); + File aaa = new File(aa, "aaa"); + aaa.mkdirs(); + File aaTxt = new File(aa, "aa.txt"); + FileUtils.write(aaTxt, "aa"); + + File ab = new File(a, "ab"); + ab.mkdirs(); + File abTxt = new File(ab, "ab.txt"); + FileUtils.write(abTxt, "ab"); + + File b = new File(root, "b"); + + File ba = new File(b, "ba"); + ba.mkdirs(); + File baTxt = new File(ba, "ba.txt"); + FileUtils.write(baTxt, "ba"); + + File _a = new File(b, "_a"); + new FilePath(_a).symlinkTo(a.getAbsolutePath(), TaskListener.NULL); + + File _aatxt = new File(b, "_aatxt"); + new FilePath(_aatxt).symlinkTo(aaTxt.getAbsolutePath(), TaskListener.NULL); + + File _b = new File(a, "_b"); + new FilePath(_b).symlinkTo(b.getAbsolutePath(), TaskListener.NULL); + File _b2 = new File(aaa, "_b2"); + new FilePath(_b2).symlinkTo(b.getAbsolutePath(), TaskListener.NULL); + } + + @Issue("SECURITY-904") + @Test public void forFile_isDescendant() throws Exception { + this.prepareFileStructureForIsDescendant(); + + File root = tmp.getRoot(); + File a = new File(root, "a"); + File aa = new File(a, "aa"); + VirtualFile virtualRoot = VirtualFile.forFile(root); + // keep the root information for isDescendant + VirtualFile virtualRootChildA = virtualRoot.child("a"); + VirtualFile virtualFromA = VirtualFile.forFile(a); + + checkCommonAssertionForIsDescendant(virtualRoot, virtualRootChildA, virtualFromA, aa.getAbsolutePath()); + } + + @Test + @Issue("SECURITY-904") + public void forFilePath_isDescendant() throws Exception { + this.prepareFileStructureForIsDescendant(); + + File root = tmp.getRoot(); + File a = new File(root, "a"); + File aa = new File(a, "aa"); + VirtualFile virtualRoot = VirtualFile.forFilePath(new FilePath(root)); + // keep the root information for isDescendant + VirtualFile virtualRootChildA = virtualRoot.child("a"); + VirtualFile virtualFromA = VirtualFile.forFilePath(new FilePath(a)); + + checkCommonAssertionForIsDescendant(virtualRoot, virtualRootChildA, virtualFromA, aa.getAbsolutePath()); + } + + private void checkCommonAssertionForIsDescendant(VirtualFile virtualRoot, VirtualFile virtualRootChildA, VirtualFile virtualFromA, String absolutePath) throws Exception { + try { + virtualRootChildA.isDescendant(absolutePath); + fail("isDescendant should have refused the absolute path"); + } catch (IllegalArgumentException e) {} + + assertTrue(virtualRootChildA.isDescendant("aa")); + assertTrue(virtualRootChildA.isDescendant("aa/aa.txt")); + assertTrue(virtualRootChildA.isDescendant("aa\\aa.txt")); + assertTrue(virtualRootChildA.isDescendant("ab")); + assertTrue(virtualRootChildA.isDescendant("ab/ab.txt")); + assertTrue(virtualRootChildA.isDescendant("ab//ab.txt")); + assertTrue(virtualRootChildA.isDescendant("ab/nonExistingFile.txt")); + assertTrue(virtualRootChildA.isDescendant("nonExistingFolder")); + assertTrue(virtualRootChildA.isDescendant("nonExistingFolder/nonExistingFile.txt")); + + assertTrue(virtualRootChildA.isDescendant("_b")); + assertTrue(virtualRootChildA.isDescendant("_b/ba")); + assertTrue(virtualRootChildA.isDescendant("_b/ba/ba.txt")); + assertTrue(virtualRootChildA.isDescendant("aa/aaa/_b2")); + assertTrue(virtualRootChildA.isDescendant("aa/aaa/_b2/ba")); + assertTrue(virtualRootChildA.isDescendant("aa/aaa/_b2/ba/ba.txt")); + + // such approach could be used to check existence of file inside symlink + assertTrue(virtualRootChildA.isDescendant("_b/ba/ba-unexistingFile.txt")); + + // we go outside, then inside = forbidden, could be used to check existence of symlinks + assertTrue(virtualRootChildA.isDescendant("_b/_a")); + assertTrue(virtualRootChildA.isDescendant("_b/_a/aa")); + assertTrue(virtualRootChildA.isDescendant("_b/_a/aa/aa.txt")); + + assertTrue(virtualFromA.isDescendant("aa")); + assertFalse(virtualFromA.isDescendant("_b")); + assertFalse(virtualFromA.isDescendant("_b/ba/ba-unexistingFile.txt")); + assertFalse(virtualFromA.isDescendant("_b/_a")); + assertFalse(virtualFromA.isDescendant("_b/_a/aa")); + assertFalse(virtualFromA.isDescendant("_b/_a/aa/aa.txt")); + assertFalse(virtualFromA.isDescendant("aa/aaa/_b2")); + assertFalse(virtualFromA.isDescendant("aa/aaa/_b2/ba")); + assertFalse(virtualFromA.isDescendant("aa/aaa/_b2/ba/ba.txt")); + + assertTrue(virtualRoot.isDescendant("aa")); + assertTrue(virtualRoot.isDescendant("aa/aa.txt")); + assertTrue(virtualRoot.isDescendant("ab")); + assertTrue(virtualRoot.isDescendant("ab/ab.txt")); + assertTrue(virtualRoot.isDescendant("ab/nonExistingFile.txt")); + assertTrue(virtualRoot.isDescendant("nonExistingFolder")); + assertTrue(virtualRoot.isDescendant("nonExistingFolder/nonExistingFile.txt")); + + assertTrue(virtualRoot.isDescendant("_b")); + assertTrue(virtualRoot.isDescendant("_b/ba")); + assertTrue(virtualRoot.isDescendant("_b/ba/ba.txt")); + assertTrue(virtualRoot.isDescendant("_b/_a")); + assertTrue(virtualRoot.isDescendant("_b/_a/aa")); + assertTrue(virtualRoot.isDescendant("_b/_a/aa/")); + assertTrue(virtualRoot.isDescendant("_b/_a/aa/../ab/ab.txt")); + assertTrue(virtualRoot.isDescendant("_b/_a/aa/aa.txt")); + } + + @Test + @Issue("JENKINS-55050") + public void forFile_listOnlyDescendants_withoutIllegal() throws Exception { + this.prepareFileStructureForIsDescendant(); + + File root = tmp.getRoot(); + File a = new File(root, "a"); + File b = new File(root, "b"); + VirtualFile virtualRoot = VirtualFile.forFile(root); + VirtualFile virtualFromA = VirtualFile.forFile(a); + VirtualFile virtualFromB = VirtualFile.forFile(b); + + checkCommonAssertionForList(virtualRoot, virtualFromA, virtualFromB); + } + + @Test + @Issue("SECURITY-904") + public void forFilePath_listOnlyDescendants_withoutIllegal() throws Exception { + this.prepareFileStructureForIsDescendant(); + + File root = tmp.getRoot(); + File a = new File(root, "a"); + File b = new File(root, "b"); + VirtualFile virtualRoot = VirtualFile.forFilePath(new FilePath(root)); + VirtualFile virtualFromA = VirtualFile.forFilePath(new FilePath(a)); + VirtualFile virtualFromB = VirtualFile.forFilePath(new FilePath(b)); + + checkCommonAssertionForList(virtualRoot, virtualFromA, virtualFromB); + } + + private void checkCommonAssertionForList(VirtualFile virtualRoot, VirtualFile virtualFromA, VirtualFile virtualFromB) throws Exception { + // outside link to folder is not returned + assertThat(virtualFromA.listOnlyDescendants(), containsInAnyOrder( + VFMatcher.hasName("aa"), + VFMatcher.hasName("ab") + )); + + // outside link to file is not returned + assertThat(virtualFromB.listOnlyDescendants(), contains( + VFMatcher.hasName("ba") + )); + + assertThat(virtualFromA.child("_b").listOnlyDescendants(), hasSize(0)); + + assertThat(virtualFromA.child("aa").listOnlyDescendants(), containsInAnyOrder( + VFMatcher.hasName("aaa"), + VFMatcher.hasName("aa.txt") + )); + + // only a outside link + assertThat(virtualFromA.child("aa").child("aaa").listOnlyDescendants(), hasSize(0)); + + // as we start from the root, the a/_b linking to root/b is legal + assertThat(virtualRoot.child("a").listOnlyDescendants(), containsInAnyOrder( + VFMatcher.hasName("aa"), + VFMatcher.hasName("ab"), + VFMatcher.hasName("_b") + )); + + assertThat(virtualRoot.child("a").child("_b").listOnlyDescendants(), containsInAnyOrder( + VFMatcher.hasName("_a"), + VFMatcher.hasName("_aatxt"), + VFMatcher.hasName("ba") + )); + + assertThat(virtualRoot.child("a").child("_b").child("_a").listOnlyDescendants(), containsInAnyOrder( + VFMatcher.hasName("aa"), + VFMatcher.hasName("ab"), + VFMatcher.hasName("_b") + )); + } + + private abstract static class VFMatcher extends TypeSafeMatcher { + private final String description; + + private VFMatcher(String description) { + this.description = description; + } + + public void describeTo(Description description) { + description.appendText(this.description); + } + + public static VFMatcher hasName(String expectedName) { + return new VFMatcher("Has name: " + expectedName) { + protected boolean matchesSafely(VirtualFile vf) { + return expectedName.equals(vf.getName()); + } + }; + } + } } diff --git a/test/src/test/java/hudson/model/DirectoryBrowserSupportSEC904Test.java b/test/src/test/java/hudson/model/DirectoryBrowserSupportSEC904Test.java deleted file mode 100644 index 06c9b057a4..0000000000 --- a/test/src/test/java/hudson/model/DirectoryBrowserSupportSEC904Test.java +++ /dev/null @@ -1,606 +0,0 @@ -/* - * The MIT License - * - * Copyright (c) 2004-2009, Sun Microsystems, Inc., Kohsuke Kawaguchi - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN - * THE SOFTWARE. - */ -package hudson.model; - -import com.gargoylesoftware.htmlunit.Page; -import com.gargoylesoftware.htmlunit.UnexpectedPage; -import hudson.FilePath; -import hudson.Functions; -import hudson.tasks.BatchFile; -import hudson.tasks.Shell; -import org.apache.commons.io.FileUtils; -import org.apache.commons.io.IOUtils; -import org.junit.Assume; -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.io.InputStream; -import java.io.OutputStream; -import java.net.HttpURLConnection; -import java.net.URI; -import java.net.URISyntaxException; -import java.net.URL; -import java.nio.file.Files; -import java.util.List; -import java.util.stream.Collectors; -import java.util.zip.ZipEntry; -import java.util.zip.ZipFile; - -import static org.hamcrest.CoreMatchers.allOf; -import static org.hamcrest.CoreMatchers.containsString; -import static org.hamcrest.CoreMatchers.equalTo; -import static org.hamcrest.CoreMatchers.not; -import static org.hamcrest.Matchers.contains; -import static org.hamcrest.Matchers.containsInAnyOrder; -import static org.hamcrest.Matchers.hasSize; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertThat; -import static org.junit.Assert.fail; - -public class DirectoryBrowserSupportSEC904Test { - - @Rule - public JenkinsRule j = new JenkinsRule(); - - @Test - @Issue("SECURITY-904") - public void symlink_outsideWorkspace_areNotAllowed() throws Exception { - FreeStyleProject p = j.createFreeStyleProject(); - - File secretsFolder = new File(j.jenkins.getRootDir(), "secrets"); - File secretTarget = new File(secretsFolder, "goal.txt"); - String secretContent = "secret"; - FileUtils.write(secretTarget, secretContent); - - /* - * secrets/ - * goal.txt - * workspace/ - * intermediateFolder/ - * public2.key - * otherFolder/ - * to_secret3 -> ../../../../secrets/ - * to_secret2 -> ../../../secrets/ - * to_secret_goal2 -> ../../../secrets/goal.txt - * public1.key - * to_secret1 -> ../../secrets/ - * to_secret_goal1 -> ../../secrets/goal.txt - * - */ - if (Functions.isWindows()) { - // no need to test mklink /H since we cannot create an hard link to a non-existing file - // and so you need to have access to the master file system directly which is already a problem - - String script = loadContentFromResource("outsideWorkspaceStructure.bat"); - p.getBuildersList().add(new BatchFile(script)); - } else { - String script = loadContentFromResource("outsideWorkspaceStructure.sh"); - p.getBuildersList().add(new Shell(script)); - } - - assertEquals(Result.SUCCESS, p.scheduleBuild2(0).get().getResult()); - - JenkinsRule.WebClient wc = j.createWebClient(); - wc.getOptions().setThrowExceptionOnFailingStatusCode(false); - { // workspace root must be reachable (regular case) - Page page = wc.goTo(p.getUrl() + "ws/", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); - String workspaceContent = page.getWebResponse().getContentAsString(); - assertThat(workspaceContent, allOf( - containsString("public1.key"), - containsString("intermediateFolder"), - containsString("to_secrets1"), - containsString("to_secrets_goal1"), - not(containsString("to_secrets2")), - not(containsString("to_secrets_goal2")) - )); - } - { // to_secrets1 not reachable - Page page = wc.goTo(p.getUrl() + "ws/to_secrets1/", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); - } - { // to_secrets_goal1 not reachable - Page page = wc.goTo(p.getUrl() + "ws/to_secrets_goal1/", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); - } - { // intermediateFolder must be reachable (regular case) - Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); - String workspaceContent = page.getWebResponse().getContentAsString(); - assertThat(workspaceContent, allOf( - not(containsString("to_secrets1")), - not(containsString("to_secrets_goal1")), - containsString("to_secrets2"), - containsString("to_secrets_goal2") - )); - } - { // to_secrets2 not reachable - Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/to_secrets2/", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); - } - { // using symbolic in the intermediate path - Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/to_secrets2/master.key", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); - } - { // to_secrets_goal2 not reachable - Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/to_secrets_goal2/", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); - } - - // pattern search feature - { // the pattern allow us to search inside the files / folders, - // without the patch the master.key from inside the outside symlinks would have been linked - Page page = wc.goTo(p.getUrl() + "ws/**/*.key", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); - String workspaceContent = page.getWebResponse().getContentAsString(); - assertThat(workspaceContent, allOf( - not(containsString("master.key")), - containsString("public1.key"), - containsString("public2.key") - )); - } - - // zip feature - { // all the outside folders / files are not included in the zip - Page zipPage = wc.goTo(p.getUrl() + "ws/*zip*/ws.zip", null); - assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); - - List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); - assertThat(entryNames, containsInAnyOrder( - p.getName() + "/intermediateFolder/public2.key", - p.getName() + "/public1.key" - )); - } - { // all the outside folders / files are not included in the zip - Page zipPage = wc.goTo(p.getUrl() + "ws/intermediateFolder/*zip*/intermediateFolder.zip", null); - assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); - - List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); - assertThat(entryNames, contains("intermediateFolder/public2.key")); - } - } - - /* - * If the glob filter is used, we do not want that it leaks some information. - * Presence of a folder means that the folder contains one or multiple results, so we need to hide it completely - */ - @Test - @Issue("SECURITY-904") - public void symlink_avoidLeakingInformation_aboutIllegalFolder() throws Exception { - FreeStyleProject p = j.createFreeStyleProject(); - - File secretsFolder = new File(j.jenkins.getRootDir(), "secrets"); - File secretTarget = new File(secretsFolder, "goal.txt"); - String secretContent = "secret"; - FileUtils.write(secretTarget, secretContent); - FileUtils.write(new File(secretsFolder, "public_fake1.key"), secretContent); - FileUtils.write(new File(secretsFolder, "public_fake2.key"), secretContent); - FileUtils.write(new File(secretsFolder, "public_fake3.key"), secretContent); - - /* - * secrets/ - * goal.txt - * public_fake1.key - * public_fake2.key - * public_fake3.key - * workspace/ - * intermediateFolder/ - * public2.key - * otherFolder/ - * to_secret3 -> ../../../../secrets/ - * to_secret2 -> ../../../secrets/ - * to_secret_goal2 -> ../../../secrets/goal.txt - * public1.key - * to_secret1 -> ../../secrets/ - * to_secret_goal1 -> ../../secrets/goal.txt - * - */ - if (Functions.isWindows()) { - // no need to test mklink /H since we cannot create an hard link to a non-existing file - // and so you need to have access to the master file system directly which is already a problem - - String script = loadContentFromResource("outsideWorkspaceStructure.bat"); - p.getBuildersList().add(new BatchFile(script)); - } else { - String script = loadContentFromResource("outsideWorkspaceStructure.sh"); - p.getBuildersList().add(new Shell(script)); - } - - assertEquals(Result.SUCCESS, p.scheduleBuild2(0).get().getResult()); - - JenkinsRule.WebClient wc = j.createWebClient(); - wc.getOptions().setThrowExceptionOnFailingStatusCode(false); - - // the pattern allow us to search inside the files / folders, - // but it should not provide / leak information about non readable folders - - { // without the patch the otherFolder and to_secrets[1,2,3] will appear in the results (once) - Page page = wc.goTo(p.getUrl() + "ws/**/goal.txt", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); - String workspaceContent = page.getWebResponse().getContentAsString(); - assertThat(workspaceContent, allOf( - // really not satisfying the query - not(containsString("public1.key")), - not(containsString("public2.key")), - // those following presences would have leak information that there is some file satisfying that pattern inside - not(containsString("to_secrets")), - not(containsString("to_secrets2")), - not(containsString("to_secrets3")) - )); - } - { // without the patch the otherFolder and to_secrets[1,2,3] will appear in the results (3 times each) - Page page = wc.goTo(p.getUrl() + "ws/**/public*.key", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); - String workspaceContent = page.getWebResponse().getContentAsString(); - assertThat(workspaceContent, allOf( - containsString("public1.key"), - containsString("public2.key"), - // those following presences would have leak information that there is some file satisfying that pattern inside - not(containsString("otherFolder")), - not(containsString("to_secrets")), - not(containsString("to_secrets2")), - not(containsString("to_secrets3")) - )); - } - } - - // The hard links (mklink /H) to file are impossible to be detected and will allow a user to retrieve any file in the system - // to achieve that they should already have access to the system or the Script Console. - @Test - @Issue("SECURITY-904") - public void junctionAndSymlink_outsideWorkspace_areNotAllowed_windowsJunction() throws Exception { - Assume.assumeTrue(Functions.isWindows()); - - FreeStyleProject p = j.createFreeStyleProject(); - - File secretsFolder = new File(j.jenkins.getRootDir(), "secrets"); - File secretTarget = new File(secretsFolder, "goal.txt"); - String secretContent = "secret"; - FileUtils.write(secretTarget, secretContent); - - /* - * secrets/ - * goal.txt - * workspace/ - * intermediateFolder/ - * public2.key - * otherFolder/ - * to_secret3s -> symlink ../../../../secrets/ - * to_secret3j -> junction ../../../../secrets/ - * to_secret2s -> symlink ../../../secrets/ - * to_secret2j -> junction ../../../secrets/ - * to_secret_goal2 -> symlink ../../../secrets/goal.txt - * public1.key - * to_secret1s -> symlink ../../secrets/ - * to_secret1j -> junction ../../secrets/ - * to_secret_goal1 -> symlink ../../secrets/goal.txt - * - */ - String script = loadContentFromResource("outsideWorkspaceStructureWithJunctions.bat"); - p.getBuildersList().add(new BatchFile(script)); - - assertEquals(Result.SUCCESS, p.scheduleBuild2(0).get().getResult()); - - JenkinsRule.WebClient wc = j.createWebClient(); - wc.getOptions().setThrowExceptionOnFailingStatusCode(false); - { // workspace root must be reachable (regular case) - Page page = wc.goTo(p.getUrl() + "ws/", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); - String workspaceContent = page.getWebResponse().getContentAsString(); - assertThat(workspaceContent, allOf( - containsString("public1.key"), - containsString("intermediateFolder"), - containsString("to_secrets1j"), - containsString("to_secrets1s"), - containsString("to_secrets_goal1"), - not(containsString("to_secrets2")), - not(containsString("to_secrets_goal2")) - )); - } - { // to_secrets1s not reachable - Page page = wc.goTo(p.getUrl() + "ws/to_secrets1s/", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); - } - { // to_secrets1j not reachable - Page page = wc.goTo(p.getUrl() + "ws/to_secrets1j/", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); - } - { // to_secrets_goal1 not reachable - Page page = wc.goTo(p.getUrl() + "ws/to_secrets_goal1/", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); - } - { // intermediateFolder must be reachable (regular case) - Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); - String workspaceContent = page.getWebResponse().getContentAsString(); - assertThat(workspaceContent, allOf( - not(containsString("to_secrets1")), - not(containsString("to_secrets_goal1")), - containsString("to_secrets2s"), - containsString("to_secrets2j"), - containsString("to_secrets_goal2") - )); - } - { // to_secrets2s not reachable - Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/to_secrets2s/", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); - } - { // to_secrets2j not reachable - Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/to_secrets2j/", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); - } - { // using symbolic in the intermediate path - Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/to_secrets2s/master.key", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); - } - { // using symbolic in the intermediate path - Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/to_secrets2j/master.key", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); - } - { // to_secrets_goal2 not reachable - Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/to_secrets_goal2/", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); - } - - // pattern search feature - { // the pattern allow us to search inside the files / folders, - // without the patch the master.key from inside the outside symlinks would have been linked - Page page = wc.goTo(p.getUrl() + "ws/**/*.key", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); - String workspaceContent = page.getWebResponse().getContentAsString(); - assertThat(workspaceContent, allOf( - not(containsString("master.key")), - containsString("public1.key"), - containsString("public2.key"), - containsString("intermediateFolder"), - not(containsString("otherFolder")), - not(containsString("to_secrets3j")), - not(containsString("to_secrets3s")), - not(containsString("to_secrets2j")), - not(containsString("to_secrets2s")), - not(containsString("to_secrets1j")), - not(containsString("to_secrets1s")) - )); - } - - // zip feature - { // all the outside folders / files are not included in the zip - Page zipPage = wc.goTo(p.getUrl() + "ws/*zip*/ws.zip", null); - assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); - - List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); - assertThat(entryNames, containsInAnyOrder( - p.getName() + "/intermediateFolder/public2.key", - p.getName() + "/public1.key" - )); - } - { // all the outside folders / files are not included in the zip - Page zipPage = wc.goTo(p.getUrl() + "ws/intermediateFolder/*zip*/intermediateFolder.zip", null); - assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); - - List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); - assertThat(entryNames, contains("intermediateFolder/public2.key")); - } - } - - private List getListOfEntriesInDownloadedZip(UnexpectedPage zipPage) throws Exception { - List result; - - File zipfile = null; - ZipFile readzip = null; - try { - zipfile = download(zipPage); - - readzip = new ZipFile(zipfile); - result = readzip.stream().map(ZipEntry::getName).collect(Collectors.toList()); - } - finally { - if (readzip != null) { - readzip.close(); - } - if (zipfile != null) { - zipfile.delete(); - } - } - return result; - } - - //TODO taken from existing test without modification to ease merge back - private File download(UnexpectedPage page) throws IOException { - File file = File.createTempFile("DirectoryBrowserSupport", "zipDownload"); - file.delete(); - try (InputStream is = page.getInputStream(); - OutputStream os = Files.newOutputStream(file.toPath())) { - IOUtils.copy(is, os); - } - - return file; - } - - @Test - @Issue("SECURITY-904") - public void directSymlink_forTestingZip() throws Exception { - FreeStyleProject p = j.createFreeStyleProject(); - - assertEquals(Result.SUCCESS, p.scheduleBuild2(0).get().getResult()); - FilePath ws = p.getSomeWorkspace(); - - /* - * secrets/ - * goal.txt - * workspace/ - * /a1/to_secrets1 - * /b1/b2/to_secrets1 - * /c1/c2/c3/to_secrets1 - */ - File secretsFolder = new File(j.jenkins.getRootDir(), "secrets"); - FilePath a1 = ws.child("a1"); - a1.mkdirs(); - a1.child("to_secrets1").symlinkTo(secretsFolder.getAbsolutePath(), TaskListener.NULL); - FilePath b2 = ws.child("b1").child("b2"); - b2.mkdirs(); - b2.child("to_secrets2").symlinkTo(secretsFolder.getAbsolutePath(), TaskListener.NULL); - FilePath c3 = ws.child("c1").child("c2").child("c3"); - c3.mkdirs(); - c3.child("to_secrets3").symlinkTo(secretsFolder.getAbsolutePath(), TaskListener.NULL); - - JenkinsRule.WebClient wc = j.createWebClient(); - wc.getOptions().setThrowExceptionOnFailingStatusCode(false); - { - Page zipPage = wc.goTo(p.getUrl() + "ws/*zip*/ws.zip", null); - assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); - - List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); - assertThat(entryNames, hasSize(0)); - } - { - Page zipPage = wc.goTo(p.getUrl() + "ws/a1/*zip*/a1.zip", null); - assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); - - List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); - assertThat(entryNames, hasSize(0)); - } - { - Page zipPage = wc.goTo(p.getUrl() + "ws/b1/b2/*zip*/b2.zip", null); - assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); - - List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); - assertThat(entryNames, hasSize(0)); - } - { - Page zipPage = wc.goTo(p.getUrl() + "ws/c1/c2/c3/*zip*/c3.zip", null); - assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); - - List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); - assertThat(entryNames, hasSize(0)); - } - } - - @Test - @Issue("SECURITY-904") - public void symlink_insideWorkspace_areStillAllowed() throws Exception { - FreeStyleProject p = j.createFreeStyleProject(); - - // build once to have the workspace set up - assertEquals(Result.SUCCESS, p.scheduleBuild2(0).get().getResult()); - - File jobWorkspaceFolder = new File(new File(j.jenkins.getRootDir(), "workspace"), p.name); - File folderInsideWorkspace = new File(jobWorkspaceFolder, "asset"); - folderInsideWorkspace.mkdir(); - File fileTarget = new File(folderInsideWorkspace, "goal.txt"); - String publicContent = "not-secret"; - FileUtils.write(fileTarget, publicContent); - - /* - * workspace/ - * asset/ - * goal.txt - * intermediateFolder/ - * to_internal2 -> ../asset - * to_internal_goal2 -> ../asset/goal.txt - * to_internal1 -> ./asset/ - * to_internal_goal1 -> ./asset/goal.txt - */ - if (Functions.isWindows()) { - String script = loadContentFromResource("insideWorkspaceStructure.bat"); - p.getBuildersList().add(new BatchFile(script)); - } else { - String script = loadContentFromResource("insideWorkspaceStructure.sh"); - p.getBuildersList().add(new Shell(script)); - } - - assertEquals(Result.SUCCESS, p.scheduleBuild2(0).get().getResult()); - - JenkinsRule.WebClient wc = j.createWebClient(); - wc.getOptions().setThrowExceptionOnFailingStatusCode(false); - { // workspace root must be reachable (regular case) - Page page = wc.goTo(p.getUrl() + "ws/", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); - String workspaceContent = page.getWebResponse().getContentAsString(); - assertThat(workspaceContent, allOf( - containsString("asset"), - containsString("to_internal1"), - containsString("to_internal_goal1"), - containsString("intermediateFolder"), - not(containsString("to_internal2")), - not(containsString("to_internal_goal2") - ))); - } - { // to_internal1 reachable - Page page = wc.goTo(p.getUrl() + "ws/to_internal1/", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); - String workspaceContent = page.getWebResponse().getContentAsString(); - assertThat(workspaceContent, containsString("goal.txt")); - } - { // to_internal_goal1 reachable - Page page = wc.goTo(p.getUrl() + "ws/to_internal_goal1/", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); - String workspaceContent = page.getWebResponse().getContentAsString(); - assertThat(workspaceContent, containsString(publicContent)); - } - { // to_internal2 reachable - Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/to_internal2/", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); - String workspaceContent = page.getWebResponse().getContentAsString(); - assertThat(workspaceContent, containsString("goal.txt")); - } - { // to_internal_goal2 reachable - Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/to_internal_goal2/", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); - String workspaceContent = page.getWebResponse().getContentAsString(); - assertThat(workspaceContent, containsString(publicContent)); - } - { // direct to goal - Page page = wc.goTo(p.getUrl() + "ws/asset/goal.txt/", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); - String workspaceContent = page.getWebResponse().getContentAsString(); - assertThat(workspaceContent, containsString(publicContent)); - } - { // the zip will only contain folder from inside the workspace - Page page = wc.goTo(p.getUrl() + "ws/*zip*/ws.zip", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); - } - { // the zip will only contain folder from inside the workspace - Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/*zip*/intermediateFolder.zip", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); - } - { // the zip will only contain folder from inside the workspace - Page page = wc.goTo(p.getUrl() + "ws/asset/*zip*/asset.zip", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); - } - } - - private String loadContentFromResource(String fileNameInResources) throws IOException, URISyntaxException { - URL resourceUrl = DirectoryBrowserSupportSEC904Test.class.getResource(DirectoryBrowserSupportSEC904Test.class.getSimpleName() + "/" + fileNameInResources); - if (resourceUrl == null) { - fail("The resource with fileName " + fileNameInResources + " is not present in the resources of the test"); - } - File resourceFile = new File(resourceUrl.toURI()); - return FileUtils.readFileToString(resourceFile); - } -} diff --git a/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java b/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java index 45cfaac28f..3af2089395 100644 --- a/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java +++ b/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java @@ -23,9 +23,6 @@ */ package hudson.model; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; import hudson.FilePath; import hudson.Functions; import hudson.Launcher; @@ -36,9 +33,14 @@ import hudson.tasks.Shell; import java.io.File; import java.io.IOException; import java.io.InputStream; +import java.net.HttpURLConnection; +import java.net.URISyntaxException; import java.nio.file.Files; +import java.util.stream.Collectors; +import java.util.zip.ZipEntry; import java.util.zip.ZipFile; +import org.apache.commons.io.FileUtils; import org.junit.Assume; import org.junit.Rule; import org.junit.Test; @@ -74,6 +76,15 @@ import org.jvnet.hudson.test.TestExtension; import org.kohsuke.stapler.StaplerRequest; import org.kohsuke.stapler.StaplerResponse; +import static org.hamcrest.CoreMatchers.*; +import static org.hamcrest.CoreMatchers.allOf; +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.not; +import static org.hamcrest.Matchers.*; +import static org.hamcrest.Matchers.hasSize; +import static org.junit.Assert.*; + /** * @author Kohsuke Kawaguchi */ @@ -423,4 +434,528 @@ public class DirectoryBrowserSupportTest { } } + @Test + @Issue("SECURITY-904") + public void symlink_outsideWorkspace_areNotAllowed() throws Exception { + FreeStyleProject p = j.createFreeStyleProject(); + + File secretsFolder = new File(j.jenkins.getRootDir(), "secrets"); + File secretTarget = new File(secretsFolder, "goal.txt"); + String secretContent = "secret"; + FileUtils.write(secretTarget, secretContent); + + /* + * secrets/ + * goal.txt + * workspace/ + * intermediateFolder/ + * public2.key + * otherFolder/ + * to_secret3 -> ../../../../secrets/ + * to_secret2 -> ../../../secrets/ + * to_secret_goal2 -> ../../../secrets/goal.txt + * public1.key + * to_secret1 -> ../../secrets/ + * to_secret_goal1 -> ../../secrets/goal.txt + * + */ + if (Functions.isWindows()) { + // no need to test mklink /H since we cannot create an hard link to a non-existing file + // and so you need to have access to the master file system directly which is already a problem + + String script = loadContentFromResource("outsideWorkspaceStructure.bat"); + p.getBuildersList().add(new BatchFile(script)); + } else { + String script = loadContentFromResource("outsideWorkspaceStructure.sh"); + p.getBuildersList().add(new Shell(script)); + } + + assertEquals(Result.SUCCESS, p.scheduleBuild2(0).get().getResult()); + + JenkinsRule.WebClient wc = j.createWebClient(); + wc.getOptions().setThrowExceptionOnFailingStatusCode(false); + { // workspace root must be reachable (regular case) + Page page = wc.goTo(p.getUrl() + "ws/", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + String workspaceContent = page.getWebResponse().getContentAsString(); + assertThat(workspaceContent, allOf( + containsString("public1.key"), + containsString("intermediateFolder"), + containsString("to_secrets1"), + containsString("to_secrets_goal1"), + not(containsString("to_secrets2")), + not(containsString("to_secrets_goal2")) + )); + } + { // to_secrets1 not reachable + Page page = wc.goTo(p.getUrl() + "ws/to_secrets1/", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); + } + { // to_secrets_goal1 not reachable + Page page = wc.goTo(p.getUrl() + "ws/to_secrets_goal1/", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); + } + { // intermediateFolder must be reachable (regular case) + Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + String workspaceContent = page.getWebResponse().getContentAsString(); + assertThat(workspaceContent, allOf( + not(containsString("to_secrets1")), + not(containsString("to_secrets_goal1")), + containsString("to_secrets2"), + containsString("to_secrets_goal2") + )); + } + { // to_secrets2 not reachable + Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/to_secrets2/", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); + } + { // using symbolic in the intermediate path + Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/to_secrets2/master.key", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); + } + { // to_secrets_goal2 not reachable + Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/to_secrets_goal2/", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); + } + + // pattern search feature + { // the pattern allow us to search inside the files / folders, + // without the patch the master.key from inside the outside symlinks would have been linked + Page page = wc.goTo(p.getUrl() + "ws/**/*.key", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + String workspaceContent = page.getWebResponse().getContentAsString(); + assertThat(workspaceContent, allOf( + not(containsString("master.key")), + containsString("public1.key"), + containsString("public2.key") + )); + } + + // zip feature + { // all the outside folders / files are not included in the zip + Page zipPage = wc.goTo(p.getUrl() + "ws/*zip*/ws.zip", null); + assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + + List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); + assertThat(entryNames, containsInAnyOrder( + p.getName() + "/intermediateFolder/public2.key", + p.getName() + "/public1.key" + )); + } + { // all the outside folders / files are not included in the zip + Page zipPage = wc.goTo(p.getUrl() + "ws/intermediateFolder/*zip*/intermediateFolder.zip", null); + assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + + List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); + assertThat(entryNames, contains("intermediateFolder/public2.key")); + } + } + + /* + * If the glob filter is used, we do not want that it leaks some information. + * Presence of a folder means that the folder contains one or multiple results, so we need to hide it completely + */ + @Test + @Issue("SECURITY-904") + public void symlink_avoidLeakingInformation_aboutIllegalFolder() throws Exception { + FreeStyleProject p = j.createFreeStyleProject(); + + File secretsFolder = new File(j.jenkins.getRootDir(), "secrets"); + File secretTarget = new File(secretsFolder, "goal.txt"); + String secretContent = "secret"; + FileUtils.write(secretTarget, secretContent); + FileUtils.write(new File(secretsFolder, "public_fake1.key"), secretContent); + FileUtils.write(new File(secretsFolder, "public_fake2.key"), secretContent); + FileUtils.write(new File(secretsFolder, "public_fake3.key"), secretContent); + + /* + * secrets/ + * goal.txt + * public_fake1.key + * public_fake2.key + * public_fake3.key + * workspace/ + * intermediateFolder/ + * public2.key + * otherFolder/ + * to_secret3 -> ../../../../secrets/ + * to_secret2 -> ../../../secrets/ + * to_secret_goal2 -> ../../../secrets/goal.txt + * public1.key + * to_secret1 -> ../../secrets/ + * to_secret_goal1 -> ../../secrets/goal.txt + * + */ + if (Functions.isWindows()) { + // no need to test mklink /H since we cannot create an hard link to a non-existing file + // and so you need to have access to the master file system directly which is already a problem + + String script = loadContentFromResource("outsideWorkspaceStructure.bat"); + p.getBuildersList().add(new BatchFile(script)); + } else { + String script = loadContentFromResource("outsideWorkspaceStructure.sh"); + p.getBuildersList().add(new Shell(script)); + } + + assertEquals(Result.SUCCESS, p.scheduleBuild2(0).get().getResult()); + + JenkinsRule.WebClient wc = j.createWebClient(); + wc.getOptions().setThrowExceptionOnFailingStatusCode(false); + + // the pattern allow us to search inside the files / folders, + // but it should not provide / leak information about non readable folders + + { // without the patch the otherFolder and to_secrets[1,2,3] will appear in the results (once) + Page page = wc.goTo(p.getUrl() + "ws/**/goal.txt", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + String workspaceContent = page.getWebResponse().getContentAsString(); + assertThat(workspaceContent, allOf( + // really not satisfying the query + not(containsString("public1.key")), + not(containsString("public2.key")), + // those following presences would have leak information that there is some file satisfying that pattern inside + not(containsString("to_secrets")), + not(containsString("to_secrets2")), + not(containsString("to_secrets3")) + )); + } + { // without the patch the otherFolder and to_secrets[1,2,3] will appear in the results (3 times each) + Page page = wc.goTo(p.getUrl() + "ws/**/public*.key", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + String workspaceContent = page.getWebResponse().getContentAsString(); + assertThat(workspaceContent, allOf( + containsString("public1.key"), + containsString("public2.key"), + // those following presences would have leak information that there is some file satisfying that pattern inside + not(containsString("otherFolder")), + not(containsString("to_secrets")), + not(containsString("to_secrets2")), + not(containsString("to_secrets3")) + )); + } + } + + // The hard links (mklink /H) to file are impossible to be detected and will allow a user to retrieve any file in the system + // to achieve that they should already have access to the system or the Script Console. + @Test + @Issue("SECURITY-904") + public void junctionAndSymlink_outsideWorkspace_areNotAllowed_windowsJunction() throws Exception { + Assume.assumeTrue(Functions.isWindows()); + + FreeStyleProject p = j.createFreeStyleProject(); + + File secretsFolder = new File(j.jenkins.getRootDir(), "secrets"); + File secretTarget = new File(secretsFolder, "goal.txt"); + String secretContent = "secret"; + FileUtils.write(secretTarget, secretContent); + + /* + * secrets/ + * goal.txt + * workspace/ + * intermediateFolder/ + * public2.key + * otherFolder/ + * to_secret3s -> symlink ../../../../secrets/ + * to_secret3j -> junction ../../../../secrets/ + * to_secret2s -> symlink ../../../secrets/ + * to_secret2j -> junction ../../../secrets/ + * to_secret_goal2 -> symlink ../../../secrets/goal.txt + * public1.key + * to_secret1s -> symlink ../../secrets/ + * to_secret1j -> junction ../../secrets/ + * to_secret_goal1 -> symlink ../../secrets/goal.txt + * + */ + String script = loadContentFromResource("outsideWorkspaceStructureWithJunctions.bat"); + p.getBuildersList().add(new BatchFile(script)); + + assertEquals(Result.SUCCESS, p.scheduleBuild2(0).get().getResult()); + + JenkinsRule.WebClient wc = j.createWebClient(); + wc.getOptions().setThrowExceptionOnFailingStatusCode(false); + { // workspace root must be reachable (regular case) + Page page = wc.goTo(p.getUrl() + "ws/", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + String workspaceContent = page.getWebResponse().getContentAsString(); + assertThat(workspaceContent, allOf( + containsString("public1.key"), + containsString("intermediateFolder"), + containsString("to_secrets1j"), + containsString("to_secrets1s"), + containsString("to_secrets_goal1"), + not(containsString("to_secrets2")), + not(containsString("to_secrets_goal2")) + )); + } + { // to_secrets1s not reachable + Page page = wc.goTo(p.getUrl() + "ws/to_secrets1s/", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); + } + { // to_secrets1j not reachable + Page page = wc.goTo(p.getUrl() + "ws/to_secrets1j/", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); + } + { // to_secrets_goal1 not reachable + Page page = wc.goTo(p.getUrl() + "ws/to_secrets_goal1/", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); + } + { // intermediateFolder must be reachable (regular case) + Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + String workspaceContent = page.getWebResponse().getContentAsString(); + assertThat(workspaceContent, allOf( + not(containsString("to_secrets1")), + not(containsString("to_secrets_goal1")), + containsString("to_secrets2s"), + containsString("to_secrets2j"), + containsString("to_secrets_goal2") + )); + } + { // to_secrets2s not reachable + Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/to_secrets2s/", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); + } + { // to_secrets2j not reachable + Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/to_secrets2j/", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); + } + { // using symbolic in the intermediate path + Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/to_secrets2s/master.key", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); + } + { // using symbolic in the intermediate path + Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/to_secrets2j/master.key", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); + } + { // to_secrets_goal2 not reachable + Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/to_secrets_goal2/", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); + } + + // pattern search feature + { // the pattern allow us to search inside the files / folders, + // without the patch the master.key from inside the outside symlinks would have been linked + Page page = wc.goTo(p.getUrl() + "ws/**/*.key", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + String workspaceContent = page.getWebResponse().getContentAsString(); + assertThat(workspaceContent, allOf( + not(containsString("master.key")), + containsString("public1.key"), + containsString("public2.key"), + containsString("intermediateFolder"), + not(containsString("otherFolder")), + not(containsString("to_secrets3j")), + not(containsString("to_secrets3s")), + not(containsString("to_secrets2j")), + not(containsString("to_secrets2s")), + not(containsString("to_secrets1j")), + not(containsString("to_secrets1s")) + )); + } + + // zip feature + { // all the outside folders / files are not included in the zip + Page zipPage = wc.goTo(p.getUrl() + "ws/*zip*/ws.zip", null); + assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + + List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); + assertThat(entryNames, containsInAnyOrder( + p.getName() + "/intermediateFolder/public2.key", + p.getName() + "/public1.key" + )); + } + { // all the outside folders / files are not included in the zip + Page zipPage = wc.goTo(p.getUrl() + "ws/intermediateFolder/*zip*/intermediateFolder.zip", null); + assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + + List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); + assertThat(entryNames, contains("intermediateFolder/public2.key")); + } + } + + private List getListOfEntriesInDownloadedZip(UnexpectedPage zipPage) throws Exception { + List result; + + File zipfile = null; + ZipFile readzip = null; + try { + zipfile = download(zipPage); + + readzip = new ZipFile(zipfile); + result = readzip.stream().map(ZipEntry::getName).collect(Collectors.toList()); + } + finally { + if (readzip != null) { + readzip.close(); + } + if (zipfile != null) { + zipfile.delete(); + } + } + return result; + } + + @Test + @Issue("SECURITY-904") + public void directSymlink_forTestingZip() throws Exception { + FreeStyleProject p = j.createFreeStyleProject(); + + assertEquals(Result.SUCCESS, p.scheduleBuild2(0).get().getResult()); + FilePath ws = p.getSomeWorkspace(); + + /* + * secrets/ + * goal.txt + * workspace/ + * /a1/to_secrets1 + * /b1/b2/to_secrets1 + * /c1/c2/c3/to_secrets1 + */ + File secretsFolder = new File(j.jenkins.getRootDir(), "secrets"); + FilePath a1 = ws.child("a1"); + a1.mkdirs(); + a1.child("to_secrets1").symlinkTo(secretsFolder.getAbsolutePath(), TaskListener.NULL); + FilePath b2 = ws.child("b1").child("b2"); + b2.mkdirs(); + b2.child("to_secrets2").symlinkTo(secretsFolder.getAbsolutePath(), TaskListener.NULL); + FilePath c3 = ws.child("c1").child("c2").child("c3"); + c3.mkdirs(); + c3.child("to_secrets3").symlinkTo(secretsFolder.getAbsolutePath(), TaskListener.NULL); + + JenkinsRule.WebClient wc = j.createWebClient(); + wc.getOptions().setThrowExceptionOnFailingStatusCode(false); + { + Page zipPage = wc.goTo(p.getUrl() + "ws/*zip*/ws.zip", null); + assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + + List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); + assertThat(entryNames, hasSize(0)); + } + { + Page zipPage = wc.goTo(p.getUrl() + "ws/a1/*zip*/a1.zip", null); + assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + + List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); + assertThat(entryNames, hasSize(0)); + } + { + Page zipPage = wc.goTo(p.getUrl() + "ws/b1/b2/*zip*/b2.zip", null); + assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + + List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); + assertThat(entryNames, hasSize(0)); + } + { + Page zipPage = wc.goTo(p.getUrl() + "ws/c1/c2/c3/*zip*/c3.zip", null); + assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + + List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); + assertThat(entryNames, hasSize(0)); + } + } + + @Test + @Issue("SECURITY-904") + public void symlink_insideWorkspace_areStillAllowed() throws Exception { + FreeStyleProject p = j.createFreeStyleProject(); + + // build once to have the workspace set up + assertEquals(Result.SUCCESS, p.scheduleBuild2(0).get().getResult()); + + File jobWorkspaceFolder = new File(new File(j.jenkins.getRootDir(), "workspace"), p.name); + File folderInsideWorkspace = new File(jobWorkspaceFolder, "asset"); + folderInsideWorkspace.mkdir(); + File fileTarget = new File(folderInsideWorkspace, "goal.txt"); + String publicContent = "not-secret"; + FileUtils.write(fileTarget, publicContent); + + /* + * workspace/ + * asset/ + * goal.txt + * intermediateFolder/ + * to_internal2 -> ../asset + * to_internal_goal2 -> ../asset/goal.txt + * to_internal1 -> ./asset/ + * to_internal_goal1 -> ./asset/goal.txt + */ + if (Functions.isWindows()) { + String script = loadContentFromResource("insideWorkspaceStructure.bat"); + p.getBuildersList().add(new BatchFile(script)); + } else { + String script = loadContentFromResource("insideWorkspaceStructure.sh"); + p.getBuildersList().add(new Shell(script)); + } + + assertEquals(Result.SUCCESS, p.scheduleBuild2(0).get().getResult()); + + JenkinsRule.WebClient wc = j.createWebClient(); + wc.getOptions().setThrowExceptionOnFailingStatusCode(false); + { // workspace root must be reachable (regular case) + Page page = wc.goTo(p.getUrl() + "ws/", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + String workspaceContent = page.getWebResponse().getContentAsString(); + assertThat(workspaceContent, allOf( + containsString("asset"), + containsString("to_internal1"), + containsString("to_internal_goal1"), + containsString("intermediateFolder"), + not(containsString("to_internal2")), + not(containsString("to_internal_goal2") + ))); + } + { // to_internal1 reachable + Page page = wc.goTo(p.getUrl() + "ws/to_internal1/", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + String workspaceContent = page.getWebResponse().getContentAsString(); + assertThat(workspaceContent, containsString("goal.txt")); + } + { // to_internal_goal1 reachable + Page page = wc.goTo(p.getUrl() + "ws/to_internal_goal1/", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + String workspaceContent = page.getWebResponse().getContentAsString(); + assertThat(workspaceContent, containsString(publicContent)); + } + { // to_internal2 reachable + Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/to_internal2/", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + String workspaceContent = page.getWebResponse().getContentAsString(); + assertThat(workspaceContent, containsString("goal.txt")); + } + { // to_internal_goal2 reachable + Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/to_internal_goal2/", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + String workspaceContent = page.getWebResponse().getContentAsString(); + assertThat(workspaceContent, containsString(publicContent)); + } + { // direct to goal + Page page = wc.goTo(p.getUrl() + "ws/asset/goal.txt/", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + String workspaceContent = page.getWebResponse().getContentAsString(); + assertThat(workspaceContent, containsString(publicContent)); + } + { // the zip will only contain folder from inside the workspace + Page page = wc.goTo(p.getUrl() + "ws/*zip*/ws.zip", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + } + { // the zip will only contain folder from inside the workspace + Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/*zip*/intermediateFolder.zip", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + } + { // the zip will only contain folder from inside the workspace + Page page = wc.goTo(p.getUrl() + "ws/asset/*zip*/asset.zip", null); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); + } + } + + private String loadContentFromResource(String fileNameInResources) throws IOException, URISyntaxException { + URL resourceUrl = DirectoryBrowserSupportTest.class.getResource(DirectoryBrowserSupportTest.class.getSimpleName() + "/" + fileNameInResources); + if (resourceUrl == null) { + fail("The resource with fileName " + fileNameInResources + " is not present in the resources of the test"); + } + File resourceFile = new File(resourceUrl.toURI()); + return FileUtils.readFileToString(resourceFile); + } } diff --git a/test/src/test/resources/hudson/model/DirectoryBrowserSupportSEC904Test/insideWorkspaceStructure.bat b/test/src/test/resources/hudson/model/DirectoryBrowserSupportTest/insideWorkspaceStructure.bat similarity index 100% rename from test/src/test/resources/hudson/model/DirectoryBrowserSupportSEC904Test/insideWorkspaceStructure.bat rename to test/src/test/resources/hudson/model/DirectoryBrowserSupportTest/insideWorkspaceStructure.bat diff --git a/test/src/test/resources/hudson/model/DirectoryBrowserSupportSEC904Test/insideWorkspaceStructure.sh b/test/src/test/resources/hudson/model/DirectoryBrowserSupportTest/insideWorkspaceStructure.sh similarity index 100% rename from test/src/test/resources/hudson/model/DirectoryBrowserSupportSEC904Test/insideWorkspaceStructure.sh rename to test/src/test/resources/hudson/model/DirectoryBrowserSupportTest/insideWorkspaceStructure.sh diff --git a/test/src/test/resources/hudson/model/DirectoryBrowserSupportSEC904Test/outsideWorkspaceStructure.bat b/test/src/test/resources/hudson/model/DirectoryBrowserSupportTest/outsideWorkspaceStructure.bat similarity index 100% rename from test/src/test/resources/hudson/model/DirectoryBrowserSupportSEC904Test/outsideWorkspaceStructure.bat rename to test/src/test/resources/hudson/model/DirectoryBrowserSupportTest/outsideWorkspaceStructure.bat diff --git a/test/src/test/resources/hudson/model/DirectoryBrowserSupportSEC904Test/outsideWorkspaceStructure.sh b/test/src/test/resources/hudson/model/DirectoryBrowserSupportTest/outsideWorkspaceStructure.sh similarity index 100% rename from test/src/test/resources/hudson/model/DirectoryBrowserSupportSEC904Test/outsideWorkspaceStructure.sh rename to test/src/test/resources/hudson/model/DirectoryBrowserSupportTest/outsideWorkspaceStructure.sh diff --git a/test/src/test/resources/hudson/model/DirectoryBrowserSupportSEC904Test/outsideWorkspaceStructureWithJunctions.bat b/test/src/test/resources/hudson/model/DirectoryBrowserSupportTest/outsideWorkspaceStructureWithJunctions.bat similarity index 100% rename from test/src/test/resources/hudson/model/DirectoryBrowserSupportSEC904Test/outsideWorkspaceStructureWithJunctions.bat rename to test/src/test/resources/hudson/model/DirectoryBrowserSupportTest/outsideWorkspaceStructureWithJunctions.bat