Protect against invalid use of profiles

Author: ryanjbaxter

spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/EnvironmentController.java

@@ -36,7 +36,6 @@
 import org.springframework.cloud.config.environment.Environment;
 import org.springframework.cloud.config.environment.EnvironmentMediaType;
 import org.springframework.cloud.config.environment.PropertySource;
-import org.springframework.cloud.config.server.support.PathUtils;
 import org.springframework.http.HttpHeaders;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.MediaType;
@@ -51,6 +50,7 @@
 
 import static org.springframework.cloud.config.server.support.EnvironmentPropertySource.prepareEnvironment;
 import static org.springframework.cloud.config.server.support.EnvironmentPropertySource.resolvePlaceholders;
+import static org.springframework.cloud.config.server.support.PathUtils.isInvalidEncodedLocation;
 
 /**
  * @author Dave Syer
@@ -131,6 +131,9 @@ public Environment getEnvironment(String name, String profiles, String label, bo
 		try {
 			name = normalize(name);
 			label = normalize(label);
+			if (isInvalidEncodedLocation(profiles)) {
+				throw new InvalidEnvironmentRequestException("Invalid request");
+			}
 			Environment environment = this.repository.findOne(name, profiles, label, includeOrigin);
 			if (!this.acceptEmpty && (environment == null || environment.getPropertySources().isEmpty())) {
 				throw new EnvironmentNotFoundException("Profile Not found");
@@ -145,7 +148,7 @@ public Environment getEnvironment(String name, String profiles, String label, bo
 	}
 
 	private String normalize(String part) {
-		if (PathUtils.isInvalidEncodedLocation(part)) {
+		if (isInvalidEncodedLocation(part)) {
 			throw new InvalidEnvironmentRequestException("Invalid request");
 		}
 		return Environment.normalize(part);

spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/resource/ResourceController.java

@@ -31,6 +31,7 @@
 import org.springframework.cloud.config.environment.Environment;
 import org.springframework.cloud.config.server.encryption.ResourceEncryptor;
 import org.springframework.cloud.config.server.environment.EnvironmentRepository;
+import org.springframework.cloud.config.server.environment.InvalidEnvironmentRequestException;
 import org.springframework.core.io.Resource;
 import org.springframework.http.HttpHeaders;
 import org.springframework.http.MediaType;
@@ -48,6 +49,7 @@
 
 import static org.springframework.cloud.config.server.support.EnvironmentPropertySource.prepareEnvironment;
 import static org.springframework.cloud.config.server.support.EnvironmentPropertySource.resolvePlaceholders;
+import static org.springframework.cloud.config.server.support.PathUtils.isInvalidEncodedLocation;
 
 /**
  * An HTTP endpoint for serving up templated plain text resources from an underlying
@@ -142,6 +144,9 @@ synchronized String retrieve(ServletWebRequest request, String name, String prof
 			boolean resolvePlaceholders, String acceptedCharset) throws IOException {
 		name = Environment.normalize(name);
 		label = Environment.normalize(label);
+		if (isInvalidEncodedLocation(profile)) {
+			throw new InvalidEnvironmentRequestException("Invalid request");
+		}
 		Resource resource = this.resourceRepository.findOne(name, profile, label, path);
 		if (checkNotModified(request, resource)) {
 			// Content was not modified. Just return.
@@ -213,6 +218,9 @@ private synchronized byte[] binary(ServletWebRequest request, String name, Strin
 			String path) throws IOException {
 		name = Environment.normalize(name);
 		label = Environment.normalize(label);
+		if (isInvalidEncodedLocation(profile)) {
+			throw new InvalidEnvironmentRequestException("Invalid request");
+		}
 		Resource resource = this.resourceRepository.findOne(name, profile, label, path);
 		if (checkNotModified(request, resource)) {
 			// Content was not modified. Just return.

spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/support/AbstractScmAccessor.java

@@ -38,6 +38,8 @@
 import org.springframework.core.io.support.PathMatchingResourcePatternResolver;
 import org.springframework.util.FileSystemUtils;
 import org.springframework.util.StringUtils;
+import org.springframework.web.util.UriComponents;
+import org.springframework.web.util.UriComponentsBuilder;
 
 /**
  * Base class for components that want to access a source control management system.
@@ -150,9 +152,21 @@ public void setUri(String uri) {
 			// If there's no context path add one
 			uri = uri + "/";
 		}
+		validateNoTemplateInAuthority(uri);
 		this.uri = uri;
 	}
 
+	private void validateNoTemplateInAuthority(String urlTemplate) {
+		UriComponents components = UriComponentsBuilder.fromUriString(urlTemplate).build();
+		// If the port is templated this call will throw an Exception
+		components.getPort();
+		String host = components.getHost();
+		if (host != null && (host.contains("{") || host.contains("}"))) {
+			throw new IllegalArgumentException("Template placeholders not allowed in host: " + urlTemplate);
+		}
+
+	}
+
 	public File getBasedir() {
 		return this.basedir;
 	}

spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/EnvironmentControllerTests.java

@@ -554,6 +554,18 @@ public void nameWithPoundEncoded() {
 			.isInstanceOf(InvalidEnvironmentRequestException.class);
 	}
 
+	@Test
+	public void invalidProfileTests() {
+		assertThatThrownBy(() -> this.controller.labelled("application", "bar,..,foo", "label"))
+			.isInstanceOf(InvalidEnvironmentRequestException.class);
+		assertThatThrownBy(() -> this.controller.labelled("application", "..", "label"))
+			.isInstanceOf(InvalidEnvironmentRequestException.class);
+		assertThatThrownBy(() -> this.controller.labelled("application", "%2e%2e", "label"))
+			.isInstanceOf(InvalidEnvironmentRequestException.class);
+		assertThatThrownBy(() -> this.controller.labelled("application", "bar,%2e%2e,foo", "label"))
+			.isInstanceOf(InvalidEnvironmentRequestException.class);
+	}
+
 	abstract class MockMvcTestCases {
 
 		protected MockMvc mvc;

spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/MultipleJGitEnvironmentApplicationPlaceholderRepositoryTests.java

@@ -34,6 +34,7 @@
 import org.springframework.core.env.StandardEnvironment;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 /**
  * @author Dave Syer
@@ -115,6 +116,15 @@ public void otherMappingRepo() {
 		assertVersion(environment);
 	}
 
+	@Test
+	void invalidAuthorityTests() {
+		assertThatThrownBy(() -> createRepository("test", "*-config-repo", "http://{profile}:8080/test1-config-repo"))
+			.isInstanceOf(IllegalArgumentException.class);
+		assertThatThrownBy(
+				() -> createRepository("test", "*-config-repo", "http://localhost:{profile}/test1-config-repo"))
+			.isInstanceOf(IllegalStateException.class);
+	}
+
 	@Test
 	@Disabled("not supported yet (placeholders in search paths with lists)")
 	public void profilesInSearchPaths() {

spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/resource/ResourceControllerTests.java

@@ -28,6 +28,7 @@
 import org.springframework.boot.WebApplicationType;
 import org.springframework.boot.builder.SpringApplicationBuilder;
 import org.springframework.cloud.config.server.encryption.ResourceEncryptor;
+import org.springframework.cloud.config.server.environment.InvalidEnvironmentRequestException;
 import org.springframework.cloud.config.server.environment.NativeEnvironmentProperties;
 import org.springframework.cloud.config.server.environment.NativeEnvironmentRepository;
 import org.springframework.cloud.config.server.environment.NativeEnvironmentRepositoryTests;
@@ -37,6 +38,7 @@
 import org.springframework.web.context.request.ServletWebRequest;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.Mockito.mock;
@@ -368,4 +370,27 @@ public void setSearchLocationsAppendSlashByConstructor() {
 		assertThat(repo.getSearchLocations()[0]).isEqualTo("classpath:/test/");
 	}
 
+	@Test
+	public void invalidProfileTests() {
+		assertThatThrownBy(
+				() -> this.controller.retrieve("application", "bar,..,foo", "label", "template.json", true, "UTF-8"))
+			.isInstanceOf(InvalidEnvironmentRequestException.class);
+		assertThatThrownBy(() -> this.controller.binary("application", "bar,..,foo", "label", "template.json"))
+			.isInstanceOf(InvalidEnvironmentRequestException.class);
+		assertThatThrownBy(() -> this.controller.retrieve("application", "..", "label", "template.json", true, "UTF-8"))
+			.isInstanceOf(InvalidEnvironmentRequestException.class);
+		assertThatThrownBy(() -> this.controller.binary("application", "..", "label", "template.json"))
+			.isInstanceOf(InvalidEnvironmentRequestException.class);
+		assertThatThrownBy(
+				() -> this.controller.retrieve("application", "%2e%2e", "label", "template.json", true, "UTF-8"))
+			.isInstanceOf(InvalidEnvironmentRequestException.class);
+		assertThatThrownBy(() -> this.controller.binary("application", "%2e%2e", "label", "template.json"))
+			.isInstanceOf(InvalidEnvironmentRequestException.class);
+		assertThatThrownBy(() -> this.controller.retrieve("application", "bar,%2e%2e,foo", "label", "template.json",
+				true, "UTF-8"))
+			.isInstanceOf(InvalidEnvironmentRequestException.class);
+		assertThatThrownBy(() -> this.controller.binary("application", "bar,%2e%2e,foo", "label", "template.json"))
+			.isInstanceOf(InvalidEnvironmentRequestException.class);
+	}
+
 }