Fix request verification (#3969)
* Add tests reproducing the issue. * Strip `'` from path for route search. * Add information on contributing, community support and commercial support to project site. * Add checking for "#". Add logging. * Add test. * Revert "Add test." This reverts commit 81875849 * Revert "Add checking for "#". Add logging." This reverts commit 0232945a * Verify for urls. * Verify for encoded and double-encoded Strings. * Verify insecure paths with `..\` and '..//'. Add more tests.
This commit is contained in:
committed by
GitHub
parent
df64a02013
commit
8ecb3dca51
@@ -213,6 +213,10 @@ public class SimpleRouteLocator implements RouteLocator, Ordered {
|
||||
private String adjustPath(final String path) {
|
||||
String adjustedPath = path;
|
||||
|
||||
if (path.startsWith("'")) {
|
||||
adjustedPath = path.substring(1);
|
||||
}
|
||||
|
||||
if (RequestUtils.isDispatcherServletRequest()
|
||||
&& StringUtils.hasText(this.dispatcherServletPath)) {
|
||||
if (!this.dispatcherServletPath.equals("/")
|
||||
|
||||
@@ -0,0 +1,30 @@
|
||||
/*
|
||||
* Copyright 2013-2021 the original author or authors.
|
||||
*
|
||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||
* you may not use this file except in compliance with the License.
|
||||
* You may obtain a copy of the License at
|
||||
*
|
||||
* https://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing, software
|
||||
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
* See the License for the specific language governing permissions and
|
||||
* limitations under the License.
|
||||
*/
|
||||
|
||||
package org.springframework.cloud.netflix.zuul.filters.pre;
|
||||
|
||||
/**
|
||||
* {@link Exception} thrown when path contains insecure segments.
|
||||
*
|
||||
* @author Olga Maciaszek-Sharma
|
||||
*/
|
||||
public class InsecureRequestPathException extends RuntimeException {
|
||||
|
||||
public InsecureRequestPathException(String uri) {
|
||||
super("Path cannot contain insecure segments: " + uri);
|
||||
}
|
||||
|
||||
}
|
||||
@@ -16,8 +16,10 @@
|
||||
|
||||
package org.springframework.cloud.netflix.zuul.filters.pre;
|
||||
|
||||
import java.io.UnsupportedEncodingException;
|
||||
import java.net.MalformedURLException;
|
||||
import java.net.URL;
|
||||
import java.net.URLDecoder;
|
||||
import java.util.regex.Pattern;
|
||||
|
||||
import javax.servlet.http.HttpServletRequest;
|
||||
@@ -34,6 +36,7 @@ import org.springframework.cloud.netflix.zuul.filters.ZuulProperties;
|
||||
import org.springframework.cloud.netflix.zuul.filters.support.FilterConstants;
|
||||
import org.springframework.cloud.netflix.zuul.util.RequestUtils;
|
||||
import org.springframework.http.HttpHeaders;
|
||||
import org.springframework.util.ResourceUtils;
|
||||
import org.springframework.util.StringUtils;
|
||||
import org.springframework.web.util.UrlPathHelper;
|
||||
|
||||
@@ -128,6 +131,9 @@ public class PreDecorationFilter extends ZuulFilter {
|
||||
RequestContext ctx = RequestContext.getCurrentContext();
|
||||
final String requestURI = this.urlPathHelper
|
||||
.getPathWithinApplication(ctx.getRequest());
|
||||
if (insecurePath(requestURI)) {
|
||||
throw new InsecureRequestPathException(requestURI);
|
||||
}
|
||||
Route route = this.routeLocator.getMatchingRoute(requestURI);
|
||||
if (route != null) {
|
||||
String location = route.getLocation();
|
||||
@@ -221,6 +227,50 @@ public class PreDecorationFilter extends ZuulFilter {
|
||||
return forwardURI;
|
||||
}
|
||||
|
||||
private boolean insecurePath(String path) {
|
||||
if (StringUtils.isEmpty(path)) {
|
||||
return false;
|
||||
}
|
||||
if (path.contains("%")) {
|
||||
try {
|
||||
path = URLDecoder.decode(path, "UTF-8");
|
||||
}
|
||||
catch (UnsupportedEncodingException ignored) {
|
||||
// Should never happen...
|
||||
}
|
||||
}
|
||||
if (isInsecurePath(path)) {
|
||||
return true;
|
||||
}
|
||||
return isInsecurePath(urlPathHelper.removeSemicolonContent(path));
|
||||
}
|
||||
|
||||
private boolean isInsecurePath(String path) {
|
||||
if (path.contains(":/")) {
|
||||
String relativePath = (path.charAt(0) == '/' ? path.substring(1) : path);
|
||||
if (ResourceUtils.isUrl(relativePath) || relativePath.startsWith("url:")) {
|
||||
if (log.isWarnEnabled()) {
|
||||
log.warn(
|
||||
"Path represents URL or has \"url:\" prefix: [" + path + "]");
|
||||
}
|
||||
return true;
|
||||
}
|
||||
}
|
||||
if (path.contains("../")) {
|
||||
if (log.isWarnEnabled()) {
|
||||
log.warn("Path contains \"../\"");
|
||||
}
|
||||
return true;
|
||||
}
|
||||
if (path.contains("..\\")) {
|
||||
if (log.isWarnEnabled()) {
|
||||
log.warn("Path contains \"..\\\"");
|
||||
}
|
||||
return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
private void addProxyHeaders(RequestContext ctx, Route route) {
|
||||
HttpServletRequest request = ctx.getRequest();
|
||||
String host = toHostHeader(request);
|
||||
|
||||
@@ -41,9 +41,11 @@ import org.springframework.mock.web.MockHttpServletRequest;
|
||||
import org.springframework.util.MultiValueMap;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
import static org.assertj.core.api.Assertions.assertThatThrownBy;
|
||||
import static org.mockito.MockitoAnnotations.initMocks;
|
||||
import static org.springframework.cloud.netflix.zuul.filters.support.FilterConstants.FORWARD_TO_KEY;
|
||||
import static org.springframework.cloud.netflix.zuul.filters.support.FilterConstants.PRE_TYPE;
|
||||
import static org.springframework.cloud.netflix.zuul.filters.support.FilterConstants.PROXY_KEY;
|
||||
import static org.springframework.cloud.netflix.zuul.filters.support.FilterConstants.REQUEST_URI_KEY;
|
||||
import static org.springframework.cloud.netflix.zuul.filters.support.FilterConstants.SERVICE_ID_KEY;
|
||||
|
||||
@@ -691,6 +693,90 @@ public class PreDecorationFilterTests {
|
||||
assertThat(forwardUri).isEqualTo("/mypath");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void correctRouteFromUriWithQuotes() {
|
||||
this.properties.setStripPrefix(false);
|
||||
this.request.setRequestURI("'/api/admin/index'");
|
||||
this.routeLocator.addRoute(new ZuulRoute("admin", "/admin/**", "test",
|
||||
"http://127.0.0.1:8080/admin", false, null,
|
||||
new HashSet<>(Collections.singletonList("username"))));
|
||||
this.routeLocator.addRoute(new ZuulRoute("api", "/api/**", "test",
|
||||
"http://127.0.0.1:8080/api", false, null, null));
|
||||
this.filter.run();
|
||||
RequestContext ctx = RequestContext.getCurrentContext();
|
||||
assertThat(ctx.get(PROXY_KEY)).isEqualTo("api");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void exceptionThrownForInsecurePath() {
|
||||
request.setRequestURI("'/api/..;/admin/index'");
|
||||
assertThatThrownBy(() -> filter.run())
|
||||
.isInstanceOf(InsecureRequestPathException.class);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void exceptionThrownForInsecurePathWithBackslash() {
|
||||
request.setRequestURI("'/api/..\\admin/index'");
|
||||
assertThatThrownBy(() -> filter.run())
|
||||
.isInstanceOf(InsecureRequestPathException.class);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void exceptionThrownForInsecurePathWithDoubleSlash() {
|
||||
request.setRequestURI("'/api/..//admin/index'");
|
||||
assertThatThrownBy(() -> filter.run())
|
||||
.isInstanceOf(InsecureRequestPathException.class);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void exceptionThrownForEncodedInsecurePathWithBackslash() {
|
||||
request.setRequestURI("%27%2Fapi%2F..%5Cadmin%2Findex%27");
|
||||
assertThatThrownBy(() -> filter.run())
|
||||
.isInstanceOf(InsecureRequestPathException.class);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void exceptionThrownForEncodedInsecurePathWithDoubleSlash() {
|
||||
request.setRequestURI("%27%2Fapi%2F..%2F%2Fadmin%2Findex%27");
|
||||
assertThatThrownBy(() -> filter.run())
|
||||
.isInstanceOf(InsecureRequestPathException.class);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void exceptionThrownForDoubleEncodedInsecurePathWithBackslash() {
|
||||
request.setRequestURI("%2527%252Fapi%252F..%255Cadmin%252Findex%2527");
|
||||
assertThatThrownBy(() -> filter.run())
|
||||
.isInstanceOf(InsecureRequestPathException.class);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void exceptionThrownForDoubleEncodedInsecurePathWithDoubleSlash() {
|
||||
request.setRequestURI("%27%2Fapi%2F..%2F%2Fadmin%2Findex%27");
|
||||
assertThatThrownBy(() -> filter.run())
|
||||
.isInstanceOf(InsecureRequestPathException.class);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void exceptionThrownForEncodedInsecurePath() {
|
||||
request.setRequestURI("%2527%252Fapi%252F..%252F%252Fadmin%252Findex%2527");
|
||||
assertThatThrownBy(() -> filter.run())
|
||||
.isInstanceOf(InsecureRequestPathException.class);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void exceptionThrownForDoubleEncodedInsecurePath() {
|
||||
request.setRequestURI("%2527%252Fapi%252F..%253B%252Fadmin%252Findex%2527");
|
||||
assertThatThrownBy(() -> filter.run())
|
||||
.isInstanceOf(InsecureRequestPathException.class);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void exceptionThrownForInsecurePathWithUrl() {
|
||||
request.setRequestURI("http:////admin.index'");
|
||||
assertThatThrownBy(() -> filter.run())
|
||||
.isInstanceOf(InsecureRequestPathException.class);
|
||||
}
|
||||
|
||||
private Object getHeader(List<Pair<String, String>> headers, String key) {
|
||||
String value = null;
|
||||
for (Pair<String, String> pair : headers) {
|
||||
|
||||
Reference in New Issue
Block a user