Skip to content

Commit

Permalink
Cleans invalid paths
Browse files Browse the repository at this point in the history
fixes gh-1355
  • Loading branch information
spencergibb committed Apr 16, 2019
1 parent 9230764 commit 3632fc6
Show file tree
Hide file tree
Showing 2 changed files with 170 additions and 13 deletions.
Expand Up @@ -17,14 +17,20 @@
package org.springframework.cloud.config.server.resource; package org.springframework.cloud.config.server.resource;


import java.io.IOException; import java.io.IOException;
import java.io.UnsupportedEncodingException;
import java.net.URLDecoder;
import java.util.Collection; import java.util.Collection;
import java.util.LinkedHashSet; import java.util.LinkedHashSet;
import java.util.Set; 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.cloud.config.server.environment.SearchPathLocator;
import org.springframework.context.ResourceLoaderAware; import org.springframework.context.ResourceLoaderAware;
import org.springframework.core.io.Resource; import org.springframework.core.io.Resource;
import org.springframework.core.io.ResourceLoader; import org.springframework.core.io.ResourceLoader;
import org.springframework.util.ResourceUtils;
import org.springframework.util.StringUtils; import org.springframework.util.StringUtils;


/** /**
Expand All @@ -35,6 +41,8 @@
public class GenericResourceRepository public class GenericResourceRepository
implements ResourceRepository, ResourceLoaderAware { implements ResourceRepository, ResourceLoaderAware {


private static final Log logger = LogFactory.getLog(GenericResourceRepository.class);

private ResourceLoader resourceLoader; private ResourceLoader resourceLoader;


private SearchPathLocator service; private SearchPathLocator service;
Expand All @@ -51,22 +59,28 @@ public void setResourceLoader(ResourceLoader resourceLoader) {
@Override @Override
public synchronized Resource findOne(String application, String profile, String label, public synchronized Resource findOne(String application, String profile, String label,
String path) { String path) {
String[] locations = this.service.getLocations(application, profile, label).getLocations();
try { if (StringUtils.hasText(path)) {
for (int i = locations.length; i-- > 0;) { String[] locations = this.service.getLocations(application, profile, label)
String location = locations[i]; .getLocations();
for (String local : getProfilePaths(profile, path)) { try {
Resource file = this.resourceLoader.getResource(location) for (int i = locations.length; i-- > 0; ) {
.createRelative(local); String location = locations[i];
if (file.exists() && file.isReadable()) { for (String local : getProfilePaths(profile, path)) {
return file; if (!isInvalidPath(local) && !isInvalidEncodedPath(local)) {
Resource file = this.resourceLoader.getResource(location)
.createRelative(local);
if (file.exists() && file.isReadable()) {
return file;
}
}
} }
} }
} }
} catch (IOException e) {
catch (IOException e) { throw new NoSuchResourceException(
throw new NoSuchResourceException( "Error : " + path + ". (" + e.getMessage() + ")");
"Error : " + path + ". (" + e.getMessage() + ")"); }
} }
throw new NoSuchResourceException("Not found: " + path); throw new NoSuchResourceException("Not found: " + path);
} }
Expand Down Expand Up @@ -94,4 +108,129 @@ private Collection<String> getProfilePaths(String profiles, String path) {
return paths; 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.
* <p>The default implementation replaces:
* <ul>
* <li>Backslash with forward slash.
* <li>Duplicate occurrences of slash with a single slash.
* <li>Any combination of leading slash and control characters (00-1F and 7F)
* with a single "/" or "". For example {@code " / // foo/bar"}
* becomes {@code "/foo/bar"}.
* </ul>
* @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:
* <ul>
* <li>Paths that contain "WEB-INF" or "META-INF"
* <li>Paths that contain "../" after a call to
* {@link org.springframework.util.StringUtils#cleanPath}.
* <li>Paths that represent a {@link org.springframework.util.ResourceUtils#isUrl
* valid URL} or would represent one after the leading slash is removed.
* </ul>
* <p><strong>Note:</strong> 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;
}
} }
Expand Up @@ -18,15 +18,19 @@


import org.junit.After; import org.junit.After;
import org.junit.Before; import org.junit.Before;
import org.junit.Rule;
import org.junit.Test; import org.junit.Test;
import org.junit.rules.ExpectedException;


import org.springframework.boot.WebApplicationType; import org.springframework.boot.WebApplicationType;
import org.springframework.boot.builder.SpringApplicationBuilder; 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.NativeEnvironmentProperties;
import org.springframework.cloud.config.server.environment.NativeEnvironmentRepository; import org.springframework.cloud.config.server.environment.NativeEnvironmentRepository;
import org.springframework.cloud.config.server.environment.NativeEnvironmentRepositoryTests; import org.springframework.cloud.config.server.environment.NativeEnvironmentRepositoryTests;
import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.ConfigurableApplicationContext;


import static org.hamcrest.Matchers.containsString;
import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNotNull;


/** /**
Expand All @@ -35,6 +39,12 @@
*/ */
public class GenericResourceRepositoryTests { public class GenericResourceRepositoryTests {


@Rule
public OutputCapture output = new OutputCapture();

@Rule
public ExpectedException exception = ExpectedException.none();

private GenericResourceRepository repository; private GenericResourceRepository repository;
private ConfigurableApplicationContext context; private ConfigurableApplicationContext context;
private NativeEnvironmentRepository nativeRepository; private NativeEnvironmentRepository nativeRepository;
Expand Down Expand Up @@ -79,4 +89,12 @@ public void locateMissingResource() {
assertNotNull(this.repository.findOne("blah", "default", "master", "foo.txt")); assertNotNull(this.repository.findOne("blah", "default", "master", "foo.txt"));
} }


@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"));
}

} }

0 comments on commit 3632fc6

Please sign in to comment.