You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by re...@apache.org on 2016/09/01 11:56:29 UTC
svn commit: r1758763 - in /jackrabbit/branches/2.4: ./
jackrabbit-webdav/src/main/java/org/apache/jackrabbit/webdav/util/CSRFUtil.java
jackrabbit-webdav/src/test/java/org/apache/jackrabbit/webdav/util/CSRFUtilTest.java
Author: reschke
Date: Thu Sep 1 11:56:29 2016
New Revision: 1758763
URL: http://svn.apache.org/viewvc?rev=1758763&view=rev
Log:
JCR-3909: CSRF bug in Jackrabbit-Webdav (ported to 2.4)
Modified:
jackrabbit/branches/2.4/ (props changed)
jackrabbit/branches/2.4/jackrabbit-webdav/src/main/java/org/apache/jackrabbit/webdav/util/CSRFUtil.java
jackrabbit/branches/2.4/jackrabbit-webdav/src/test/java/org/apache/jackrabbit/webdav/util/CSRFUtilTest.java
Propchange: jackrabbit/branches/2.4/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Thu Sep 1 11:56:29 2016
@@ -1,3 +1,3 @@
/jackrabbit/branches/JCR-2272:1173165-1176545
/jackrabbit/sandbox/JCR-2415-lucene-3.0:1060860-1064038
-/jackrabbit/trunk:1221447,1221579,1221593,1221789,1221818,1225179,1225191,1225196,1225207,1225525,1225528,1226452,1226472,1226515,1226750,1226863,1227171,1227240,1227590,1227593,1227615,1228058,1228149,1228155,1228160,1230507,1230681,1230688,1231204,1232035,1232100,1232404,1232831,1232920,1232922,1233069,1233344,1233446,1233468,1233471,1233544,1234807,1235192,1235375,1235423,1236709,1236775,1236819-1236821,1240053,1241461,1242775,1245443,1291424,1296202,1296226,1297526,1298428,1301046,1301397,1302401,1303438,1304323,1304382,1306337,1307456,1309908,1311861,1324713,1327180,1327432,1327926,1329198,1334998,1335017,1335030,1336017,1336252,1338172,1341373,1346045,1348860,1349185,1352440,1352791,1353920,1354499,1358543,1360013,1360571,1361941,1362796,1362924,1367057,1368796,1399576,1400843,1400935,1403408,1403768,1415093,1415574,1416387,1416863,1418236,1437374,1437384,1437618,1437963,1438158,1439346,1439797,1444755,1445122,1461064,1461137,1461613,1462115,1462153,1462205,1462211,1466060,146
6085,1466938,1467255,1467363,1469312,1469799,1469892,1469940,1470573,1471286,1475718,1478684,1479518,1487803,1497492,1498840,1498850,1499285,1505795,1505907,1505942,1506594,1508053,1509101,1517602,1517627,1517711,1519376,1526945,1530005,1535539,1537027,1556248,1582373,1634584,1680757,1729382,1732436
+/jackrabbit/trunk:1221447,1221579,1221593,1221789,1221818,1225179,1225191,1225196,1225207,1225525,1225528,1226452,1226472,1226515,1226750,1226863,1227171,1227240,1227590,1227593,1227615,1228058,1228149,1228155,1228160,1230507,1230681,1230688,1231204,1232035,1232100,1232404,1232831,1232920,1232922,1233069,1233344,1233446,1233468,1233471,1233544,1234807,1235192,1235375,1235423,1236709,1236775,1236819-1236821,1240053,1241461,1242775,1245443,1291424,1296202,1296226,1297526,1298428,1301046,1301397,1302401,1303438,1304323,1304382,1306337,1307456,1309908,1311861,1324713,1327180,1327432,1327926,1329198,1334998,1335017,1335030,1336017,1336252,1338172,1341373,1346045,1348860,1349185,1352440,1352791,1353920,1354499,1358543,1360013,1360571,1361941,1362796,1362924,1367057,1368796,1399576,1400843,1400935,1403408,1403768,1415093,1415574,1416387,1416863,1418236,1437374,1437384,1437618,1437963,1438158,1439346,1439797,1444755,1445122,1461064,1461137,1461613,1462115,1462153,1462205,1462211,1466060,146
6085,1466938,1467255,1467363,1469312,1469799,1469892,1469940,1470573,1471286,1475718,1478684,1479518,1487803,1497492,1498840,1498850,1499285,1505795,1505907,1505942,1506594,1508053,1509101,1517602,1517627,1517711,1519376,1526945,1530005,1535539,1537027,1556248,1582373,1634584,1680757,1709811,1729382,1732436
Modified: jackrabbit/branches/2.4/jackrabbit-webdav/src/main/java/org/apache/jackrabbit/webdav/util/CSRFUtil.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.4/jackrabbit-webdav/src/main/java/org/apache/jackrabbit/webdav/util/CSRFUtil.java?rev=1758763&r1=1758762&r2=1758763&view=diff
==============================================================================
--- jackrabbit/branches/2.4/jackrabbit-webdav/src/main/java/org/apache/jackrabbit/webdav/util/CSRFUtil.java (original)
+++ jackrabbit/branches/2.4/jackrabbit-webdav/src/main/java/org/apache/jackrabbit/webdav/util/CSRFUtil.java Thu Sep 1 11:56:29 2016
@@ -16,12 +16,14 @@
*/
package org.apache.jackrabbit.webdav.util;
+import org.apache.jackrabbit.webdav.DavMethods;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import javax.servlet.http.HttpServletRequest;
import java.net.MalformedURLException;
import java.net.URL;
+import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;
@@ -37,6 +39,19 @@ public class CSRFUtil {
public static final String DISABLED = "disabled";
/**
+ * Request content types for CSRF checking, see JCR-3909
+ */
+ public static final Set<String> CONTENT_TYPES = Collections.unmodifiableSet(new HashSet<String>(
+ Arrays.asList(
+ new String[] {
+ "application/x-www-form-urlencoded",
+ "multipart/form-data",
+ "text/plain"
+ }
+ )
+ ));
+
+ /**
* logger instance
*/
private static final Logger log = LoggerFactory.getLogger(CSRFUtil.class);
@@ -93,19 +108,21 @@ public class CSRFUtil {
}
public boolean isValidRequest(HttpServletRequest request) throws MalformedURLException {
- if (disabled) {
+ int methodCode = DavMethods.getMethodCode(request.getMethod());
+ if (disabled || DavMethods.DAV_POST != methodCode || !CONTENT_TYPES.contains(request.getContentType())) {
return true;
} else {
+
String refHeader = request.getHeader("Referer");
+ // empty referrer headers are not allowed for POST + relevant content types (see JCR-3909)
if (refHeader == null) {
- // empty referrer is always allowed
- return true;
- } else {
- String host = new URL(refHeader).getHost();
- // test referrer-host equelst server or
- // if it is contained in the set of explicitly allowed host names
- return host.equals(request.getServerName()) || allowedReferrerHosts.contains(host);
+ return false;
}
+
+ String host = new URL(refHeader).getHost();
+ // test referrer-host equals server or
+ // if it is contained in the set of explicitly allowed host names
+ return host.equals(request.getServerName()) || allowedReferrerHosts.contains(host);
}
}
}
\ No newline at end of file
Modified: jackrabbit/branches/2.4/jackrabbit-webdav/src/test/java/org/apache/jackrabbit/webdav/util/CSRFUtilTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.4/jackrabbit-webdav/src/test/java/org/apache/jackrabbit/webdav/util/CSRFUtilTest.java?rev=1758763&r1=1758762&r2=1758763&view=diff
==============================================================================
--- jackrabbit/branches/2.4/jackrabbit-webdav/src/test/java/org/apache/jackrabbit/webdav/util/CSRFUtilTest.java (original)
+++ jackrabbit/branches/2.4/jackrabbit-webdav/src/test/java/org/apache/jackrabbit/webdav/util/CSRFUtilTest.java Thu Sep 1 11:56:29 2016
@@ -29,11 +29,14 @@ import java.io.UnsupportedEncodingExcept
import java.net.MalformedURLException;
import java.security.Principal;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collection;
import java.util.Enumeration;
+import java.util.HashSet;
import java.util.List;
import java.util.Locale;
import java.util.Map;
+import java.util.Set;
/**
* <code>CSRFUtilTest</code>...
@@ -42,64 +45,77 @@ public class CSRFUtilTest extends TestCa
private static final String SERVER_NAME = "localhost";
+ private static final String GET = "GET";
+ private static final String POST = "POST";
+
private static final List<String> validURLs = new ArrayList<String>();
+ private static final List<String> invalidURLs = new ArrayList<String>();
static {
- validURLs.add(null);
validURLs.add("http://localhost:4503/jackrabbit/server");
validURLs.add("https://localhost:4503/jackrabbit/server");
validURLs.add("https://localhost/jackrabbit/server");
+
+ invalidURLs.add("http://invalidHost/test");
+ invalidURLs.add("http://host1:8080/test");
+ invalidURLs.add("http://user:pw@host2/test");
}
- private static void testValid(CSRFUtil util, Collection<String> validURLs) throws MalformedURLException {
- for (String url : validURLs) {
- assertTrue(url, util.isValidRequest(createRequest(url)));
+ private static void testValid(CSRFUtil util, Collection<String> validURLs, String method, Set<String> contentTypes) throws MalformedURLException {
+ if (null == contentTypes) {
+ for (String url : validURLs) {
+ assertTrue(url, util.isValidRequest(createRequest(url, method, null)));
+ }
+ } else {
+ for (String contentType : contentTypes) {
+ for (String url : validURLs) {
+ assertTrue(url, util.isValidRequest(createRequest(url, method, contentType)));
+ }
+ }
}
}
- private static void testInvalid(CSRFUtil util, Collection<String> invalidURLs) throws MalformedURLException {
- for (String url : invalidURLs) {
- assertFalse(url, util.isValidRequest(createRequest(url)));
+ private static void testInvalid(CSRFUtil util, Collection<String> invalidURLs, String method, Set<String> contentTypes) throws MalformedURLException {
+ if (null == contentTypes) {
+ for (String url : validURLs) {
+ assertFalse(url, util.isValidRequest(createRequest(url, method, null)));
+ }
+ } else {
+ for (String contentType : contentTypes) {
+ for (String url : invalidURLs) {
+ assertFalse(url, util.isValidRequest(createRequest(url, method, contentType)));
+ }
+ }
}
}
- private static HttpServletRequest createRequest(String url) {
- return new DummyRequest(url, SERVER_NAME);
+ private static HttpServletRequest createRequest(String url, String method, String contentType) {
+ return new DummyRequest(url, SERVER_NAME, method, contentType);
}
public void testNullConfig() throws Exception {
CSRFUtil util = new CSRFUtil(null);
-
- testValid(util, validURLs);
-
- List<String> invalidURLs = new ArrayList<String>();
- invalidURLs.add("http://invalidHost/test");
- invalidURLs.add("http://host1:8080/test");
- invalidURLs.add("http://user:pw@host2/test");
- testInvalid(util, invalidURLs);
+ testValid(util, validURLs, POST, CSRFUtil.CONTENT_TYPES);
+ testInvalid(util, invalidURLs, POST, CSRFUtil.CONTENT_TYPES);
}
public void testEmptyConfig() throws Exception {
CSRFUtil util = new CSRFUtil("");
- testValid(util, validURLs);
+ testValid(util, validURLs, POST, CSRFUtil.CONTENT_TYPES);
+ testInvalid(util, invalidURLs, POST, CSRFUtil.CONTENT_TYPES);
+ }
- List<String> invalidURLs = new ArrayList<String>();
- invalidURLs.add("http://invalidHost/test");
- invalidURLs.add("http://host1:8080/test");
- invalidURLs.add("http://user:pw@host2/test");
- testInvalid(util, invalidURLs);
+ public void testNoReferrer() throws Exception {
+ CSRFUtil util = new CSRFUtil("");
+ testValid(util, validURLs, POST, CSRFUtil.CONTENT_TYPES);
+ assertFalse("no referrer", util.isValidRequest(createRequest(null, POST, "text/plain")));
}
public void testDisabledConfig() throws Exception {
CSRFUtil util = new CSRFUtil(CSRFUtil.DISABLED);
- testValid(util, validURLs);
-
+ testValid(util, validURLs, POST, CSRFUtil.CONTENT_TYPES);
// since test is disabled any other referer host must be allowed
- List<String> otherHosts = new ArrayList<String>();
- otherHosts.add("http://validHost:80/test");
- otherHosts.add("http://host1/test");
- otherHosts.add("https://user:pw@host2/test");
- testValid(util, otherHosts);
+ testValid(util, invalidURLs, POST, CSRFUtil.CONTENT_TYPES);
}
public void testConfig() throws Exception {
@@ -121,20 +137,31 @@ public class CSRFUtilTest extends TestCa
for (String config : configs) {
CSRFUtil util = new CSRFUtil(config);
- testValid(util, validURLs);
- testValid(util, otherHosts);
- testInvalid(util, invalidURLs);
+ testValid(util, validURLs, POST, CSRFUtil.CONTENT_TYPES);
+ testValid(util, otherHosts, POST, CSRFUtil.CONTENT_TYPES);
+ testInvalid(util, invalidURLs, POST, CSRFUtil.CONTENT_TYPES);
}
}
+ public void testMethodsAndMediaType() throws Exception {
+ CSRFUtil util = new CSRFUtil("");
+ testValid(util, invalidURLs, GET, CSRFUtil.CONTENT_TYPES);
+ testValid(util, invalidURLs, POST, new HashSet<String>(Arrays.asList(new String[] {"application/json"})));
+ testInvalid(util, invalidURLs, POST, CSRFUtil.CONTENT_TYPES);
+ }
+
private static final class DummyRequest implements HttpServletRequest {
private final String referer;
private final String serverName;
+ private final String method;
+ private final String contentType;
- private DummyRequest(String referer, String serverName) {
+ private DummyRequest(String referer, String serverName, String method, String contentType) {
this.referer = referer;
this.serverName = serverName;
+ this.method = method;
+ this.contentType = contentType;
}
//---------------------------------------------< HttpServletRequest >---
@@ -171,7 +198,7 @@ public class CSRFUtilTest extends TestCa
return 0;
}
public String getMethod() {
- return null;
+ return method;
}
public String getPathInfo() {
return null;
@@ -240,7 +267,7 @@ public class CSRFUtilTest extends TestCa
return 0;
}
public String getContentType() {
- return null;
+ return contentType;
}
public ServletInputStream getInputStream() throws IOException {
return null;