You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@drill.apache.org by ar...@apache.org on 2017/09/12 10:47:40 UTC
drill git commit: DRILL-5766: Fix XSS vulnerabilities in Drill
Repository: drill
Updated Branches:
refs/heads/master 7a900b71f -> c7c8ffd6f
DRILL-5766: Fix XSS vulnerabilities in Drill
1. Bumped up freemarker version to 2.3.26-incubating.
2. Indicated default output format in Freemarker configuration (HTML).
3. Fixed Web UI bugs listed in DRILL-5346, DRILL-5341, DRILL-5339, DRILL-5338.
closes #935
Project: http://git-wip-us.apache.org/repos/asf/drill/repo
Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/c7c8ffd6
Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/c7c8ffd6
Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/c7c8ffd6
Branch: refs/heads/master
Commit: c7c8ffd6f5e1b8f4677f9e95c317482d929a1bb5
Parents: 7a900b7
Author: Arina Ielchiieva <ar...@gmail.com>
Authored: Thu Sep 7 15:30:39 2017 +0300
Committer: Arina Ielchiieva <ar...@gmail.com>
Committed: Tue Sep 12 12:53:53 2017 +0300
----------------------------------------------------------------------
.../drill/exec/server/rest/DrillRestServer.java | 43 ++++-
.../drill/exec/server/rest/WebServer.java | 2 +-
.../src/main/resources/rest/logs/log.ftl | 2 +-
.../src/main/resources/rest/options.ftl | 3 -
.../src/main/resources/rest/profile/list.ftl | 135 ++++---------
.../src/main/resources/rest/profile/profile.ftl | 187 +++++++++++++------
.../src/main/resources/rest/query/result.ftl | 4 +-
pom.xml | 2 +-
8 files changed, 207 insertions(+), 171 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/drill/blob/c7c8ffd6/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRestServer.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRestServer.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRestServer.java
index bd01fea..238f5ca 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRestServer.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRestServer.java
@@ -22,6 +22,13 @@ import com.fasterxml.jackson.jaxrs.base.JsonMappingExceptionMapper;
import com.fasterxml.jackson.jaxrs.base.JsonParseExceptionMapper;
import com.fasterxml.jackson.jaxrs.json.JacksonJaxbJsonProvider;
import com.google.common.base.Strings;
+import freemarker.cache.ClassTemplateLoader;
+import freemarker.cache.FileTemplateLoader;
+import freemarker.cache.MultiTemplateLoader;
+import freemarker.cache.TemplateLoader;
+import freemarker.cache.WebappTemplateLoader;
+import freemarker.core.HTMLOutputFormat;
+import freemarker.template.Configuration;
import org.apache.drill.common.config.DrillConfig;
import org.apache.drill.exec.ExecConstants;
import org.apache.drill.exec.memory.BufferAllocator;
@@ -48,17 +55,22 @@ import org.glassfish.jersey.server.filter.RolesAllowedDynamicFeature;
import org.glassfish.jersey.server.mvc.freemarker.FreemarkerMvcFeature;
import javax.inject.Inject;
+import javax.servlet.ServletContext;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpSession;
+import java.io.File;
+import java.io.IOException;
import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.net.SocketAddress;
import java.security.Principal;
+import java.util.ArrayList;
+import java.util.List;
public class DrillRestServer extends ResourceConfig {
static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(DrillRestServer.class);
- public DrillRestServer(final WorkManager workManager) {
+ public DrillRestServer(final WorkManager workManager, final ServletContext servletContext) {
register(DrillRoot.class);
register(StatusResources.class);
register(StorageResources.class);
@@ -67,10 +79,14 @@ public class DrillRestServer extends ResourceConfig {
register(MetricsResources.class);
register(ThreadsResources.class);
register(LogsResources.class);
+
+ property(FreemarkerMvcFeature.TEMPLATE_OBJECT_FACTORY, getFreemarkerConfiguration(servletContext));
register(FreemarkerMvcFeature.class);
+
register(MultiPartFeature.class);
property(ServerProperties.METAINF_SERVICES_LOOKUP_DISABLE, true);
+
final boolean isAuthEnabled =
workManager.getContext().getConfig().getBoolean(ExecConstants.USER_AUTHENTICATION_ENABLED);
@@ -112,6 +128,31 @@ public class DrillRestServer extends ResourceConfig {
});
}
+ /**
+ * Creates freemarker configuration settings,
+ * default output format to trigger auto-escaping policy
+ * and template loaders.
+ *
+ * @param servletContext servlet context
+ * @return freemarker configuration settings
+ */
+ private Configuration getFreemarkerConfiguration(ServletContext servletContext) {
+ Configuration configuration = new Configuration(Configuration.VERSION_2_3_26);
+ configuration.setOutputFormat(HTMLOutputFormat.INSTANCE);
+
+ List<TemplateLoader> loaders = new ArrayList<>();
+ loaders.add(new WebappTemplateLoader(servletContext));
+ loaders.add(new ClassTemplateLoader(DrillRestServer.class, "/"));
+ try {
+ loaders.add(new FileTemplateLoader(new File("/")));
+ } catch (IOException e) {
+ logger.error("Could not set up file template loader.", e);
+ }
+ configuration.setTemplateLoader(new MultiTemplateLoader(loaders.toArray(new TemplateLoader[loaders.size()])));
+ return configuration;
+ }
+
+
public static class AuthWebUserConnectionProvider implements Factory<WebUserConnection> {
@Inject
http://git-wip-us.apache.org/repos/asf/drill/blob/c7c8ffd6/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java
index 3fc95cd..8818349 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java
@@ -171,7 +171,7 @@ public class WebServer implements AutoCloseable {
servletContextHandler.setErrorHandler(errorHandler);
servletContextHandler.setContextPath("/");
- final ServletHolder servletHolder = new ServletHolder(new ServletContainer(new DrillRestServer(workManager)));
+ final ServletHolder servletHolder = new ServletHolder(new ServletContainer(new DrillRestServer(workManager, servletContextHandler.getServletContext())));
servletHolder.setInitOrder(1);
servletContextHandler.addServlet(servletHolder, "/*");
http://git-wip-us.apache.org/repos/asf/drill/blob/c7c8ffd6/exec/java-exec/src/main/resources/rest/logs/log.ftl
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/resources/rest/logs/log.ftl b/exec/java-exec/src/main/resources/rest/logs/log.ftl
index f5386bd..4f37beb 100644
--- a/exec/java-exec/src/main/resources/rest/logs/log.ftl
+++ b/exec/java-exec/src/main/resources/rest/logs/log.ftl
@@ -24,7 +24,7 @@
<#if (model.getLines()?size > 0)>
<pre>
<#list model.getLines() as line>
-${line?html}
+${line}
</#list>
</pre>
<#else>
http://git-wip-us.apache.org/repos/asf/drill/blob/c7c8ffd6/exec/java-exec/src/main/resources/rest/options.ftl
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/resources/rest/options.ftl b/exec/java-exec/src/main/resources/rest/options.ftl
index 7ba1250..e280e81 100644
--- a/exec/java-exec/src/main/resources/rest/options.ftl
+++ b/exec/java-exec/src/main/resources/rest/options.ftl
@@ -18,9 +18,6 @@
<div class="page-header">
</div>
<h4>System options</h4>
- <div align="right">
- <a href="http://drill.apache.org/docs/planning-and-execution-options/">Documentation</a>
- </div>
<div class="table-responsive">
<table class="table table-hover">
<tbody>
http://git-wip-us.apache.org/repos/asf/drill/blob/c7c8ffd6/exec/java-exec/src/main/resources/rest/profile/list.ftl
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/resources/rest/profile/list.ftl b/exec/java-exec/src/main/resources/rest/profile/list.ftl
index 1fcffb6..aba0033 100644
--- a/exec/java-exec/src/main/resources/rest/profile/list.ftl
+++ b/exec/java-exec/src/main/resources/rest/profile/list.ftl
@@ -28,55 +28,7 @@
</#if>
<#if (model.getRunningQueries()?size > 0) >
<h3>Running Queries</h3>
- <div class="table-responsive">
- <table class="table table-hover">
- <thead>
- <td>Time</td>
- <!-- <td>Query Id</td> -->
- <td>User</td>
- <td>Query</td>
- <td>State</td>
- <td>Elapsed</td>
- <td>Foreman</td>
- </thead>
- <tbody>
- <#list model.getRunningQueries() as query>
- <tr>
- <td>${query.getTime()}</td>
- <!--
- <td>
- <a href="/profiles/${query.getQueryId()}">
- <div style="height:100%;width:100%">
- ${query.getQueryId()}
- </div>
- </a>
- </td>
- -->
- <td>
- <a href="/profiles/${query.getQueryId()}">
- <div style="height:100%;width:100%;white-space:pre-line">${query.getUser()}</div>
- </a>
- </td>
- <td>
- <a href="/profiles/${query.getQueryId()}">
- <div style="height:100%;width:100%;white-space:pre-line">${query.getQuery()}</div>
- </a>
- </td>
- <td>
- <div style="height:100%;width:100%">${query.getState()}</div>
- <td>
- <div style="height:100%;width:100%">${query.getDuration()}</div>
- <td>
- <div style="height:100%;width:100%">
- ${query.getForeman()}
- </div>
- </td>
-
- </tr>
- </#list>
- </tbody>
- </table>
- </div>
+ <@list_queries queries=model.getRunningQueries()/>
<div class="page-header">
</div>
<#else>
@@ -86,57 +38,40 @@
</div>
</#if>
<h3>Completed Queries</h3>
- <div class="table-responsive">
- <table class="table table-hover">
- <thead>
- <td>Time</td>
- <td>User</td>
- <!-- <td>Query Id</td> -->
- <td>Query</td>
- <td>State</td>
- <td>Duration</td>
- <td>Foreman</td>
- </thead>
- <tbody>
- <#list model.getFinishedQueries() as query>
- <tr>
- <td>${query.getTime()}</td>
- <!--
- <td>
- <a href="/profiles/${query.getQueryId()}">
- <div style="height:100%;width:100%">
- ${query.getQueryId()}
- </div>
- </a>
- </td>
- -->
- <td>
- <a href="/profiles/${query.getQueryId()}">
- <div style="height:100%;width:100%;white-space:pre-line">${query.getUser()}</div>
- </a>
- </td>
-
- <td>
- <a href="/profiles/${query.getQueryId()}">
- <div style="height:100%;width:100%;white-space:pre-line">${query.getQuery()}</div>
- </a>
- </td>
- <td>
- <div style="height:100%;width:100%">${query.getState()}</div>
- </td>
- <td>
- <div style="height:100%;width:100%">${query.getDuration()}</div>
- </td>
- <td>
- <div style="height:100%;width:100%">
- ${query.getForeman()}
- </div>
- </td>
- </tr>
- </#list>
- </tbody>
- </table>
- </div>
+ <@list_queries queries=model.getFinishedQueries()/>
+</#macro>
+
+<#macro list_queries queries>
+ <div class="table-responsive">
+ <table class="table table-hover">
+ <thead>
+ <tr>
+ <th>Time</th>
+ <th>User</th>
+ <th>Query</th>
+ <th>State</th>
+ <th>Duration</th>
+ <th>Foreman</th>
+ </tr>
+ </thead>
+ <tbody>
+ <#list queries as query>
+ <tr>
+ <td>${query.getTime()}</td>
+ <td>${query.getUser()}</td>
+ <td>
+ <a href="/profiles/${query.getQueryId()}">
+ <div style="height:100%;width:100%;white-space:pre-line">${query.getQuery()}</div>
+ </a>
+ </td>
+ <td>${query.getState()}</td>
+ <td>${query.getDuration()}</td>
+ <td>${query.getForeman()}</td>
+ </tr>
+ </#list>
+ </tbody>
+ </table>
+ </div>
</#macro>
<@page_html/>
\ No newline at end of file
http://git-wip-us.apache.org/repos/asf/drill/blob/c7c8ffd6/exec/java-exec/src/main/resources/rest/profile/profile.ftl
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/resources/rest/profile/profile.ftl b/exec/java-exec/src/main/resources/rest/profile/profile.ftl
index 95d7d56..5bb7be8 100644
--- a/exec/java-exec/src/main/resources/rest/profile/profile.ftl
+++ b/exec/java-exec/src/main/resources/rest/profile/profile.ftl
@@ -17,7 +17,7 @@
<script>
var globalconfig = {
"queryid" : "${model.getQueryId()}",
- "operators" : ${model.getOperatorsJSON()}
+ "operators" : ${model.getOperatorsJSON()?no_esc}
};
</script>
</#macro>
@@ -32,7 +32,9 @@
<li><a href="#query-physical" role="tab" data-toggle="tab">Physical Plan</a></li>
<li><a href="#query-visual" role="tab" data-toggle="tab">Visualized Plan</a></li>
<li><a href="#query-edit" role="tab" data-toggle="tab">Edit Query</a></li>
- <#if model.hasError() ><li><a href="#query-error" role="tab" data-toggle="tab">Error</a></li></#if>
+ <#if model.hasError()>
+ <li><a href="#query-error" role="tab" data-toggle="tab">Error</a></li>
+ </#if>
</ul>
<div id="query-content" class="tab-content">
<div id="query-query" class="tab-pane">
@@ -45,71 +47,133 @@
<svg id="query-visual-canvas" class="center-block"></svg>
</div>
<div id="query-edit" class="tab-pane">
- <form role="form" action="/query" method="POST">
- <div class="form-group">
- <textarea class="form-control" id="query" name="query" style="font-family: Courier;">${model.getProfile().query}</textarea>
- </div>
- <div class="form-group">
- <div class="radio-inline">
- <label>
- <input type="radio" name="queryType" id="sql" value="SQL" checked>
- SQL
- </label>
- </div>
- <div class="radio-inline">
- <label>
- <input type="radio" name="queryType" id="physical" value="PHYSICAL">
- PHYSICAL
- </label>
- </div>
- <div class="radio-inline">
- <label>
- <input type="radio" name="queryType" id="logical" value="LOGICAL">
- LOGICAL
- </label>
+ <p>
+ <form role="form" action="/query" method="POST">
+ <div class="form-group">
+ <textarea class="form-control" id="query" name="query" style="font-family: Courier;">${model.getProfile().query}</textarea>
</div>
- </div>
- <button type="submit" class="btn btn-default">Re-run query</button>
- </form>
+ <div class="form-group">
+ <div class="radio-inline">
+ <label>
+ <input type="radio" name="queryType" id="sql" value="SQL" checked>
+ SQL
+ </label>
+ </div>
+ <div class="radio-inline">
+ <label>
+ <input type="radio" name="queryType" id="physical" value="PHYSICAL">
+ PHYSICAL
+ </label>
+ </div>
+ <div class="radio-inline">
+ <label>
+ <input type="radio" name="queryType" id="logical" value="LOGICAL">
+ LOGICAL
+ </label>
+ </div>
+ </div>
+ <button type="submit" class="btn btn-default">Re-run query</button>
+ </form>
+ </p>
+ <p>
<form action="/profiles/cancel/${model.queryId}" method="GET">
- <button type="link" class="btn btn-default">Cancel query</button>
+ <div class="form-group">
+ <button type="submit" class="btn btn-default">Cancel query</button>
+ </div>
</form>
+ </p>
</div>
- <#if model.hasError() >
- <div id="query-error" class="tab-pane">
- <p>
- <pre>
- ${model.getProfile().error?trim}
- </pre>
- </p>
- <p>Failure node: ${model.getProfile().errorNode}</p>
- <p>Error ID: ${model.getProfile().errorId}</p>
+ <#if model.hasError()>
+ <div id="query-error" class="tab-pane fade">
+ <p><pre>${model.getProfile().error?trim}</pre></p>
+ <p>Failure node: ${model.getProfile().errorNode}</p>
+ <p>Error ID: ${model.getProfile().errorId}</p>
- <h3 class="panel-title">
- <a data-toggle="collapse" href="#error-verbose">
- Verbose Error Message...
- </a>
- </h3>
- <div id="error-verbose" class="panel-collapse collapse">
- <div class="panel-body">
- <p></p><p></p>
- <pre>
- ${model.getProfile().verboseError?trim}
- </pre>
+ <div class="page-header"></div>
+ <h3>Verbose Error Message</h3>
+ <div class="panel panel-default">
+ <div class="panel-heading">
+ <h4 class="panel-title">
+ <a data-toggle="collapse" href="#error-message-overview">
+ Overview
+ </a>
+ </h4>
+ </div>
+ <div id="error-message-overview" class="panel-collapse collapse">
+ <div class="panel-body">
+ <pre>${model.getProfile().verboseError?trim}</pre>
+ </div>
+ </div>
</div>
- </div>
+ </div>
</#if>
+
</div>
<div class="page-header"></div>
<h3>Query Profile</h3>
- <p>STATE: ${model.getProfile().getState().name()}</p>
- <p>FOREMAN: ${model.getProfile().getForeman().getAddress()}</p>
- <p>TOTAL FRAGMENTS: ${model.getProfile().getTotalFragments()}</p>
- <p>DURATION: ${model.getProfileDuration()}</p>
- <p style="text-indent:5em;">PLANNING: ${model.getPlanningDuration()}</p>
- <p style="text-indent:5em;">QUEUED: ${model.getQueuedDuration()}</p>
- <p style="text-indent:5em;">EXECUTION: ${model.getExecutionDuration()}</p>
+ <div class="panel-group" id="query-profile-accordion">
+ <div class="panel panel-default">
+ <div class="panel-heading">
+ <h4 class="panel-title">
+ <a data-toggle="collapse" href="#query-profile-overview">
+ Overview
+ </a>
+ </h4>
+ </div>
+ <div id="query-profile-overview" class="panel-collapse collapse in">
+ <div class="panel-body">
+ <table class="table table-bordered">
+ <thead>
+ <tr>
+ <th>State</th>
+ <th>Foreman</th>
+ <th>Total Fragments</th>
+ </tr>
+ </thead>
+ <tbody>
+ <tr>
+ <td>${model.getProfile().getState().name()}</td>
+ <td>${model.getProfile().getForeman().getAddress()}</td>
+ <td>${model.getProfile().getTotalFragments()}</td>
+ </tr>
+ </tbody>
+ </table>
+ </div>
+ </div>
+ </div>
+ <div class="panel panel-default">
+ <div class="panel-heading">
+ <h4 class="panel-title">
+ <a data-toggle="collapse" href="#query-profile-duration">
+ Duration
+ </a>
+ </h4>
+ </div>
+ <div id="query-profile-duration" class="panel-collapse collapse">
+ <div class="panel-body">
+ <table class="table table-bordered">
+ <thead>
+ <tr>
+ <th>Planning</th>
+ <th>Queued</th>
+ <th>Execution</th>
+ <th>Total</th>
+ </tr>
+ </thead>
+ <tbody>
+ <tr>
+ <td>${model.getPlanningDuration()}</td>
+ <td>${model.getQueuedDuration()}</td>
+ <td>${model.getExecutionDuration()}</td>
+ <td>${model.getProfileDuration()}</td>
+ </tr>
+ </tbody>
+ </table>
+ </div>
+ </div>
+ </div>
+ </div>
<#assign options = model.getOptions()>
<#if (options?keys?size > 0)>
@@ -162,8 +226,7 @@
</div>
<div id="fragment-overview" class="panel-collapse collapse">
<div class="panel-body">
- <svg id="fragment-overview-canvas" class="center-block"></svg>
- ${model.getFragmentsOverview()}
+ ${model.getFragmentsOverview()?no_esc}
</div>
</div>
</div>
@@ -178,7 +241,7 @@
</div>
<div id="${frag.getId()}" class="panel-collapse collapse">
<div class="panel-body">
- ${frag.getContent()}
+ ${frag.getContent()?no_esc}
</div>
</div>
</div>
@@ -199,7 +262,7 @@
</div>
<div id="operator-overview" class="panel-collapse collapse">
<div class="panel-body">
- ${model.getOperatorsOverview()}
+ ${model.getOperatorsOverview()?no_esc}
</div>
</div>
</div>
@@ -215,7 +278,7 @@
</div>
<div id="${op.getId()}" class="panel-collapse collapse">
<div class="panel-body">
- ${op.getContent()}
+ ${op.getContent()?no_esc}
</div>
<div class="panel panel-info">
<div class="panel-heading">
@@ -227,7 +290,7 @@
</div>
<div id="${op.getId()}-metrics" class="panel-collapse collapse">
<div class="panel-body" style="display:block;overflow-x:auto">
- ${op.getMetricsTable()}
+ ${op.getMetricsTable()?no_esc}
</div>
</div>
</div>
http://git-wip-us.apache.org/repos/asf/drill/blob/c7c8ffd6/exec/java-exec/src/main/resources/rest/query/result.ftl
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/resources/rest/query/result.ftl b/exec/java-exec/src/main/resources/rest/query/result.ftl
index 8382e4f..3e1f49f 100644
--- a/exec/java-exec/src/main/resources/rest/query/result.ftl
+++ b/exec/java-exec/src/main/resources/rest/query/result.ftl
@@ -31,7 +31,7 @@
<thead>
<tr>
<#list model.getColumns() as value>
- <th>${value?html}</th>
+ <th>${value}</th>
</#list>
</tr>
</thead>
@@ -39,7 +39,7 @@
<#list model.getRows() as record>
<tr>
<#list record as value>
- <td>${value!"null"?html}</td>
+ <td>${value!"null"}</td>
</#list>
</tr>
</#list>
http://git-wip-us.apache.org/repos/asf/drill/blob/c7c8ffd6/pom.xml
----------------------------------------------------------------------
diff --git a/pom.xml b/pom.xml
index 6bd5b66..a843960 100644
--- a/pom.xml
+++ b/pom.xml
@@ -52,7 +52,7 @@
<hadoop.version>2.7.1</hadoop.version>
<hbase.version>1.1.3</hbase.version>
<fmpp.version>0.9.15</fmpp.version>
- <freemarker.version>2.3.21</freemarker.version>
+ <freemarker.version>2.3.26-incubating</freemarker.version>
</properties>
<scm>