Skip to content

Commit 7709cd0

Browse files
spencergibbryanjbaxter
spencergibb
authored and
ryanjbaxter
committed
Validates name, profile and path
1 parent 80de5a5 commit 7709cd0

7 files changed

Lines changed: 192 additions & 3 deletions

File tree

‎spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/encryption/EncryptionController.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import org.apache.commons.logging.Log;
2727
import org.apache.commons.logging.LogFactory;
2828

29+
import org.springframework.cloud.config.server.environment.InvalidEnvironmentRequestException;
2930
import org.springframework.cloud.context.encrypt.KeyFormatException;
3031
import org.springframework.http.HttpStatus;
3132
import org.springframework.http.MediaType;
@@ -43,6 +44,9 @@
4344
import org.springframework.web.bind.annotation.RequestMapping;
4445
import org.springframework.web.bind.annotation.RestController;
4546

47+
import static org.springframework.cloud.config.server.support.PathUtils.isInvalidEncodedLocation;
48+
import static org.springframework.cloud.config.server.support.PathUtils.isInvalidProfiles;
49+
4650
/**
4751
* @author Dave Syer
4852
* @author Tim Ysewyn
@@ -144,6 +148,13 @@ public String decrypt(@PathVariable String name, @PathVariable String profiles,
144148
}
145149

146150
private TextEncryptor getEncryptor(String name, String profiles, String data) {
151+
if (isInvalidEncodedLocation(name)) {
152+
throw new InvalidEnvironmentRequestException("Invalid request");
153+
}
154+
if (isInvalidProfiles(profiles)) {
155+
throw new InvalidEnvironmentRequestException("Invalid request");
156+
}
157+
147158
if (encryptorLocator == null) {
148159
if (logger.isDebugEnabled()) {
149160
logger.debug("Text encryptorLocator is null.");

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
import static org.springframework.cloud.config.server.support.EnvironmentPropertySource.prepareEnvironment;
5252
import static org.springframework.cloud.config.server.support.EnvironmentPropertySource.resolvePlaceholders;
5353
import static org.springframework.cloud.config.server.support.PathUtils.isInvalidEncodedLocation;
54+
import static org.springframework.cloud.config.server.support.PathUtils.isInvalidProfiles;
5455

5556
/**
5657
* @author Dave Syer
@@ -131,7 +132,7 @@ public Environment getEnvironment(String name, String profiles, String label, bo
131132
try {
132133
name = normalize(name);
133134
label = normalize(label);
134-
if (isInvalidEncodedLocation(profiles)) {
135+
if (isInvalidProfiles(profiles)) {
135136
throw new InvalidEnvironmentRequestException("Invalid request");
136137
}
137138
Environment environment = this.repository.findOne(name, profiles, label, includeOrigin);

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
import static org.springframework.cloud.config.server.support.EnvironmentPropertySource.prepareEnvironment;
5151
import static org.springframework.cloud.config.server.support.EnvironmentPropertySource.resolvePlaceholders;
5252
import static org.springframework.cloud.config.server.support.PathUtils.isInvalidEncodedLocation;
53+
import static org.springframework.cloud.config.server.support.PathUtils.isInvalidProfiles;
5354

5455
/**
5556
* An HTTP endpoint for serving up templated plain text resources from an underlying
@@ -144,9 +145,10 @@ synchronized String retrieve(ServletWebRequest request, String name, String prof
144145
boolean resolvePlaceholders, String acceptedCharset) throws IOException {
145146
name = normalize(name);
146147
label = normalize(label);
147-
if (isInvalidEncodedLocation(profile)) {
148+
if (isInvalidProfiles(profile)) {
148149
throw new InvalidEnvironmentRequestException("Invalid request");
149150
}
151+
path = normalize(path);
150152
Resource resource = this.resourceRepository.findOne(name, profile, label, path);
151153
if (checkNotModified(request, resource)) {
152154
// Content was not modified. Just return.
@@ -225,9 +227,10 @@ private synchronized byte[] binary(ServletWebRequest request, String name, Strin
225227
String path) throws IOException {
226228
name = normalize(name);
227229
label = normalize(label);
228-
if (isInvalidEncodedLocation(profile)) {
230+
if (isInvalidProfiles(profile)) {
229231
throw new InvalidEnvironmentRequestException("Invalid request");
230232
}
233+
path = normalize(path);
231234
Resource resource = this.resourceRepository.findOne(name, profile, label, path);
232235
if (checkNotModified(request, resource)) {
233236
// Content was not modified. Just return.

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

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,20 @@
2020
import java.net.URLDecoder;
2121
import java.net.URLEncoder;
2222
import java.nio.charset.StandardCharsets;
23+
import java.util.Arrays;
24+
import java.util.Collection;
2325
import java.util.List;
26+
import java.util.Map;
27+
import java.util.stream.Collectors;
2428

2529
import org.apache.commons.logging.Log;
2630
import org.apache.commons.logging.LogFactory;
2731

32+
import org.springframework.cloud.config.environment.Environment;
2833
import org.springframework.core.io.ClassPathResource;
2934
import org.springframework.core.io.Resource;
3035
import org.springframework.core.io.UrlResource;
36+
import org.springframework.util.Assert;
3137
import org.springframework.util.ObjectUtils;
3238
import org.springframework.util.ResourceUtils;
3339
import org.springframework.util.StringUtils;
@@ -36,9 +42,53 @@ public abstract class PathUtils {
3642

3743
private static final Log logger = LogFactory.getLog(PathUtils.class);
3844

45+
private static final String PROFILE_ALLOWED_CHARS = "-_.+@";
46+
3947
private PathUtils() {
4048
}
4149

50+
public static boolean isInvalidProfiles(String profiles) {
51+
if (ObjectUtils.isEmpty(profiles)) {
52+
return false;
53+
}
54+
try {
55+
if (profiles.contains(",")) {
56+
validateProfile(StringUtils.commaDelimitedListToSet(profiles));
57+
}
58+
else {
59+
validateProfile(profiles);
60+
}
61+
}
62+
catch (IllegalStateException ex) {
63+
return true;
64+
}
65+
return isInvalidEncodedLocation(profiles);
66+
}
67+
68+
private static void validateProfile(Object value) {
69+
if (value instanceof Collection<?> list) {
70+
list.forEach(PathUtils::validateProfile);
71+
return;
72+
}
73+
if (value instanceof Map<?, ?> map) {
74+
map.forEach((k, v) -> validateProfile(v));
75+
return;
76+
}
77+
String profile = (value != null) ? value.toString() : null;
78+
Assert.state(StringUtils.hasText(profile), "Invalid empty profile");
79+
for (int i = 0; i < profile.length(); i++) {
80+
int codePoint = profile.codePointAt(i);
81+
boolean isAllowedChar = PROFILE_ALLOWED_CHARS.indexOf(codePoint) != -1;
82+
Assert.state(isAllowedChar || Character.isLetterOrDigit(codePoint),
83+
() -> "Profile '%s' must contain a letter, digit or allowed char (%s)".formatted(profile,
84+
Arrays.stream(PROFILE_ALLOWED_CHARS.split(""))
85+
.collect(Collectors.joining("', '", "'", "'"))));
86+
Assert.state((i > 0 && i < profile.length() - 1) || Character.isLetterOrDigit(codePoint),
87+
() -> "Profile '%s' must start and end with a letter or digit".formatted(profile));
88+
}
89+
90+
}
91+
4292
/**
4393
* Check whether the given location contains invalid escape sequences.
4494
* @param location the location to validate
@@ -80,6 +130,13 @@ private static boolean isInvalidLocation(String location) {
80130
logger.warn("Location contains \"#\"");
81131
}
82132
}
133+
if (!isInvalid) {
134+
// locations can't start with slash
135+
isInvalid = location.startsWith(Environment.SLASH_PLACEHOLDER);
136+
if (isInvalid && logger.isWarnEnabled()) {
137+
logger.warn("Location starts with \"/\"");
138+
}
139+
}
83140

84141
return isInvalid;
85142
}

‎spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/encryption/EncryptionControllerTests.java‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,14 @@
2121
import org.junit.jupiter.api.Test;
2222
import org.mockito.ArgumentCaptor;
2323

24+
import org.springframework.cloud.config.server.environment.InvalidEnvironmentRequestException;
2425
import org.springframework.http.MediaType;
2526
import org.springframework.security.crypto.encrypt.Encryptors;
2627
import org.springframework.security.crypto.encrypt.RsaSecretEncryptor;
2728
import org.springframework.security.crypto.encrypt.TextEncryptor;
2829

2930
import static org.assertj.core.api.Assertions.assertThat;
31+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
3032
import static org.assertj.core.api.AssertionsForClassTypes.assertThatExceptionOfType;
3133
import static org.mockito.ArgumentMatchers.anyString;
3234
import static org.mockito.Mockito.atLeastOnce;
@@ -158,4 +160,52 @@ public TextEncryptor locate(Map<String, String> keys) {
158160
assertThat(decrypt).as("Wrong decrypted plaintext: " + decrypt).isEqualTo("foo bar");
159161
}
160162

163+
@Test
164+
public void getPublicKeyNameWithDotDot() {
165+
this.controller = new EncryptionController(new SingleTextEncryptorLocator(new RsaSecretEncryptor()));
166+
assertThatThrownBy(() -> this.controller.getPublicKey("../app", "default"))
167+
.isInstanceOf(InvalidEnvironmentRequestException.class)
168+
.hasMessageContaining("Invalid request");
169+
}
170+
171+
@Test
172+
public void getPublicKeyProfilesWithDotDot() {
173+
this.controller = new EncryptionController(new SingleTextEncryptorLocator(new RsaSecretEncryptor()));
174+
assertThatThrownBy(() -> this.controller.getPublicKey("app", "../default"))
175+
.isInstanceOf(InvalidEnvironmentRequestException.class)
176+
.hasMessageContaining("Invalid request");
177+
}
178+
179+
@Test
180+
public void decryptNameWithDotDot() {
181+
this.controller = new EncryptionController(new SingleTextEncryptorLocator(new RsaSecretEncryptor()));
182+
assertThatThrownBy(() -> this.controller.decrypt("../app", "default", "hello", MediaType.TEXT_PLAIN))
183+
.isInstanceOf(InvalidEnvironmentRequestException.class)
184+
.hasMessageContaining("Invalid request");
185+
}
186+
187+
@Test
188+
public void decryptProfilesWithDotDot() {
189+
this.controller = new EncryptionController(new SingleTextEncryptorLocator(new RsaSecretEncryptor()));
190+
assertThatThrownBy(() -> this.controller.decrypt("app", "../default", "hello", MediaType.TEXT_PLAIN))
191+
.isInstanceOf(InvalidEnvironmentRequestException.class)
192+
.hasMessageContaining("Invalid request");
193+
}
194+
195+
@Test
196+
public void encryptNameWithDotDot() {
197+
this.controller = new EncryptionController(new SingleTextEncryptorLocator(new RsaSecretEncryptor()));
198+
assertThatThrownBy(() -> this.controller.encrypt("../app", "default", "hello", MediaType.TEXT_PLAIN))
199+
.isInstanceOf(InvalidEnvironmentRequestException.class)
200+
.hasMessageContaining("Invalid request");
201+
}
202+
203+
@Test
204+
public void encryptProfilesWithDotDot() {
205+
this.controller = new EncryptionController(new SingleTextEncryptorLocator(new RsaSecretEncryptor()));
206+
assertThatThrownBy(() -> this.controller.encrypt("app", "../default", "hello", MediaType.TEXT_PLAIN))
207+
.isInstanceOf(InvalidEnvironmentRequestException.class)
208+
.hasMessageContaining("Invalid request");
209+
}
210+
161211
}

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -524,12 +524,24 @@ private void whenPlaceholdersSystemPropsWithDefault() {
524524
when(this.repository.findOne("foo", "bar", null, false)).thenReturn(this.environment);
525525
}
526526

527+
@Test
528+
public void nameStartsWithSlash() {
529+
assertThatThrownBy(() -> this.controller.labelled("(_)spam", "bar", null))
530+
.isInstanceOf(InvalidEnvironmentRequestException.class);
531+
}
532+
527533
@Test
528534
public void labelWithPreviousDirectory() {
529535
assertThatThrownBy(() -> this.controller.labelled("foo", "bar", "..(_).."))
530536
.isInstanceOf(InvalidEnvironmentRequestException.class);
531537
}
532538

539+
@Test
540+
public void labelStartsWithSlash() {
541+
assertThatThrownBy(() -> this.controller.labelled("foo", "bar", "(_)spam"))
542+
.isInstanceOf(InvalidEnvironmentRequestException.class);
543+
}
544+
533545
@Test
534546
public void labelWithPreviousDirectoryEncodedParenthesis() {
535547
assertThatThrownBy(() -> this.controller.labelled("foo", "bar", "..%28_%29.."))
@@ -564,6 +576,8 @@ public void invalidProfileTests() {
564576
.isInstanceOf(InvalidEnvironmentRequestException.class);
565577
assertThatThrownBy(() -> this.controller.labelled("application", "bar,%2e%2e,foo", "label"))
566578
.isInstanceOf(InvalidEnvironmentRequestException.class);
579+
assertThatThrownBy(() -> this.controller.labelled("application", "bar%2Ffoo", "label"))
580+
.isInstanceOf(InvalidEnvironmentRequestException.class);
567581
}
568582

569583
abstract class MockMvcTestCases {

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

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,14 @@ public void applicationPlaceholderWithSlashNullLabel() throws Exception {
148148
assertThat(resource).isEqualToIgnoringNewLines("foo: dev_bar/spam");
149149
}
150150

151+
@Test
152+
public void applicationPlaceholderStartsWithSlash() {
153+
this.environmentRepository.setSearchLocations("classpath:/test/{label}");
154+
assertThatThrownBy(() -> this.controller.retrieve("(_)dev", "bar", null, "foo.txt", true, "UTF-8"))
155+
.isInstanceOf(InvalidEnvironmentRequestException.class)
156+
.hasMessageContaining("Invalid request");
157+
}
158+
151159
@Test
152160
public void labelPlaceholderWithSlash() throws Exception {
153161
this.environmentRepository.setSearchLocations("classpath:/test/{label}");
@@ -163,6 +171,22 @@ public void labelPlaceholderWithSlashAndDotDot() {
163171
.hasMessageContaining("Invalid request");
164172
}
165173

174+
@Test
175+
public void labelPlaceholderStartsWithSlash() {
176+
this.environmentRepository.setSearchLocations("classpath:/test/{label}");
177+
assertThatThrownBy(() -> this.controller.retrieve("dev", "bar", "(_)spam", "foo.txt", true, "UTF-8"))
178+
.isInstanceOf(InvalidEnvironmentRequestException.class)
179+
.hasMessageContaining("Invalid request");
180+
}
181+
182+
@Test
183+
public void pathWithSlashAndDotDot() {
184+
this.environmentRepository.setSearchLocations("classpath:/test/{label}");
185+
assertThatThrownBy(() -> this.controller.retrieve("dev", "bar", "dev", "../foo.txt", true, "UTF-8"))
186+
.isInstanceOf(InvalidEnvironmentRequestException.class)
187+
.hasMessageContaining("Invalid request");
188+
}
189+
166190
@Test
167191
public void profilePlaceholderNullLabel() throws Exception {
168192
this.environmentRepository.setSearchLocations("classpath:/test/{profile}");
@@ -268,6 +292,14 @@ public void applicationPlaceholderWithSlashForBinaryNullLabel() throws Exception
268292
assertThat(new String(resource)).isEqualToIgnoringNewLines("foo: dev_bar/spam");
269293
}
270294

295+
@Test
296+
public void applicationPlaceholderStartsWithSlashBinary() {
297+
this.environmentRepository.setSearchLocations("classpath:/test/{label}");
298+
assertThatThrownBy(() -> this.controller.binary("(_)spam", "bar", null, "foo.txt"))
299+
.isInstanceOf(InvalidEnvironmentRequestException.class)
300+
.hasMessageContaining("Invalid request");
301+
}
302+
271303
@Test
272304
public void labelPlaceholderWithSlashForBinary() throws Exception {
273305
this.environmentRepository.setSearchLocations("classpath:/test/{label}");
@@ -283,6 +315,22 @@ public void labelPlaceholderWithSlashForBinaryAndDotDot() throws Exception {
283315
.hasMessageContaining("Invalid request");
284316
}
285317

318+
@Test
319+
public void labelPlaceholderStartsWithSlashBinary() {
320+
this.environmentRepository.setSearchLocations("classpath:/test/{label}");
321+
assertThatThrownBy(() -> this.controller.binary("dev", "bar", "(_)spam", "foo.txt"))
322+
.isInstanceOf(InvalidEnvironmentRequestException.class)
323+
.hasMessageContaining("Invalid request");
324+
}
325+
326+
@Test
327+
public void pathWithSlashAndDotDotForBinary() throws Exception {
328+
this.environmentRepository.setSearchLocations("classpath:/test/{label}");
329+
assertThatThrownBy(() -> this.controller.binary("dev", "bar", "dev", "../foo.txt"))
330+
.isInstanceOf(InvalidEnvironmentRequestException.class)
331+
.hasMessageContaining("Invalid request");
332+
}
333+
286334
@Test
287335
public void profilePlaceholderForBinaryNullLabel() throws Exception {
288336
this.environmentRepository.setSearchLocations("classpath:/test/{profile}");
@@ -407,6 +455,11 @@ public void invalidProfileTests() {
407455
.isInstanceOf(InvalidEnvironmentRequestException.class);
408456
assertThatThrownBy(() -> this.controller.binary("application", "bar,%2e%2e,foo", "label", "template.json"))
409457
.isInstanceOf(InvalidEnvironmentRequestException.class);
458+
assertThatThrownBy(
459+
() -> this.controller.retrieve("application", "bar%2ffoo", "label", "template.json", true, "UTF-8"))
460+
.isInstanceOf(InvalidEnvironmentRequestException.class);
461+
assertThatThrownBy(() -> this.controller.binary("application", "bar%2ffoo", "label", "template.json"))
462+
.isInstanceOf(InvalidEnvironmentRequestException.class);
410463
}
411464

412465
}

0 commit comments

Comments
 (0)