Skip to content

Commit

Permalink
Make sure DirectoryPathTree does not allow reading outside the root dir
Browse files Browse the repository at this point in the history
  • Loading branch information
aloubyansky committed Feb 16, 2022
1 parent 35f1bd3 commit 9ac9675
Show file tree
Hide file tree
Showing 9 changed files with 374 additions and 0 deletions.
5 changes: 5 additions & 0 deletions independent-projects/bootstrap/app-model/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,11 @@
<artifactId>junit-jupiter</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.assertj</groupId>
<artifactId>assertj-core</artifactId>
<scope>test</scope>
</dependency>

<dependency>
<groupId>io.quarkus</groupId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,24 @@ public void walk(PathVisitor visitor) {
}
}

private String ensureRelative(String path) {
if (path == null || path.isEmpty()) {
return path;
}
if (DirectoryPathTree.isWindowsAbsolutePath(path)) {
// We are allowing absolute paths on Linux but interpreting the root as the root of the tree.
// However, Windows absolute paths, including the disk part, are not expected.
throw new IllegalArgumentException(path + " does not appear to be a path relative to the root of the path tree");
}
if (path.charAt(0) == '/') {
return path.substring(1);
}
return path;
}

@Override
protected <T> T apply(String relativePath, Function<PathVisit, T> func, boolean manifestEnabled) {
relativePath = ensureRelative(relativePath);
if (!PathFilter.isVisible(pathFilter, relativePath)) {
return func.apply(null);
}
Expand All @@ -73,6 +89,7 @@ protected <T> T apply(String relativePath, Function<PathVisit, T> func, boolean

@Override
public void accept(String relativePath, Consumer<PathVisit> consumer) {
relativePath = ensureRelative(relativePath);
if (!PathFilter.isVisible(pathFilter, relativePath)) {
consumer.accept(null);
return;
Expand All @@ -97,6 +114,7 @@ public void accept(String relativePath, Consumer<PathVisit> consumer) {

@Override
public boolean contains(String relativePath) {
relativePath = ensureRelative(relativePath);
if (!PathFilter.isVisible(pathFilter, relativePath)) {
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import java.io.IOException;
import java.io.Serializable;
import java.nio.file.FileSystems;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
Expand All @@ -10,11 +11,25 @@
import java.util.Objects;
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.regex.Pattern;

public class DirectoryPathTree extends PathTreeWithManifest implements OpenPathTree, Serializable {

private static final long serialVersionUID = 2255956884896445059L;

private static final boolean USE_WINDOWS_ABSOLUTE_PATH_PATTERN = !FileSystems.getDefault().getSeparator().equals("/");

private static volatile Pattern windowsAbsolutePathPattern;

private static Pattern windowsAbsolutePathPattern() {
return windowsAbsolutePathPattern == null ? windowsAbsolutePathPattern = Pattern.compile("[a-zA-Z]:\\\\.*")
: windowsAbsolutePathPattern;
}

static boolean isWindowsAbsolutePath(String path) {
return USE_WINDOWS_ABSOLUTE_PATH_PATTERN ? windowsAbsolutePathPattern().matcher(path).matches() : false;
}

private Path dir;
private PathFilter pathFilter;

Expand Down Expand Up @@ -55,8 +70,40 @@ public void walk(PathVisitor visitor) {
PathTreeVisit.walk(dir, dir, pathFilter, getMultiReleaseMapping(), visitor);
}

private String ensureRelative(String path) {
if (path == null || path.isEmpty()) {
return path;
}
if (isWindowsAbsolutePath(path)) {
// We are allowing absolute paths on Linux but interpreting the root as the root of the tree.
// However, Windows absolute paths, including the disk part, are not expected.
throw new IllegalArgumentException(
path + " does not appear to be a path relative to the root of the path tree " + dir);
}
// this is to disallow reading outside the path tree root
if (path.contains("..")) {
final Path absolutePath = dir.resolve(path).normalize().toAbsolutePath();
if (absolutePath.startsWith(dir)) {
return dir.relativize(absolutePath).toString();
}
return null;
}
if (path.charAt(0) == '/') {
if (path.length() == 1) {
return "";
}
if (path.charAt(1) == '/') {
throw new IllegalArgumentException(
path + " does not appear to be a path relative to the root of the path tree " + dir);
}
return path.substring(1);
}
return path;
}

@Override
protected <T> T apply(String relativePath, Function<PathVisit, T> func, boolean manifestEnabled) {
relativePath = ensureRelative(relativePath);
if (!PathFilter.isVisible(pathFilter, relativePath)) {
return func.apply(null);
}
Expand All @@ -69,6 +116,7 @@ protected <T> T apply(String relativePath, Function<PathVisit, T> func, boolean

@Override
public void accept(String relativePath, Consumer<PathVisit> consumer) {
relativePath = ensureRelative(relativePath);
if (!PathFilter.isVisible(pathFilter, relativePath)) {
consumer.accept(null);
return;
Expand All @@ -83,6 +131,7 @@ public void accept(String relativePath, Consumer<PathVisit> consumer) {

@Override
public boolean contains(String relativePath) {
relativePath = ensureRelative(relativePath);
if (!PathFilter.isVisible(pathFilter, relativePath)) {
return false;
}
Expand All @@ -92,6 +141,7 @@ public boolean contains(String relativePath) {

@Override
public Path getPath(String relativePath) {
relativePath = ensureRelative(relativePath);
if (!PathFilter.isVisible(pathFilter, relativePath)) {
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ public class PathFilter implements Serializable {
private static final long serialVersionUID = -5712472676677054175L;

public static boolean isVisible(PathFilter filter, String path) {
if (path == null) {
return false;
}
if (filter == null) {
return true;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
package io.quarkus.paths;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.fail;

import java.io.IOException;
import java.io.UncheckedIOException;
import java.net.URL;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.HashSet;
import java.util.Set;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.condition.DisabledOnOs;
import org.junit.jupiter.api.condition.EnabledOnOs;
import org.junit.jupiter.api.condition.OS;

public class DirectoryPathTreeTest {

private static final String BASE_DIR = "paths/directory-path-tree";

private static volatile Path baseDir;

@BeforeAll
public static void staticInit() throws Exception {
final URL url = Thread.currentThread().getContextClassLoader().getResource(BASE_DIR);
if (url == null) {
throw new IllegalStateException("Failed to locate " + BASE_DIR + " on the classpath");
}
baseDir = Path.of(url.toURI()).toAbsolutePath();
if (!Files.exists(baseDir)) {
throw new IllegalStateException("Failed to locate " + baseDir);
}
}

@Test
public void acceptExistingPath() throws Exception {
final Path root = resolveTreeRoot("root");
final PathTree tree = PathTree.ofDirectoryOrArchive(root);
tree.accept("README.md", visit -> {
assertThat(visit).isNotNull();
assertThat(visit.getRelativePath("/")).isEqualTo("README.md");
assertThat(visit.getPath()).exists();
assertThat(visit.getRoot()).isEqualTo(root);
try {
assertThat(Files.readString(visit.getPath())).isEqualTo("test readme");
} catch (IOException e) {
throw new UncheckedIOException(e);
}
});
}

@Test
public void acceptNonExistentPath() throws Exception {
final Path root = resolveTreeRoot("root");
final PathTree tree = PathTree.ofDirectoryOrArchive(root);
tree.accept("non-existent", visit -> {
assertThat(visit).isNull();
});
}

@Test
public void acceptLegalAbsolutePath() throws Exception {
final Path root = resolveTreeRoot("root");
final PathTree tree = PathTree.ofDirectoryOrArchive(root);
tree.accept("/README.md", visit -> {
assertThat(visit).isNotNull();
assertThat(visit.getRelativePath("/")).isEqualTo("README.md");
assertThat(visit.getPath()).exists();
assertThat(visit.getRoot()).isEqualTo(root);
try {
assertThat(Files.readString(visit.getPath())).isEqualTo("test readme");
} catch (IOException e) {
throw new UncheckedIOException(e);
}
});
}

@Test
@DisabledOnOs(OS.WINDOWS)
public void acceptIllegalAbsolutePathOnLinux() throws Exception {
final Path root = resolveTreeRoot("root");
final PathTree tree = PathTree.ofDirectoryOrArchive(root);
final Path absolute = root.getParent().resolve("external.txt");
assertThat(absolute).exists();
tree.accept(absolute.toString(), visit -> {
assertThat(visit).isNull();
});
}

@Test
@EnabledOnOs(OS.WINDOWS)
public void acceptIllegalAbsolutePathOnWindows() throws Exception {
final Path root = resolveTreeRoot("root");
final PathTree tree = PathTree.ofDirectoryOrArchive(root);
final Path absolute = root.getParent().resolve("external.txt");
assertThat(absolute).exists();
try {
tree.accept(absolute.toString(), visit -> {
fail("Windows absolute paths are not allowed");
});
} catch (IllegalArgumentException e) {
// expected
}
}

@Test
public void acceptExistingRelativeNonNormalizedPath() throws Exception {
final Path root = resolveTreeRoot("root");
final PathTree tree = PathTree.ofDirectoryOrArchive(root);
tree.accept("../root/./other/../README.md", visit -> {
assertThat(visit).isNotNull();
assertThat(visit.getRelativePath("/")).isEqualTo("README.md");
assertThat(visit.getPath()).exists();
assertThat(visit.getRoot()).isEqualTo(root);
try {
assertThat(Files.readString(visit.getPath())).isEqualTo("test readme");
} catch (IOException e) {
throw new UncheckedIOException(e);
}
});
}

@Test
public void acceptNonExistentRelativeNonNormalizedPath() throws Exception {
final Path root = resolveTreeRoot("root");
final PathTree tree = PathTree.ofDirectoryOrArchive(root);
tree.accept("../root/./README.md/../non-existent.txt", visit -> {
assertThat(visit).isNull();
});
}

@Test
public void walk() throws Exception {
final Path root = resolveTreeRoot("root");
final PathTree tree = PathTree.ofDirectoryOrArchive(root);

final Set<String> visited = new HashSet<>();
final PathVisitor visitor = new PathVisitor() {
@Override
public void visitPath(PathVisit visit) {
visited.add(visit.getRelativePath("/"));
}
};
tree.walk(visitor);

assertThat(visited).isEqualTo(Set.of(
"",
"README.md",
"src",
"src/main",
"src/main/java",
"src/main/java/Main.java"));
}

/**
* Returns a path relative to src/test/resources/paths/directory-path-tree/
*
* @param relative relative path
* @return Path instance
*/
private Path resolveTreeRoot(String relative) {
return baseDir.resolve(relative);
}
}
Loading

0 comments on commit 9ac9675

Please sign in to comment.