Merge pull request github#5932 from zbazztian/spring-improvements

Add method invocations of Spring's SavedRequest as remote sources

Author: aschackmull

java/change-notes/2021-05-20-savedrequest-taintsources.md

@@ -0,0 +1,3 @@
+lgtm,codescanning
+* Invocations of methods from `org.springframework.security.web.savedrequest.SavedRequest`
+  have been added as sources of tainted data for all security queries.

java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll

@@ -87,6 +87,13 @@ private module Frameworks {
 private predicate sourceModelCsv(string row) {
   row =
     [
+      // org.springframework.security.web.savedrequest.SavedRequest
+      "org.springframework.security.web.savedrequest;SavedRequest;true;getRedirectUrl;;;ReturnValue;remote",
+      "org.springframework.security.web.savedrequest;SavedRequest;true;getCookies;;;ReturnValue;remote",
+      "org.springframework.security.web.savedrequest;SavedRequest;true;getHeaderValues;;;ReturnValue;remote",
+      "org.springframework.security.web.savedrequest;SavedRequest;true;getHeaderNames;;;ReturnValue;remote",
+      "org.springframework.security.web.savedrequest;SavedRequest;true;getParameterValues;;;ReturnValue;remote",
+      "org.springframework.security.web.savedrequest;SavedRequest;true;getParameterMap;;;ReturnValue;remote",
       // ServletRequestGetParameterMethod
       "javax.servlet;ServletRequest;false;getParameter;(String);;ReturnValue;remote",
       "javax.servlet;ServletRequest;false;getParameterValues;(String);;ReturnValue;remote",

java/ql/test/library-tests/dataflow/taintsources/SpringSavedRequest.java

@@ -0,0 +1,26 @@
+import org.springframework.security.web.savedrequest.SavedRequest;
+import org.springframework.security.web.savedrequest.SimpleSavedRequest;
+
+public class SpringSavedRequest {
+	SavedRequest sr;
+
+	public void test() {
+		sr.getRedirectUrl();
+		sr.getCookies();
+		sr.getHeaderValues("name");
+		sr.getHeaderNames();
+		sr.getParameterValues("name");
+		sr.getParameterMap();
+	}
+
+	SimpleSavedRequest ssr;
+
+	public void test2() {
+		ssr.getRedirectUrl();
+		ssr.getCookies();
+		ssr.getHeaderValues("name");
+		ssr.getHeaderNames();
+		ssr.getParameterValues("name");
+		ssr.getParameterMap();
+	}
+}

java/ql/test/library-tests/dataflow/taintsources/remote.expected

@@ -47,3 +47,15 @@
 | SpringMultiPart.java:21:3:21:26 | getFiles(...) | SpringMultiPart.java:21:3:21:26 | getFiles(...) |
 | SpringMultiPart.java:22:3:22:27 | getMultiFileMap(...) | SpringMultiPart.java:22:3:22:27 | getMultiFileMap(...) |
 | SpringMultiPart.java:23:3:23:41 | getMultipartContentType(...) | SpringMultiPart.java:23:3:23:41 | getMultipartContentType(...) |
+| SpringSavedRequest.java:8:3:8:21 | getRedirectUrl(...) | SpringSavedRequest.java:8:3:8:21 | getRedirectUrl(...) |
+| SpringSavedRequest.java:9:3:9:17 | getCookies(...) | SpringSavedRequest.java:9:3:9:17 | getCookies(...) |
+| SpringSavedRequest.java:10:3:10:28 | getHeaderValues(...) | SpringSavedRequest.java:10:3:10:28 | getHeaderValues(...) |
+| SpringSavedRequest.java:11:3:11:21 | getHeaderNames(...) | SpringSavedRequest.java:11:3:11:21 | getHeaderNames(...) |
+| SpringSavedRequest.java:12:3:12:31 | getParameterValues(...) | SpringSavedRequest.java:12:3:12:31 | getParameterValues(...) |
+| SpringSavedRequest.java:13:3:13:22 | getParameterMap(...) | SpringSavedRequest.java:13:3:13:22 | getParameterMap(...) |
+| SpringSavedRequest.java:19:3:19:22 | getRedirectUrl(...) | SpringSavedRequest.java:19:3:19:22 | getRedirectUrl(...) |
+| SpringSavedRequest.java:20:3:20:18 | getCookies(...) | SpringSavedRequest.java:20:3:20:18 | getCookies(...) |
+| SpringSavedRequest.java:21:3:21:29 | getHeaderValues(...) | SpringSavedRequest.java:21:3:21:29 | getHeaderValues(...) |
+| SpringSavedRequest.java:22:3:22:22 | getHeaderNames(...) | SpringSavedRequest.java:22:3:22:22 | getHeaderNames(...) |
+| SpringSavedRequest.java:23:3:23:32 | getParameterValues(...) | SpringSavedRequest.java:23:3:23:32 | getParameterValues(...) |
+| SpringSavedRequest.java:24:3:24:23 | getParameterMap(...) | SpringSavedRequest.java:24:3:24:23 | getParameterMap(...) |

java/ql/test/stubs/springframework-5.2.3/org/springframework/security/web/savedrequest/SavedRequest.java

@@ -0,0 +1,56 @@
+/*
+ * Copyright 2002-2016 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.security.web.savedrequest;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+
+import javax.servlet.http.Cookie;
+
+/**
+ * Encapsulates the functionality required of a cached request for both an authentication
+ * mechanism (typically form-based login) to redirect to the original URL and for a
+ * <tt>RequestCache</tt> to build a wrapped request, reproducing the original request
+ * data.
+ *
+ * @author Luke Taylor
+ * @since 3.0
+ */
+public interface SavedRequest extends java.io.Serializable {
+
+	/**
+	 * @return the URL for the saved request, allowing a redirect to be performed.
+	 */
+	String getRedirectUrl();
+
+	List<Cookie> getCookies();
+
+	String getMethod();
+
+	List<String> getHeaderValues(String name);
+
+	Collection<String> getHeaderNames();
+
+	List<Locale> getLocales();
+
+	String[] getParameterValues(String name);
+
+	Map<String, String[]> getParameterMap();
+
+}

java/ql/test/stubs/springframework-5.2.3/org/springframework/security/web/savedrequest/SimpleSavedRequest.java

@@ -0,0 +1,103 @@
+/*
+ * Copyright 2004, 2005, 2006 Acegi Technology Pty Limited
+ *
+ * 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.security.web.savedrequest;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+
+import javax.servlet.http.Cookie;
+
+/**
+ * A Bean implementation of SavedRequest
+ *
+ * @author Rob Winch
+ * @since 5.1
+ */
+public class SimpleSavedRequest implements SavedRequest {
+
+	public SimpleSavedRequest() {
+	}
+
+	public SimpleSavedRequest(String redirectUrl) {
+	}
+
+	public SimpleSavedRequest(SavedRequest request) {
+	}
+
+	@Override
+	public String getRedirectUrl() {
+		return null;
+	}
+
+	@Override
+	public List<Cookie> getCookies() {
+		return null;
+	}
+
+	@Override
+	public String getMethod() {
+		return null;
+	}
+
+	@Override
+	public List<String> getHeaderValues(String name) {
+		return null;
+	}
+
+	@Override
+	public Collection<String> getHeaderNames() {
+		return null;
+	}
+
+	@Override
+	public List<Locale> getLocales() {
+		return null;
+	}
+
+	@Override
+	public String[] getParameterValues(String name) {
+		return null;
+	}
+
+	@Override
+	public Map<String, String[]> getParameterMap() {
+		return null;
+	}
+
+	public void setRedirectUrl(String redirectUrl) {
+	}
+
+	public void setCookies(List<Cookie> cookies) {
+	}
+
+	public void setMethod(String method) {
+	}
+
+	public void setHeaders(Map<String, List<String>> headers) {
+	}
+
+	public void setLocales(List<Locale> locales) {
+	}
+
+	public void setParameters(Map<String, String[]> parameters) {
+	}
+
+}