Prechádzať zdrojové kódy

1. add customization support for double forwardslash in StrickHttpFirewall
2. add getEncodedUrlBlacklist() and getDecodedUrlBlacklist() method in StrickHttpFirewall

Fixes gh-6292

guo fei 6 rokov pred
rodič
commit
c0e66a9ba1

+ 37 - 4
web/src/main/java/org/springframework/security/web/firewall/StrictHttpFirewall.java

@@ -88,6 +88,8 @@ public class StrictHttpFirewall implements HttpFirewall {
 
 	private static final List<String> FORBIDDEN_FORWARDSLASH = Collections.unmodifiableList(Arrays.asList("%2f", "%2F"));
 
+	private static final List<String> FORBIDDEN_DOUBLE_FORWARDSLASH = Collections.unmodifiableList(Arrays.asList("//", "%2f%2f", "%2f%2F", "%2F%2f", "%2F%2F"));
+
 	private static final List<String> FORBIDDEN_BACKSLASH = Collections.unmodifiableList(Arrays.asList("\\", "%5c", "%5C"));
 
 	private Set<String> encodedUrlBlacklist = new HashSet<String>();
@@ -99,6 +101,7 @@ public class StrictHttpFirewall implements HttpFirewall {
 	public StrictHttpFirewall() {
 		urlBlacklistsAddAll(FORBIDDEN_SEMICOLON);
 		urlBlacklistsAddAll(FORBIDDEN_FORWARDSLASH);
+		urlBlacklistsAddAll(FORBIDDEN_DOUBLE_FORWARDSLASH);
 		urlBlacklistsAddAll(FORBIDDEN_BACKSLASH);
 
 		this.encodedUrlBlacklist.add(ENCODED_PERCENT);
@@ -203,6 +206,23 @@ public class StrictHttpFirewall implements HttpFirewall {
 		}
 	}
 
+	/**
+	 * <p>
+	 * Determines if double slash "//" that is URL encoded "%2F%2F" should be allowed in the path or
+	 * not. The default is to not allow.
+	 * </p>
+	 *
+	 * @param allowUrlEncodedDoubleSlash should a slash "//" that is URL encoded "%2F%2F" be allowed
+	 *        in the path or not. Default is false.
+	 */
+	public void setAllowUrlEncodedDoubleSlash(boolean allowUrlEncodedDoubleSlash) {
+		if (allowUrlEncodedDoubleSlash) {
+			urlBlacklistsRemoveAll(FORBIDDEN_DOUBLE_FORWARDSLASH);
+		} else {
+			urlBlacklistsAddAll(FORBIDDEN_DOUBLE_FORWARDSLASH);
+		}
+	}
+
 	/**
 	 * <p>
 	 * Determines if a period "." that is URL encoded "%2E" should be allowed in the path
@@ -412,10 +432,6 @@ public class StrictHttpFirewall implements HttpFirewall {
 			return true;
 		}
 
-		if (path.indexOf("//") > -1) {
-			return false;
-		}
-
 		for (int j = path.length(); j > 0;) {
 			int i = path.lastIndexOf('/', j - 1);
 			int gap = j - i;
@@ -433,4 +449,21 @@ public class StrictHttpFirewall implements HttpFirewall {
 		return true;
 	}
 
+	/**
+	 * Provides the existing encoded url blacklist which can add/remove entries from
+	 *
+	 * @return the existing encoded url blacklist, never null
+	 */
+	public Set<String> getEncodedUrlBlacklist() {
+		return encodedUrlBlacklist;
+	}
+
+	/**
+	 * Provides the existing decoded url blacklist which can add/remove entries from
+	 *
+	 * @return the existing decoded url blacklist, never null
+	 */
+	public Set<String> getDecodedUrlBlacklist() {
+		return decodedUrlBlacklist;
+	}
 }

+ 96 - 0
web/src/test/java/org/springframework/security/web/firewall/StrictHttpFirewallTests.java

@@ -428,4 +428,100 @@ public class StrictHttpFirewallTests {
 
 		this.firewall.getFirewalledRequest(request);
 	}
+
+	@Test
+	public void getFirewalledRequestWhenAllowUrlLowerCaseEncodedDoubleSlashThenNoException() throws Exception {
+		this.firewall.setAllowUrlEncodedSlash(true);
+		this.firewall.setAllowUrlEncodedDoubleSlash(true);
+		MockHttpServletRequest request = new MockHttpServletRequest("GET", "");
+		request.setRequestURI("/context-root/a/b%2f%2fc");
+		request.setContextPath("/context-root");
+		request.setServletPath("");
+		request.setPathInfo("/a/b//c");
+		assertThatCode(() -> this.firewall.getFirewalledRequest(request)).doesNotThrowAnyException();
+	}
+
+	@Test
+	public void getFirewalledRequestWhenAllowUrlUpperCaseEncodedDoubleSlashThenNoException() throws Exception {
+		this.firewall.setAllowUrlEncodedSlash(true);
+		this.firewall.setAllowUrlEncodedDoubleSlash(true);
+		MockHttpServletRequest request = new MockHttpServletRequest("GET", "");
+		request.setRequestURI("/context-root/a/b%2F%2Fc");
+		request.setContextPath("/context-root");
+		request.setServletPath("");
+		request.setPathInfo("/a/b//c");
+		assertThatCode(() -> this.firewall.getFirewalledRequest(request)).doesNotThrowAnyException();
+	}
+
+	@Test
+	public void getFirewalledRequestWhenAllowUrlLowerCaseAndUpperCaseEncodedDoubleSlashThenNoException()
+					throws Exception {
+		this.firewall.setAllowUrlEncodedSlash(true);
+		this.firewall.setAllowUrlEncodedDoubleSlash(true);
+		MockHttpServletRequest request = new MockHttpServletRequest("GET", "");
+		request.setRequestURI("/context-root/a/b%2f%2Fc");
+		request.setContextPath("/context-root");
+		request.setServletPath("");
+		request.setPathInfo("/a/b//c");
+		assertThatCode(() -> this.firewall.getFirewalledRequest(request)).doesNotThrowAnyException();
+	}
+
+	@Test
+	public void getFirewalledRequestWhenAllowUrlUpperCaseAndLowerCaseEncodedDoubleSlashThenNoException()
+					throws Exception {
+		this.firewall.setAllowUrlEncodedSlash(true);
+		this.firewall.setAllowUrlEncodedDoubleSlash(true);
+		MockHttpServletRequest request = new MockHttpServletRequest("GET", "");
+		request.setRequestURI("/context-root/a/b%2F%2fc");
+		request.setContextPath("/context-root");
+		request.setServletPath("");
+		request.setPathInfo("/a/b//c");
+		assertThatCode(() -> this.firewall.getFirewalledRequest(request)).doesNotThrowAnyException();
+	}
+
+	@Test
+	public void getFirewalledRequestWhenRemoveFromUpperCaseEncodedUrlBlacklistThenNoException() throws Exception {
+		this.firewall.setAllowUrlEncodedSlash(true);
+		MockHttpServletRequest request = new MockHttpServletRequest("GET", "");
+		request.setRequestURI("/context-root/a/b%2F%2Fc");
+		this.firewall.getEncodedUrlBlacklist().removeAll(Arrays.asList("%2F%2F"));
+		assertThatCode(() -> this.firewall.getFirewalledRequest(request)).doesNotThrowAnyException();
+	}
+
+	@Test
+	public void getFirewalledRequestWhenRemoveFromLowerCaseEncodedUrlBlacklistThenNoException() throws Exception {
+		this.firewall.setAllowUrlEncodedSlash(true);
+		MockHttpServletRequest request = new MockHttpServletRequest("GET", "");
+		request.setRequestURI("/context-root/a/b%2f%2fc");
+		this.firewall.getEncodedUrlBlacklist().removeAll(Arrays.asList("%2f%2f"));
+		assertThatCode(() -> this.firewall.getFirewalledRequest(request)).doesNotThrowAnyException();
+	}
+
+	@Test
+	public void getFirewalledRequestWhenRemoveFromLowerCaseAndUpperCaseEncodedUrlBlacklistThenNoException()
+					throws Exception {
+		this.firewall.setAllowUrlEncodedSlash(true);
+		MockHttpServletRequest request = new MockHttpServletRequest("GET", "");
+		request.setRequestURI("/context-root/a/b%2f%2Fc");
+		this.firewall.getEncodedUrlBlacklist().removeAll(Arrays.asList("%2f%2F"));
+		assertThatCode(() -> this.firewall.getFirewalledRequest(request)).doesNotThrowAnyException();
+	}
+
+	@Test
+	public void getFirewalledRequestWhenRemoveFromUpperCaseAndLowerCaseEncodedUrlBlacklistThenNoException()
+					throws Exception {
+		this.firewall.setAllowUrlEncodedSlash(true);
+		MockHttpServletRequest request = new MockHttpServletRequest("GET", "");
+		request.setRequestURI("/context-root/a/b%2F%2fc");
+		this.firewall.getEncodedUrlBlacklist().removeAll(Arrays.asList("%2F%2f"));
+		assertThatCode(() -> this.firewall.getFirewalledRequest(request)).doesNotThrowAnyException();
+	}
+
+	@Test
+	public void getFirewalledRequestWhenRemoveFromDecodedUrlBlacklistThenNoException() throws Exception {
+		MockHttpServletRequest request = new MockHttpServletRequest("GET", "");
+		request.setPathInfo("/a/b//c");
+		this.firewall.getDecodedUrlBlacklist().removeAll(Arrays.asList("//"));
+		assertThatCode(() -> this.firewall.getFirewalledRequest(request)).doesNotThrowAnyException();
+	}
 }