You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hawq.apache.org by nh...@apache.org on 2015/12/02 00:31:39 UTC
incubator-hawq git commit: HAWQ-189. Replace all non-alpha-numeric
characters in returned message to avoid cross-site scripting
Repository: incubator-hawq
Updated Branches:
refs/heads/master 7eeeec9d4 -> aa268c59e
HAWQ-189. Replace all non-alpha-numeric characters in returned message to avoid cross-site scripting
The recommendation to avoid XSS is to validate the input. Because the path can be of any format, depending on the custom plugins used, no generic validation is possible at the entry point. Instead we chose to make sure that the returned ok message is safe by replacing all special characters with a dot.
Project: http://git-wip-us.apache.org/repos/asf/incubator-hawq/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-hawq/commit/aa268c59
Tree: http://git-wip-us.apache.org/repos/asf/incubator-hawq/tree/aa268c59
Diff: http://git-wip-us.apache.org/repos/asf/incubator-hawq/diff/aa268c59
Branch: refs/heads/master
Commit: aa268c59e5e4980bfaf1c94e54d063e4c44c8dea
Parents: 7eeeec9
Author: Noa Horn <nh...@pivotal.io>
Authored: Tue Dec 1 15:31:05 2015 -0800
Committer: Noa Horn <nh...@pivotal.io>
Committed: Tue Dec 1 15:31:05 2015 -0800
----------------------------------------------------------------------
.../hawq/pxf/service/rest/WritableResource.java | 23 +++--
.../hawq/pxf/service/utilities/Utilities.java | 19 +++-
.../pxf/service/rest/WritableResourceTest.java | 92 ++++++++++++++++++++
.../pxf/service/utilities/UtilitiesTest.java | 19 ++++
4 files changed, 143 insertions(+), 10 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/aa268c59/pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/rest/WritableResource.java
----------------------------------------------------------------------
diff --git a/pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/rest/WritableResource.java b/pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/rest/WritableResource.java
index 1ac0ce0..02c5631 100644
--- a/pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/rest/WritableResource.java
+++ b/pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/rest/WritableResource.java
@@ -8,9 +8,9 @@ package org.apache.hawq.pxf.service.rest;
* to you 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
- *
+ *
* http://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
@@ -37,16 +37,16 @@ import javax.ws.rs.core.Response;
import org.apache.catalina.connector.ClientAbortException;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
-
import org.apache.hawq.pxf.service.Bridge;
import org.apache.hawq.pxf.service.WriteBridge;
import org.apache.hawq.pxf.service.utilities.ProtocolData;
import org.apache.hawq.pxf.service.utilities.SecuredHDFS;
+import org.apache.hawq.pxf.service.utilities.Utilities;
/*
* Running this resource manually:
*
- * run:
+ * run:
curl -i -X post "http://localhost:50070/pxf/v5w/Writable/stream?path=/data/curl/curl`date \"+%h%d_%H%M%s\"`" \
--header "X-GP-Accessor: TextFileWAccessor" \
--header "X-GP-Resolver: TextWResolver" \
@@ -75,13 +75,13 @@ import org.apache.hawq.pxf.service.utilities.SecuredHDFS;
wrote 15 bytes to curlAug11_17271376231245
file content:
- bin/hdfs dfs -cat /data/curl/*45
+ bin/hdfs dfs -cat /data/curl/*45
data111&data222
*/
-/*
+/**
* This class handles the subpath /<version>/Writable/ of this
* REST component
*/
@@ -92,7 +92,7 @@ public class WritableResource extends RestResource{
public WritableResource() {
}
- /*
+ /**
* This function is called when http://nn:port/pxf/vx/Writable/stream?path=...
* is used.
*
@@ -100,6 +100,9 @@ public class WritableResource extends RestResource{
* @param headers Holds HTTP headers from request
* @param path Holds URI path option used in this request
* @param inputStream stream of bytes to write from Hawq
+ * @return ok response if the operation finished successfully
+ * @throws Exception in case of wrong request parameters, failure to
+ * initialize bridge or to write data
*/
@POST
@Path("stream")
@@ -117,7 +120,7 @@ public class WritableResource extends RestResource{
ProtocolData protData = new ProtocolData(params);
protData.setDataSource(path);
-
+
SecuredHDFS.verifyToken(protData, servletContext);
Bridge bridge = new WriteBridge(protData);
@@ -163,7 +166,9 @@ public class WritableResource extends RestResource{
} finally {
inputStream.close();
}
- returnMsg = "wrote " + totalWritten + " bulks to " + path;
+
+ String censuredPath = Utilities.maskNonPrintables(path);
+ returnMsg = "wrote " + totalWritten + " bulks to " + censuredPath;
LOG.debug(returnMsg);
return Response.ok(returnMsg).build();
http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/aa268c59/pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/utilities/Utilities.java
----------------------------------------------------------------------
diff --git a/pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/utilities/Utilities.java b/pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/utilities/Utilities.java
index e2b4b11..cfd19d1 100644
--- a/pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/utilities/Utilities.java
+++ b/pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/utilities/Utilities.java
@@ -23,9 +23,9 @@ package org.apache.hawq.pxf.service.utilities;
import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
+import org.apache.commons.lang.StringUtils;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
-
import org.apache.hawq.pxf.api.utilities.InputData;
/**
@@ -136,4 +136,21 @@ public class Utilities {
sb.append(String.format("\\\\%03o", b & 0xff));
}
}
+
+ /**
+ * Replaces any non-alpha-numeric character with a '.'.
+ * This measure is used to prevent cross-site scripting (XSS)
+ * when an input string might include code or script. By removing
+ * all special character and returning a censured string to the user
+ * this thread is avoided.
+ *
+ * @param input string to be masked
+ * @return masked string
+ */
+ public static String maskNonPrintables(String input) {
+ if (StringUtils.isEmpty(input)) {
+ return input;
+ }
+ return input.replaceAll("[^a-zA-Z0-9_-]", ".");
+ }
}
http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/aa268c59/pxf/pxf-service/src/test/java/org/apache/hawq/pxf/service/rest/WritableResourceTest.java
----------------------------------------------------------------------
diff --git a/pxf/pxf-service/src/test/java/org/apache/hawq/pxf/service/rest/WritableResourceTest.java b/pxf/pxf-service/src/test/java/org/apache/hawq/pxf/service/rest/WritableResourceTest.java
new file mode 100644
index 0000000..0abf73b
--- /dev/null
+++ b/pxf/pxf-service/src/test/java/org/apache/hawq/pxf/service/rest/WritableResourceTest.java
@@ -0,0 +1,92 @@
+package org.apache.hawq.pxf.service.rest;
+
+import static org.junit.Assert.assertEquals;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.io.InputStream;
+import java.util.Map;
+import java.util.TreeMap;
+
+import javax.servlet.ServletContext;
+import javax.ws.rs.core.HttpHeaders;
+import javax.ws.rs.core.MultivaluedMap;
+import javax.ws.rs.core.Response;
+
+import org.apache.hawq.pxf.service.WriteBridge;
+import org.apache.hawq.pxf.service.utilities.ProtocolData;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mockito;
+import org.powermock.api.mockito.PowerMockito;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
+
+import com.sun.jersey.core.util.MultivaluedMapImpl;
+
+@RunWith(PowerMockRunner.class)
+@PrepareForTest({ WritableResource.class })
+public class WritableResourceTest {
+
+ WritableResource writableResource;
+ ServletContext servletContext;
+ HttpHeaders headers;
+ InputStream inputStream;
+ MultivaluedMap<String, String> headersMap;
+ Map<String, String> params;
+ ProtocolData protData;
+ WriteBridge bridge;
+
+ @Test
+ public void streamPathWithSpecialChars() throws Exception {
+ // test path with special characters
+ String path = "I'mso<bad>!";
+
+ Response result = writableResource.stream(servletContext, headers,
+ path, inputStream);
+
+ assertEquals(Response.Status.OK,
+ Response.Status.fromStatusCode(result.getStatus()));
+ assertEquals("wrote 0 bulks to I.mso.bad..",
+ result.getEntity().toString());
+ }
+
+ @Test
+ public void streamPathWithRegularChars() throws Exception {
+ // test path with regular characters
+ String path = "whatCAN1tellYOU";
+
+ Response result = writableResource.stream(servletContext, headers,
+ path, inputStream);
+
+ assertEquals(Response.Status.OK,
+ Response.Status.fromStatusCode(result.getStatus()));
+ assertEquals("wrote 0 bulks to " + path, result.getEntity().toString());
+ }
+
+ @Before
+ public void before() throws Exception {
+ writableResource = mock(WritableResource.class,
+ Mockito.CALLS_REAL_METHODS);
+
+ // mock input
+ servletContext = mock(ServletContext.class);
+ headers = mock(HttpHeaders.class);
+ inputStream = mock(InputStream.class);
+ // mock internal functions to do nothing
+ headersMap = new MultivaluedMapImpl();
+ when(headers.getRequestHeaders()).thenReturn(headersMap);
+ params = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
+ when(writableResource.convertToCaseInsensitiveMap(headersMap)).thenReturn(
+ params);
+ protData = mock(ProtocolData.class);
+ PowerMockito.whenNew(ProtocolData.class).withArguments(params).thenReturn(
+ protData);
+ bridge = mock(WriteBridge.class);
+ PowerMockito.whenNew(WriteBridge.class).withArguments(protData).thenReturn(
+ bridge);
+ when(protData.isThreadSafe()).thenReturn(true);
+ when(bridge.isThreadSafe()).thenReturn(true);
+ }
+}
http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/aa268c59/pxf/pxf-service/src/test/java/org/apache/hawq/pxf/service/utilities/UtilitiesTest.java
----------------------------------------------------------------------
diff --git a/pxf/pxf-service/src/test/java/org/apache/hawq/pxf/service/utilities/UtilitiesTest.java b/pxf/pxf-service/src/test/java/org/apache/hawq/pxf/service/utilities/UtilitiesTest.java
index 8db6b97..788570b 100644
--- a/pxf/pxf-service/src/test/java/org/apache/hawq/pxf/service/utilities/UtilitiesTest.java
+++ b/pxf/pxf-service/src/test/java/org/apache/hawq/pxf/service/utilities/UtilitiesTest.java
@@ -90,4 +90,23 @@ public class UtilitiesTest {
+ "Plugins provided by PXF must start with \"org.apache.hawq.pxf\"");
}
}
+
+ @Test
+ public void maskNonPrintable() throws Exception {
+ String input = "";
+ String result = Utilities.maskNonPrintables(input);
+ assertEquals("", result);
+
+ input = null;
+ result = Utilities.maskNonPrintables(input);
+ assertEquals(null, result);
+
+ input = "Lucy in the sky";
+ result = Utilities.maskNonPrintables(input);
+ assertEquals("Lucy.in.the.sky", result);
+
+ input = "with <$$$@#$!000diamonds!!?!$#&%/>";
+ result = Utilities.maskNonPrintables(input);
+ assertEquals("with.........000diamonds..........", result);
+ }
}