|
213 | 213 | */ |
214 | 214 | public final class FilePath implements SerializableOnlyOverRemoting { |
215 | 215 |
|
| 216 | + /** |
| 217 | + * Set to {@code true} to disable validation to ensure that we do not attempt to extract paths that may allow determining the path to the destination directory. |
| 218 | + */ |
| 219 | + private static /* non-final for script console */ boolean ALLOW_REENTRY_PATH_TRAVERSAL = SystemProperties.getBoolean(FilePath.class.getName() + ".ALLOW_REENTRY_PATH_TRAVERSAL"); |
| 220 | + /** |
| 221 | + * Set to {@code true} to disable the fix for SECURITY-3657 that prevents path traversal from crafted tar files. |
| 222 | + */ |
| 223 | + private static /* non-final for script console */ boolean ALLOW_UNTAR_SYMLINK_RESOLUTION = SystemProperties.getBoolean(FilePath.class.getName() + ".ALLOW_UNTAR_SYMLINK_RESOLUTION"); |
| 224 | + |
216 | 225 | public enum DisplayOption implements OpenOption, CopyOption { |
217 | 226 | IGNORE_TMP_DIRS |
218 | 227 | } |
@@ -3051,26 +3060,54 @@ private static void readFromTar(String name, File baseDir, InputStream in) throw |
3051 | 3060 | /** |
3052 | 3061 | * Reads from a tar stream and stores obtained files to the base dir. |
3053 | 3062 | * Supports large files > 10 GB since 1.627. |
| 3063 | + * This prohibits any path traversal out of the base dir, as well as writing through any existing symlinks. |
3054 | 3064 | */ |
3055 | 3065 | private static void readFromTar(String name, File baseDir, InputStream in, Charset filenamesEncoding) throws IOException { |
3056 | | - |
| 3066 | + final File absoluteBaseDir = baseDir.getAbsoluteFile(); |
| 3067 | + final Path normalizedAbsoluteBaseDir = absoluteBaseDir.toPath().normalize(); |
3057 | 3068 | try (TarInputStream t = new TarInputStream(in, filenamesEncoding.name())) { |
3058 | 3069 | TarEntry te; |
3059 | 3070 | while ((te = t.getNextEntry()) != null) { |
3060 | | - File f = new File(baseDir, te.getName()); |
3061 | | - if (!f.toPath().normalize().startsWith(baseDir.toPath())) { |
3062 | | - throw new IOException( |
3063 | | - "Tar " + name + " contains illegal file name that breaks out of the target directory: " + te.getName()); |
| 3071 | + final String entryName = te.getName(); |
| 3072 | + if (!ALLOW_REENTRY_PATH_TRAVERSAL) { |
| 3073 | + if (new File(entryName).toPath().normalize().startsWith(Path.of(".."))) { |
| 3074 | + // catch relative path that would escape and then enter the destination dir again, like `../../../var/jenkins_home/...` |
| 3075 | + throw new IOException("Tar " + name + " contains entry that escapes destination directory: " + entryName); |
| 3076 | + } |
3064 | 3077 | } |
| 3078 | + |
| 3079 | + // We cannot replace 'f' with its canonical path here, otherwise, if it is a symlink, it becomes its link target and attempting to overwrite 'f' will have unintended behavior (JENKINS-67063) |
| 3080 | + File f = new File(baseDir, entryName).getAbsoluteFile(); |
| 3081 | + File parent = f.getParentFile(); |
| 3082 | + if (!f.toPath().normalize().startsWith(normalizedAbsoluteBaseDir)) { |
| 3083 | + // This covers both relative path traversal, and potential undefined File(String, String) constructor behavior when it takes a second argument that's absolute. |
| 3084 | + throw new IOException("Tar " + name + " contains entry that escapes destination directory: " + entryName); |
| 3085 | + } |
| 3086 | + |
| 3087 | + if (!ALLOW_UNTAR_SYMLINK_RESOLUTION) { |
| 3088 | + // getCanonicalFile doesn't follow symlinks on Windows, so do this the hard way: Check each ancestor up to the base dir for whether it's a symlink |
| 3089 | + File current = parent; |
| 3090 | + while (current != null && !current.equals(absoluteBaseDir)) { |
| 3091 | + if (Util.isSymlink(current)) { |
| 3092 | + throw new IOException("Tar " + name + " attempts to write to file with symlink in path: " + entryName); |
| 3093 | + } |
| 3094 | + current = current.getParentFile(); |
| 3095 | + } |
| 3096 | + } |
| 3097 | + |
3065 | 3098 | if (te.isDirectory()) { |
3066 | 3099 | mkdirs(f); |
3067 | 3100 | } else { |
3068 | | - File parent = f.getParentFile(); |
3069 | 3101 | if (parent != null) mkdirs(parent); |
3070 | 3102 |
|
3071 | 3103 | if (te.isSymbolicLink()) { |
3072 | 3104 | new FilePath(f).symlinkTo(te.getLinkName(), TaskListener.NULL); |
3073 | 3105 | } else { |
| 3106 | + if (!ALLOW_UNTAR_SYMLINK_RESOLUTION) { |
| 3107 | + if (Util.isSymlink(f)) { |
| 3108 | + throw new IOException("Tar '" + name + "' entry '" + entryName + "' would write through existing symlink: " + f); |
| 3109 | + } |
| 3110 | + } |
3074 | 3111 | IOUtils.copy(t, f); |
3075 | 3112 |
|
3076 | 3113 | Files.setLastModifiedTime(Util.fileToPath(f), FileTime.from(te.getModTime().toInstant())); |
|
0 commit comments