Skip to content
Permalink
Browse files Browse the repository at this point in the history
Merge pull request from GHSA-cjx7-399x-p2rj
* Don't allow resolving files using relative paths outside of the base

* Revert change around reading properties files

Co-authored-by: Lukas, Strmiska <strmik@gmail.com>
Co-authored-by: jameskleeh <james.kleeh@gmail.com>
  • Loading branch information
3 people committed Jul 14, 2021
1 parent d481bee commit a0cfeb1
Show file tree
Hide file tree
Showing 7 changed files with 159 additions and 68 deletions.
Expand Up @@ -28,6 +28,7 @@
import java.util.Optional;
import java.util.stream.Stream;


/**
* Loads resources from the file system.
*
Expand All @@ -36,54 +37,67 @@
*/
public class DefaultFileSystemResourceLoader implements FileSystemResourceLoader {

private final Optional<Path> baseDirPath;
private final Path baseDirPath;
private final boolean baseExists;

/**
* Default constructor.
*/
public DefaultFileSystemResourceLoader() {
this.baseDirPath = Optional.empty();
this.baseDirPath = null;
this.baseExists = true;
}

/**
* @param baseDirPath The base directory
*/
public DefaultFileSystemResourceLoader(File baseDirPath) {
this.baseDirPath = Optional.of(baseDirPath.toPath());
this(baseDirPath.toPath().normalize());
}

/**
* @param path The path
*/
public DefaultFileSystemResourceLoader(String path) {
this.baseDirPath = Optional.of(Paths.get(normalize(path)));
this(Paths.get(normalize(path)));
}

/**
* @param path The path
*/
public DefaultFileSystemResourceLoader(Path path) {
this.baseDirPath = Optional.of(path);
Path baseDirPath;
try {
baseDirPath = path.normalize().toRealPath();
} catch (IOException e) {
baseDirPath = null;
}
this.baseExists = baseDirPath != null;
this.baseDirPath = baseDirPath;
}

@Override
public Optional<InputStream> getResourceAsStream(String path) {
Path filePath = getFilePath(normalize(path));
try {
return Optional.of(Files.newInputStream(filePath));
} catch (IOException e) {
return Optional.empty();
if (isResolvableFile(filePath)) {
try {
return Optional.of(Files.newInputStream(filePath));
} catch (IOException e) {
return Optional.empty();
}
}
return Optional.empty();
}

@Override
public Optional<URL> getResource(String path) {
Path filePath = getFilePath(normalize(path));
if (Files.exists(filePath) && Files.isReadable(filePath) && !Files.isDirectory(filePath)) {
if (isResolvableFile(filePath)) {
try {
URL url = filePath.toUri().toURL();
return Optional.of(url);
} catch (MalformedURLException e) {
// ignore
}
}
return Optional.empty();
Expand All @@ -103,6 +117,10 @@ public ResourceLoader forBase(String basePath) {
return new DefaultFileSystemResourceLoader(basePath);
}

private boolean isResolvableFile(Path filePath) {
return startsWithBase(filePath) && Files.exists(filePath) && Files.isReadable(filePath) && !Files.isDirectory(filePath);
}

@SuppressWarnings("MagicNumber")
private static String normalize(String path) {
if (path == null) {
Expand All @@ -115,6 +133,23 @@ private static String normalize(String path) {
}

private Path getFilePath(String path) {
return baseDirPath.map(dir -> dir.resolve(path)).orElseGet(() -> Paths.get(path));
if (baseDirPath != null) {
return baseDirPath.resolve(path);
} else {
return Paths.get(path);
}
}

private boolean startsWithBase(Path path) {
if (baseDirPath != null) {
Path relativePath;
try {
relativePath = baseDirPath.resolve(path).toRealPath();
return relativePath.startsWith(baseDirPath);
} catch (IOException e) {
return false;
}
}
return baseExists;
}
}
Expand Up @@ -44,8 +44,10 @@ public class DefaultClassPathResourceLoader implements ClassPathResourceLoader {

private final ClassLoader classLoader;
private final String basePath;
private final URL baseURL;
private final Map<String, Boolean> isDirectoryCache = new ConcurrentLinkedHashMap.Builder<String, Boolean>()
.maximumWeightedCapacity(50).build();
private final boolean missingPath;

/**
* Default constructor.
Expand All @@ -65,6 +67,8 @@ public DefaultClassPathResourceLoader(ClassLoader classLoader) {
public DefaultClassPathResourceLoader(ClassLoader classLoader, String basePath) {
this.classLoader = classLoader;
this.basePath = normalize(basePath);
this.baseURL = basePath != null ? classLoader.getResource(normalize(basePath)) : null;
this.missingPath = basePath != null && baseURL == null;
}

/**
Expand All @@ -75,61 +79,83 @@ public DefaultClassPathResourceLoader(ClassLoader classLoader, String basePath)
*/
@Override
public Optional<InputStream> getResourceAsStream(String path) {
if (missingPath) {
return Optional.empty();
}

URL url = classLoader.getResource(prefixPath(path));
if (url != null) {
try {
URI uri = url.toURI();
if (uri.getScheme().equals("jar")) {
synchronized (DefaultClassPathResourceLoader.class) {
FileSystem fileSystem = null;
try {
if (startsWithBase(url)) {
try {
URI uri = url.toURI();
if (uri.getScheme().equals("jar")) {
synchronized (DefaultClassPathResourceLoader.class) {
FileSystem fileSystem = null;
try {
fileSystem = FileSystems.getFileSystem(uri);
} catch (FileSystemNotFoundException e) {
//no-op
}
if (fileSystem == null || !fileSystem.isOpen()) {
try {
fileSystem = FileSystems.newFileSystem(uri, Collections.emptyMap(), classLoader);
} catch (FileSystemAlreadyExistsException e) {
fileSystem = FileSystems.getFileSystem(uri);
} catch (FileSystemNotFoundException e) {
//no-op
}
}
Path pathObject = fileSystem.getPath(path);
if (Files.isDirectory(pathObject)) {
return Optional.empty();
}
return Optional.of(new ByteArrayInputStream(Files.readAllBytes(pathObject)));
} finally {
if (fileSystem != null && fileSystem.isOpen()) {
try {
fileSystem.close();
} catch (IOException e) {
if (LOG.isDebugEnabled()) {
LOG.debug("Error shutting down JAR file system [" + fileSystem + "]: " + e.getMessage(), e);
if (fileSystem == null || !fileSystem.isOpen()) {
try {
fileSystem = FileSystems.newFileSystem(uri, Collections.emptyMap(), classLoader);
} catch (FileSystemAlreadyExistsException e) {
fileSystem = FileSystems.getFileSystem(uri);
}
}
Path pathObject = fileSystem.getPath(path);
if (Files.isDirectory(pathObject)) {
return Optional.empty();
}
return Optional.of(new ByteArrayInputStream(Files.readAllBytes(pathObject)));
} finally {
if (fileSystem != null && fileSystem.isOpen()) {
try {
fileSystem.close();
} catch (IOException e) {
if (LOG.isDebugEnabled()) {
LOG.debug("Error shutting down JAR file system [" + fileSystem + "]: " + e.getMessage(), e);
}
}
}
}
}
} else if (uri.getScheme().equals("file")) {
Path pathObject = Paths.get(uri);
if (Files.isDirectory(pathObject)) {
return Optional.empty();
}
return Optional.of(Files.newInputStream(pathObject));
}
} else if (uri.getScheme().equals("file")) {
Path pathObject = Paths.get(uri);
if (Files.isDirectory(pathObject)) {
return Optional.empty();
} catch (URISyntaxException | IOException | ProviderNotFoundException e) {
if (LOG.isDebugEnabled()) {
LOG.debug("Error establishing whether path is a directory: " + e.getMessage(), e);
}
return Optional.of(Files.newInputStream(pathObject));
}
} catch (URISyntaxException | IOException | ProviderNotFoundException e) {
if (LOG.isDebugEnabled()) {
LOG.debug("Error establishing whether path is a directory: " + e.getMessage(), e);
}
}
}
// fallback to less sophisticated approach
if (path.indexOf('.') == -1) {
return Optional.empty();
}
return Optional.ofNullable(classLoader.getResourceAsStream(prefixPath(path)));
final URL u = getResource(path).orElse(null);
if (u != null) {
try {
return Optional.of(u.openStream());
} catch (IOException e) {
// fallback to empty
}
}
return Optional.empty();
}

private boolean startsWithBase(URL url) {
if (baseURL == null) {
return true;
} else {
return url.toExternalForm().startsWith(baseURL.toExternalForm());
}
}

/**
Expand All @@ -140,11 +166,17 @@ public Optional<InputStream> getResourceAsStream(String path) {
*/
@Override
public Optional<URL> getResource(String path) {
if (missingPath) {
return Optional.empty();
}

boolean isDirectory = isDirectory(path);

if (!isDirectory) {
URL url = classLoader.getResource(prefixPath(path));
return Optional.ofNullable(url);
if (url != null && startsWithBase(url)) {
return Optional.of(url);
}
}
return Optional.empty();
}
Expand All @@ -166,7 +198,9 @@ public Stream<URL> getResources(String path) {
Stream.Builder<URL> builder = Stream.builder();
while (all.hasMoreElements()) {
URL url = all.nextElement();
builder.accept(url);
if (startsWithBase(url)) {
builder.accept(url);
}
}
return builder.build();
}
Expand Down
Expand Up @@ -40,4 +40,23 @@ class ClasspathResourceLoaderSpec extends Specification {
"classpath:/foo" | "classpath:/bar.txt"
"classpath:foo" | "classpath:/bar.txt"
}

void "test resolving a classpath resource with a relative path"() {
given:
ClassPathResourceLoader loader = new DefaultClassPathResourceLoader(getClass().getClassLoader(), base)

expect:
loader.getResource(resource).isPresent() == present
loader.getResourceAsStream(resource).map({ InputStream io -> io.close(); io})
.isPresent() == present
loader.getResources(resource).findFirst().isPresent() == present

where:
base | resource | present
"classpath:foo" | "classpath:../foo/bar.txt" | true
"classpath:foo" | "classpath:../other/../foo/bar.txt" | true
"classpath:foo" | "classpath:../foo/../other/shouldNotAccess.txt" | false
"classpath:foo" | "classpath:../other/shouldNotAccess.txt" | false
"classpath:foo" | "classpath:foo/../../other/shouldNotAccess.txt" | false
}
}
Expand Up @@ -18,25 +18,29 @@ package io.micronaut.core.io
import io.micronaut.core.io.file.DefaultFileSystemResourceLoader
import io.micronaut.core.io.file.FileSystemResourceLoader
import spock.lang.Specification

import java.nio.file.Paths
import spock.lang.Unroll

class FileSystemResourceLoaderSpec extends Specification {

void "test resolving a resource"() {
@Unroll
void "test resolving a resource for #base and #resource"() {
given:
FileSystemResourceLoader loader = new DefaultFileSystemResourceLoader(base)
DefaultFileSystemResourceLoader loader = new DefaultFileSystemResourceLoader(base)

expect:
!loader.getResource(resource).isPresent()
loader.getResource(resource).isPresent() == presentViaResolver
new File(loader.normalize(base), loader.normalize(resource)).exists() == presentOnDisk

where:
base | resource
"." | "src"
"." | "file:src/main"
"src" | "main"
"file:src" | "main"
"file:src" | "file:main"
"file:src" | "file:/main"
base | resource | presentViaResolver | presentOnDisk
"." | "src" | false | true
"." | "file:src/main" | false | true
"src" | "main" | false | true
"file:src" | "main" | false | true
"file:src" | "file:main" | false | true
"file:src" | "file:/main" | false | true
"src/test/resources" | "foo/bar.txt" | true | true
"file:src/test/resources" | "../resources/foo/bar.txt" | true | true
"file:src/test/resources" | "../../../build.gradle" | false | true
}
}
Empty file.
Expand Up @@ -453,7 +453,7 @@ protected List<PropertySource> readPropertySourceListFromFiles(String files) {
}
order++;
} else {
throw new ConfigurationException("Unsupported properties file format: " + fileName);
throw new ConfigurationException("Unsupported properties file format: " + filePath);
}
}
}
Expand Down Expand Up @@ -583,12 +583,11 @@ private void loadPropertySourceFromLoader(String name, PropertySourceLoader prop
* @param fileName Name of the file to be used as property source name
* @param filePath Absolute file path
* @param propertySourceLoader The appropriate property source loader
* @throws ConfigurationException If unable to find the appropriate property soruce loader for the given file
* @throws ConfigurationException If unable to find the appropriate property source loader for the given file
*/
private Optional<Map<String, Object>> readPropertiesFromLoader(String fileName, String filePath, PropertySourceLoader propertySourceLoader) throws ConfigurationException {
ResourceResolver resourceResolver = new ResourceResolver();
Optional<ResourceLoader> resourceLoader = resourceResolver.getSupportingLoader(filePath);
ResourceLoader loader = resourceLoader.orElse(FileSystemResourceLoader.defaultLoader());
ResourceLoader loader = new ResourceResolver().getSupportingLoader(filePath)
.orElse(FileSystemResourceLoader.defaultLoader());
try {
Optional<InputStream> inputStream = loader.getResourceAsStream(filePath);
if (inputStream.isPresent()) {
Expand Down

0 comments on commit a0cfeb1

Please sign in to comment.