diff --git a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/resource/GenericResourceRepository.java b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/resource/GenericResourceRepository.java index ffe81ca3..e354554b 100644 --- a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/resource/GenericResourceRepository.java +++ b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/resource/GenericResourceRepository.java @@ -17,14 +17,20 @@ package org.springframework.cloud.config.server.resource; import java.io.IOException; +import java.io.UnsupportedEncodingException; +import java.net.URLDecoder; import java.util.Collection; import java.util.LinkedHashSet; import java.util.Set; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + import org.springframework.cloud.config.server.environment.SearchPathLocator; import org.springframework.context.ResourceLoaderAware; import org.springframework.core.io.Resource; import org.springframework.core.io.ResourceLoader; +import org.springframework.util.ResourceUtils; import org.springframework.util.StringUtils; /** @@ -35,6 +41,8 @@ import org.springframework.util.StringUtils; public class GenericResourceRepository implements ResourceRepository, ResourceLoaderAware { + private static final Log logger = LogFactory.getLog(GenericResourceRepository.class); + private ResourceLoader resourceLoader; private SearchPathLocator service; @@ -51,23 +59,28 @@ public class GenericResourceRepository @Override public synchronized Resource findOne(String application, String profile, String label, String path) { - String[] locations = this.service.getLocations(application, profile, label) - .getLocations(); - try { - for (int i = locations.length; i-- > 0;) { - String location = locations[i]; - for (String local : getProfilePaths(profile, path)) { - Resource file = this.resourceLoader.getResource(location) - .createRelative(local); - if (file.exists() && file.isReadable()) { - return file; + + if (StringUtils.hasText(path)) { + String[] locations = this.service.getLocations(application, profile, label) + .getLocations(); + try { + for (int i = locations.length; i-- > 0; ) { + String location = locations[i]; + for (String local : getProfilePaths(profile, path)) { + if (!isInvalidPath(local) && !isInvalidEncodedPath(local)) { + Resource file = this.resourceLoader.getResource(location) + .createRelative(local); + if (file.exists() && file.isReadable()) { + return file; + } + } } } } - } - catch (IOException e) { - throw new NoSuchResourceException( - "Error : " + path + ". (" + e.getMessage() + ")"); + catch (IOException e) { + throw new NoSuchResourceException( + "Error : " + path + ". (" + e.getMessage() + ")"); + } } throw new NoSuchResourceException("Not found: " + path); } @@ -95,4 +108,129 @@ public class GenericResourceRepository return paths; } + /** + * Check whether the given path contains invalid escape sequences. + * @param path the path to validate + * @return {@code true} if the path is invalid, {@code false} otherwise + */ + private boolean isInvalidEncodedPath(String path) { + if (path.contains("%")) { + try { + // Use URLDecoder (vs UriUtils) to preserve potentially decoded UTF-8 chars + String decodedPath = URLDecoder.decode(path, "UTF-8"); + if (isInvalidPath(decodedPath)) { + return true; + } + decodedPath = processPath(decodedPath); + if (isInvalidPath(decodedPath)) { + return true; + } + } + catch (IllegalArgumentException | UnsupportedEncodingException ex) { + // Should never happen... + } + } + return false; + } + + /** + * Process the given resource path. + *

The default implementation replaces: + *

+ * @since 3.2.12 + */ + protected String processPath(String path) { + path = StringUtils.replace(path, "\\", "/"); + path = cleanDuplicateSlashes(path); + return cleanLeadingSlash(path); + } + + + private String cleanDuplicateSlashes(String path) { + StringBuilder sb = null; + char prev = 0; + for (int i = 0; i < path.length(); i++) { + char curr = path.charAt(i); + try { + if ((curr == '/') && (prev == '/')) { + if (sb == null) { + sb = new StringBuilder(path.substring(0, i)); + } + continue; + } + if (sb != null) { + sb.append(path.charAt(i)); + } + } + finally { + prev = curr; + } + } + return sb != null ? sb.toString() : path; + } + + + private String cleanLeadingSlash(String path) { + boolean slash = false; + for (int i = 0; i < path.length(); i++) { + if (path.charAt(i) == '/') { + slash = true; + } + else if (path.charAt(i) > ' ' && path.charAt(i) != 127) { + if (i == 0 || (i == 1 && slash)) { + return path; + } + return (slash ? "/" + path.substring(i) : path.substring(i)); + } + } + return (slash ? "/" : ""); + } + + + /** + * Identifies invalid resource paths. By default rejects: + * + *

Note: this method assumes that leading, duplicate '/' + * or control characters (e.g. white space) have been trimmed so that the + * path starts predictably with a single '/' or does not have one. + * @param path the path to validate + * @return {@code true} if the path is invalid, {@code false} otherwise + * @since 3.0.6 + */ + protected boolean isInvalidPath(String path) { + if (path.contains("WEB-INF") || path.contains("META-INF")) { + if (logger.isWarnEnabled()) { + logger.warn("Path with \"WEB-INF\" or \"META-INF\": [" + path + "]"); + } + return true; + } + if (path.contains(":/")) { + String relativePath = (path.charAt(0) == '/' ? path.substring(1) : path); + if (ResourceUtils.isUrl(relativePath) || relativePath.startsWith("url:")) { + if (logger.isWarnEnabled()) { + logger.warn("Path represents URL or has \"url:\" prefix: [" + path + "]"); + } + return true; + } + } + if (path.contains("..") && StringUtils.cleanPath(path).contains("../")) { + if (logger.isWarnEnabled()) { + logger.warn("Path contains \"../\" after call to StringUtils#cleanPath: [" + path + "]"); + } + return true; + } + return false; + } } diff --git a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/resource/GenericResourceRepositoryTests.java b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/resource/GenericResourceRepositoryTests.java index 7bc5af5b..51c4b0f8 100644 --- a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/resource/GenericResourceRepositoryTests.java +++ b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/resource/GenericResourceRepositoryTests.java @@ -18,16 +18,20 @@ package org.springframework.cloud.config.server.resource; import org.junit.After; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.springframework.boot.WebApplicationType; import org.springframework.boot.builder.SpringApplicationBuilder; +import org.springframework.boot.test.rule.OutputCapture; import org.springframework.cloud.config.server.environment.NativeEnvironmentProperties; import org.springframework.cloud.config.server.environment.NativeEnvironmentRepository; import org.springframework.cloud.config.server.environment.NativeEnvironmentRepositoryTests; import org.springframework.context.ConfigurableApplicationContext; import static org.assertj.core.api.Assertions.assertThat; +import static org.hamcrest.Matchers.containsString; /** * @author Dave Syer @@ -35,6 +39,12 @@ import static org.assertj.core.api.Assertions.assertThat; */ public class GenericResourceRepositoryTests { + @Rule + public OutputCapture output = new OutputCapture(); + + @Rule + public ExpectedException exception = ExpectedException.none(); + private GenericResourceRepository repository; private ConfigurableApplicationContext context; @@ -85,4 +95,12 @@ public class GenericResourceRepositoryTests { .isNotNull(); } + @Test + public void invalidPath() { + this.exception.expect(NoSuchResourceException.class); + this.nativeRepository.setSearchLocations("file:./src/test/resources/test/{profile}"); + this.repository.findOne("blah", "local", "master", "..%2F..%2Fdata-jdbc.sql"); + this.output.expect(containsString("Path contains \"../\" after call to StringUtils#cleanPath")); + } + }