Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GEOS-11176] Add validation to file wrapper resource paths #7222

Merged
merged 1 commit into from
Oct 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import java.io.OutputStream;
import java.net.URL;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
Expand Down Expand Up @@ -47,9 +48,28 @@ static final class ResourceAdaptor implements Resource {
final File file;

private ResourceAdaptor(File file) {
valid(file.getPath());
this.file = file.getAbsoluteFile();
}

/**
* GeoServer unit tests heavily utilize single period path components (e.g., ./something or
* target/./something) and Windows path separators when running on Windows so this method is
* a more lenient version of {@link Paths#valid(String)} that only checks for double period
* path components to prevent path traversal vulnerabilities.
*
* @param path the file path
* @return the file path
* @throws IllegalArgumentException If path contains '..'
*/
private static String valid(String path) {
if (path != null
&& Arrays.stream(Paths.convert(path).split("/")).anyMatch(".."::equals)) {
throw new IllegalArgumentException("Contains invalid '..' path: " + path);
}
return path;
}

@Override
public String path() {
return Paths.convert(file.getPath());
Expand Down Expand Up @@ -199,7 +219,7 @@ public Resource parent() {

@Override
public Resource get(String resourcePath) {
return new ResourceAdaptor(new File(file, resourcePath));
return new ResourceAdaptor(new File(file, valid(resourcePath)));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ static class ResourceAdaptor implements Resource {
private URL url;

public ResourceAdaptor(URL url) {
Paths.valid(url.getPath());
this.url = url;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@
import java.util.Collection;
import java.util.List;
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang3.SystemUtils;
import org.geoserver.platform.resource.Resource.Type;
import org.junit.Test;
import org.junit.experimental.theories.Theories;
import org.junit.experimental.theories.Theory;
import org.junit.runner.RunWith;
Expand Down Expand Up @@ -490,4 +492,24 @@ public void theoryRootIsAbsolute(String path) throws Exception {
final Resource res = getResource(Paths.convert(file.getPath()));
assertTrue(Paths.isAbsolute(res.path()));
}

@Test
public void testPathTraversal() {
assertInvalidPath("..");
assertInvalidPath("../foo/bar");
assertInvalidPath("foo/../bar");
assertInvalidPath("foo/bar/..");
}

@Test
public void testPathTraversalWindows() {
assumeTrue(SystemUtils.IS_OS_WINDOWS);
assertInvalidPath("..\\foo\\bar");
assertInvalidPath("foo\\..\\bar");
assertInvalidPath("foo\\bar\\..");
}

protected final void assertInvalidPath(String path) {
assertThrows(IllegalArgumentException.class, () -> getResource(path));
}
}
Loading