Skip to content

Commit

Permalink
[SECURITY-1807]
Browse files Browse the repository at this point in the history 
Co-authored-by: Kevin-CB <[email protected]>
  • Loading branch information
@jeffret-b @Kevin-CB @jenkinsci-cert-ci
2 people authored and jenkinsci-cert-ci committed Feb 23, 2023
1 parent a3f3114 commit 8045266
Show file tree
Hide file tree
Showing 10 changed files with 652 additions and 177 deletions.
233 changes: 175 additions & 58 deletions core/src/main/java/hudson/FilePath.java
View file

Large diffs are not rendered by default.

49 changes: 37 additions & 12 deletions core/src/main/java/hudson/model/DirectoryBrowserSupport.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
import java.io.Serializable;
import java.net.URL;
import java.nio.charset.StandardCharsets;
import java.nio.file.LinkOption;
import java.nio.file.OpenOption;
import java.text.Collator;
import java.util.ArrayList;
import java.util.Arrays;
Expand All @@ -49,6 +51,7 @@
import java.util.StringTokenizer;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import javax.servlet.ServletException;
Expand Down Expand Up @@ -83,6 +86,11 @@ public final class DirectoryBrowserSupport implements HttpResponse {
@SuppressFBWarnings(value = "MS_SHOULD_BE_FINAL", justification = "Accessible via System Groovy Scripts")
public static boolean ALLOW_SYMLINK_ESCAPE = SystemProperties.getBoolean(DirectoryBrowserSupport.class.getName() + ".allowSymlinkEscape");

@SuppressFBWarnings(value = "MS_SHOULD_BE_FINAL", justification = "Accessible via System Groovy Scripts")
public static boolean ALLOW_TMP_DISPLAY = SystemProperties.getBoolean(DirectoryBrowserSupport.class.getName() + ".allowTmpEscape");

private static final Pattern TMPDIR_PATTERN = Pattern.compile(".+@tmp/.*");

/**
* Escape hatch for the protection against SECURITY-2481. If enabled, the absolute paths on Windows will be allowed.
*/
Expand Down Expand Up @@ -264,7 +272,7 @@ private void serveFile(StaplerRequest req, StaplerResponse rsp, VirtualFile root
baseFile = root.child(base);
}

if (baseFile.hasSymlink(getNoFollowLinks())) {
if (baseFile.hasSymlink(getOpenOptions()) || hasTmpDir(baseFile, base, getOpenOptions())) {
rsp.sendError(HttpServletResponse.SC_NOT_FOUND);
return;
}
Expand All @@ -280,13 +288,13 @@ private void serveFile(StaplerRequest req, StaplerResponse rsp, VirtualFile root
includes = rest;
prefix = "";
}
baseFile.zip(rsp.getOutputStream(), includes, null, true, getNoFollowLinks(), prefix);
baseFile.zip(rsp.getOutputStream(), includes, null, true, prefix, getOpenOptions());
return;
}
if (plain) {
rsp.setContentType("text/plain;charset=UTF-8");
try (OutputStream os = rsp.getOutputStream()) {
for (VirtualFile kid : baseFile.list(getNoFollowLinks())) {
for (VirtualFile kid : baseFile.list(getOpenOptions())) {
os.write(kid.getName().getBytes(StandardCharsets.UTF_8));
if (kid.isDirectory()) {
os.write('/');
Expand All @@ -310,14 +318,16 @@ private void serveFile(StaplerRequest req, StaplerResponse rsp, VirtualFile root
List<List<Path>> glob = null;
boolean patternUsed = rest.length() > 0;
boolean containsSymlink = false;
if (patternUsed) {
boolean containsTmpDir = false;
if (patternUsed) {
// the rest is Ant glob pattern
glob = patternScan(baseFile, rest, createBackRef(restSize));
} else
if (serveDirIndex) {
// serve directory index
glob = baseFile.run(new BuildChildPaths(root, baseFile, req.getLocale()));
containsSymlink = baseFile.containsSymLinkChild(getNoFollowLinks());
containsSymlink = baseFile.containsSymLinkChild(getOpenOptions());
containsTmpDir = baseFile.containsTmpDirChild(getOpenOptions());
}

if (glob != null) {
Expand All @@ -333,6 +343,7 @@ private void serveFile(StaplerRequest req, StaplerResponse rsp, VirtualFile root
req.setAttribute("pattern", rest);
req.setAttribute("dir", baseFile);
req.setAttribute("showSymlinkWarning", containsSymlink);
req.setAttribute("showTmpDirWarning", containsTmpDir);
if (ResourceDomainConfiguration.isResourceRequest(req)) {
req.getView(this, "plaindir.jelly").forward(req, rsp);
} else {
Expand Down Expand Up @@ -378,7 +389,7 @@ private void serveFile(StaplerRequest req, StaplerResponse rsp, VirtualFile root
if (view) {
InputStream in;
try {
in = baseFile.open(getNoFollowLinks());
in = baseFile.open(getOpenOptions());
} catch (IOException ioe) {
rsp.sendError(HttpServletResponse.SC_NOT_FOUND);
return;
Expand Down Expand Up @@ -406,7 +417,7 @@ private void serveFile(StaplerRequest req, StaplerResponse rsp, VirtualFile root
}
InputStream in;
try {
in = baseFile.open(getNoFollowLinks());
in = baseFile.open(getOpenOptions());
} catch (IOException ioe) {
rsp.sendError(HttpServletResponse.SC_NOT_FOUND);
return;
Expand All @@ -429,6 +440,13 @@ public Boolean call() throws IOException {
}
}

private boolean hasTmpDir(VirtualFile baseFile, String base, OpenOption[] openOptions) {
if (FilePath.isTmpDir(baseFile.getName(), openOptions)) {
return true;
}
return FilePath.isIgnoreTmpDirs(openOptions) && TMPDIR_PATTERN.matcher(base).matches();
}

private List<List<Path>> keepReadabilityOnlyOnDescendants(VirtualFile root, boolean patternUsed, List<List<Path>> pathFragmentsList) {
Stream<List<Path>> pathFragmentsStream = pathFragmentsList.stream().map((List<Path> pathFragments) -> {
List<Path> mappedFragments = new ArrayList<>(pathFragments.size());
Expand Down Expand Up @@ -754,7 +772,7 @@ private static final class BuildChildPaths extends MasterToSlaveCallable<List<Li
private static List<List<Path>> buildChildPaths(VirtualFile cur, Locale locale) throws IOException {
List<List<Path>> r = new ArrayList<>();

VirtualFile[] files = cur.list(getNoFollowLinks());
VirtualFile[] files = cur.list(getOpenOptions());
Arrays.sort(files, new FileComparator(locale));

for (VirtualFile f : files) {
Expand All @@ -769,7 +787,7 @@ private static List<List<Path>> buildChildPaths(VirtualFile cur, Locale locale)
while (true) {
// files that don't start with '.' qualify for 'meaningful files', nor SCM related files
List<VirtualFile> sub = new ArrayList<>();
for (VirtualFile vf : f.list(getNoFollowLinks())) {
for (VirtualFile vf : f.list(getOpenOptions())) {
String name = vf.getName();
if (!name.startsWith(".") && !name.equals("CVS") && !name.equals(".svn")) {
sub.add(vf);
Expand All @@ -794,7 +812,7 @@ private static List<List<Path>> buildChildPaths(VirtualFile cur, Locale locale)
* @param baseRef String like "../../../" that cancels the 'rest' portion. Can be "./"
*/
private static List<List<Path>> patternScan(VirtualFile baseDir, String pattern, String baseRef) throws IOException {
Collection<String> files = baseDir.list(pattern, null, /* TODO what is the user expectation? */true, getNoFollowLinks());
Collection<String> files = baseDir.list(pattern, null, /* TODO what is the user expectation? */true, getOpenOptions());

if (!files.isEmpty()) {
List<List<Path>> r = new ArrayList<>(files.size());
Expand Down Expand Up @@ -837,8 +855,15 @@ private static void buildPathList(VirtualFile baseDir, VirtualFile filePath, Lis
pathList.add(path);
}

private static boolean getNoFollowLinks() {
return !ALLOW_SYMLINK_ESCAPE;
private static OpenOption[] getOpenOptions() {
List<OpenOption> options = new ArrayList<>();
if (!ALLOW_SYMLINK_ESCAPE) {
options.add(LinkOption.NOFOLLOW_LINKS);
}
if (!ALLOW_TMP_DISPLAY) {
options.add(FilePath.DisplayOption.IGNORE_TMP_DIRS);
}
return options.toArray(new OpenOption[0]);
}

private static final Logger LOGGER = Logger.getLogger(DirectoryBrowserSupport.class.getName());
Expand Down
7 changes: 6 additions & 1 deletion core/src/main/java/hudson/slaves/WorkspaceList.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ public void release() {
*/
@CheckForNull
public static FilePath tempDir(FilePath ws) {
return ws.sibling(ws.getName() + COMBINATOR + "tmp");
return ws.sibling(ws.getName() + TMP_DIR_SUFFIX);
}

private static final Logger LOGGER = Logger.getLogger(WorkspaceList.class.getName());
Expand All @@ -320,4 +320,9 @@ public static FilePath tempDir(FilePath ws) {
* @since 2.244
*/
public static final String COMBINATOR = SystemProperties.getString(WorkspaceList.class.getName(), "@");

/**
* Suffix for temporary folders.
*/
public static final String TMP_DIR_SUFFIX = COMBINATOR + "tmp";
}
13 changes: 7 additions & 6 deletions core/src/main/java/hudson/util/DirScanner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import java.io.FileFilter;
import java.io.IOException;
import java.io.Serializable;
import java.nio.file.OpenOption;
import java.util.HashSet;
import java.util.Set;
import org.apache.tools.ant.BuildException;
Expand Down Expand Up @@ -106,25 +107,25 @@ public static class Glob extends DirScanner {
private final String includes, excludes;

private boolean useDefaultExcludes = true;
private final boolean followSymlinks;
private OpenOption[] openOptions;

public Glob(String includes, String excludes) {
this(includes, excludes, true, true);
this(includes, excludes, true, new OpenOption[0]);
}

public Glob(String includes, String excludes, boolean useDefaultExcludes) {
this(includes, excludes, useDefaultExcludes, true);
this(includes, excludes, useDefaultExcludes, new OpenOption[0]);
}

/**
* @since 2.275 and 2.263.2
*/
@Restricted(NoExternalUse.class)
public Glob(String includes, String excludes, boolean useDefaultExcludes, boolean followSymlinks) {
public Glob(String includes, String excludes, boolean useDefaultExcludes, OpenOption... openOptions) {
this.includes = includes;
this.excludes = excludes;
this.useDefaultExcludes = useDefaultExcludes;
this.followSymlinks = followSymlinks;
this.openOptions = openOptions;
}

@Override
Expand All @@ -136,7 +137,7 @@ public void scan(File dir, FileVisitor visitor) throws IOException {
}

FileSet fs = Util.createFileSet(dir, includes, excludes);
fs.setFollowSymlinks(followSymlinks);
fs.setFollowSymlinks(!FilePath.isNoFollowLink(openOptions));
fs.setDefaultexcludes(useDefaultExcludes);

if (dir.exists()) {
Expand Down
27 changes: 13 additions & 14 deletions core/src/main/java/hudson/util/io/ArchiverFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.io.IOException;
import java.io.OutputStream;
import java.io.Serializable;
import java.nio.file.OpenOption;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;

Expand Down Expand Up @@ -65,13 +66,14 @@ public abstract class ArchiverFactory implements Serializable {
public static ArchiverFactory ZIP = new ZipArchiverFactory();

/**
* Zip format, without following symlinks.
* Zip format, with prefix and optional OpenOptions.
* @param prefix The portion of file path that will be added at the beginning of the relative path inside the archive.
* If non-empty, a trailing forward slash will be enforced.
* @param openOptions the options to apply when opening files.
*/
@Restricted(NoExternalUse.class)
public static ArchiverFactory createZipWithoutSymlink(String prefix) {
return new ZipWithoutSymLinksArchiverFactory(prefix);
public static ArchiverFactory createZipWithPrefix(String prefix, OpenOption... openOptions) {
return new ZipArchiverFactory(prefix, openOptions);
}

private static final class TarArchiverFactory extends ArchiverFactory {
Expand All @@ -91,26 +93,23 @@ public Archiver create(OutputStream out) throws IOException {
}

private static final class ZipArchiverFactory extends ArchiverFactory {
@NonNull
@Override
public Archiver create(OutputStream out) {
return new ZipArchiver(out);
}

private static final long serialVersionUID = 1L;
}

private static final class ZipWithoutSymLinksArchiverFactory extends ArchiverFactory {
private final String prefix;
private final OpenOption[] openOptions;

ZipArchiverFactory() {
this(null);
}

ZipWithoutSymLinksArchiverFactory(String prefix) {
ZipArchiverFactory(String prefix, OpenOption... openOptions) {
this.prefix = prefix;
this.openOptions = openOptions;
}

@NonNull
@Override
public Archiver create(OutputStream out) {
return new ZipArchiver(out, true, prefix);
return new ZipArchiver(out, prefix, openOptions);
}

private static final long serialVersionUID = 1L;
Expand Down
10 changes: 5 additions & 5 deletions core/src/main/java/hudson/util/io/ZipArchiver.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

package hudson.util.io;

import hudson.FilePath;
import hudson.Util;
import hudson.util.FileVisitor;
import hudson.util.IOUtils;
Expand All @@ -33,7 +34,6 @@
import java.io.OutputStream;
import java.nio.file.Files;
import java.nio.file.InvalidPathException;
import java.nio.file.LinkOption;
import java.nio.file.OpenOption;
import java.nio.file.attribute.BasicFileAttributes;
import org.apache.commons.lang.StringUtils;
Expand All @@ -55,20 +55,20 @@ final class ZipArchiver extends Archiver {
private final String prefix;

ZipArchiver(OutputStream out) {
this(out, false, "");
this(out, "");
}

// Restriction added for clarity, it's a package class, you should not use it outside of Jenkins core
@Restricted(NoExternalUse.class)
ZipArchiver(OutputStream out, boolean failOnSymLink, String prefix) {
ZipArchiver(OutputStream out, String prefix, OpenOption... openOptions) {
this.openOptions = openOptions;
if (StringUtils.isBlank(prefix)) {
this.prefix = "";
} else {
this.prefix = Util.ensureEndsWith(prefix, "/");
}

zip = new ZipOutputStream(out);
openOptions = failOnSymLink ? new LinkOption[]{LinkOption.NOFOLLOW_LINKS} : new OpenOption[0];
zip.setEncoding(System.getProperty("file.encoding"));
zip.setUseZip64(Zip64Mode.AsNeeded);
}
Expand Down Expand Up @@ -97,7 +97,7 @@ public void visit(final File f, final String _relativePath) throws IOException {
fileZipEntry.setTime(basicFileAttributes.lastModifiedTime().toMillis());
fileZipEntry.setSize(basicFileAttributes.size());
zip.putNextEntry(fileZipEntry);
try (InputStream in = Files.newInputStream(f.toPath(), openOptions)) {
try (InputStream in = FilePath.openInputStream(f, openOptions)) {
int len;
while ((len = in.read(buf)) >= 0)
zip.write(buf, 0, len);
Expand Down
Loading

0 comments on commit 8045266

Please sign in to comment.