From 3632fc6f64e567286c42c5a2f1b8142bfde505c2 Mon Sep 17 00:00:00 2001
From: Spencer Gibb <[email protected]>
Date: Tue, 2 Apr 2019 14:16:10 -0400
Subject: [PATCH] Cleans invalid paths

fixes gh-1355
---
 .../resource/GenericResourceRepository.java   | 165 ++++++++++++++++--
 .../GenericResourceRepositoryTests.java       |  18 ++
 2 files changed, 170 insertions(+), 13 deletions(-)

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 1d7b9d117..0f3a071cb 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 @@
 public class GenericResourceRepository
 		implements ResourceRepository, ResourceLoaderAware {
 
+	private static final Log logger = LogFactory.getLog(GenericResourceRepository.class);
+
 	private ResourceLoader resourceLoader;
 
 	private SearchPathLocator service;
@@ -51,22 +59,28 @@ public void setResourceLoader(ResourceLoader resourceLoader) {
 	@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);
 	}
@@ -94,4 +108,129 @@ public synchronized Resource findOne(String application, String profile, String
 		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;
+	}
 }
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 7262a4ce4..1db865aee 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,15 +18,19 @@
 
 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.hamcrest.Matchers.containsString;
 import static org.junit.Assert.assertNotNull;
 
 /**
@@ -35,6 +39,12 @@
  */
 public class GenericResourceRepositoryTests {
 
+	@Rule
+	public OutputCapture output = new OutputCapture();
+
+	@Rule
+	public ExpectedException exception = ExpectedException.none();
+
 	private GenericResourceRepository repository;
 	private ConfigurableApplicationContext context;
 	private NativeEnvironmentRepository nativeRepository;
@@ -79,4 +89,12 @@ public void locateMissingResource() {
 		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"));
+	}
+
 }