You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by mi...@apache.org on 2026/10/07 02:49:56 UTC
(impala) branch master updated (893cf933b -> bb537fbea)
This is an automated email from the ASF dual-hosted git repository.
asf-gitbox-commits pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
from 893cf933b IMPALA-11917: Codegen branch-free reads of nullable scalar slots
new 0281cdad3 IMPALA-15345: Resolve '..' segments before trusted_jar_paths check
new 384a41b8d IMPALA-15448: Executor Webserver JWT/OAuth Auth Removal
new bb537fbea IMPALA-15452: Fix rendering of special characters in the Web UI
The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
be/src/util/jwt-util.cc | 3 +-
.../org/apache/impala/service/BackendConfig.java | 44 ++++++++++++++++++--
.../apache/impala/service/BackendConfigTest.java | 47 ++++++++++++++++++++++
www/queries.tmpl | 16 +++++---
www/query_plan.tmpl | 22 +++++++---
www/query_stmt.tmpl | 4 +-
6 files changed, 120 insertions(+), 16 deletions(-)
(impala) 03/03: IMPALA-15452: Fix rendering of special characters in the Web UI
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
asf-gitbox-commits pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit bb537fbea23bd880b7fd9dc81f7ddbf9636dfa67
Author: Zoltan Borok-Nagy <bo...@cloudera.com>
AuthorDate: Mon Sep 28 18:16:36 2026 +0200
IMPALA-15452: Fix rendering of special characters in the Web UI
Plan node labels on the query plan page and the fields of imported
query profiles were added to the page as HTML, so values containing
characters like '<' or '&' were displayed incorrectly. Add them as
text instead.
Testing:
- Checked the modified scripts with values that contain special
characters.
Change-Id: I1dce193718317a32df5b9977444b8f44ebc1ecec
Assisted-by: Claude Fable 5.1 (Claude Code)
Reviewed-on: http://gerrit.cloudera.org:8080/25038
Reviewed-by: Peter Rozsa <pr...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
www/queries.tmpl | 16 ++++++++++------
www/query_plan.tmpl | 22 +++++++++++++++++-----
www/query_stmt.tmpl | 4 +++-
3 files changed, 30 insertions(+), 12 deletions(-)
diff --git a/www/queries.tmpl b/www/queries.tmpl
index 776e4c519..ae90e4198 100644
--- a/www/queries.tmpl
+++ b/www/queries.tmpl
@@ -299,8 +299,10 @@ command line parameter.</p>
"{{ __common__.host-url }}/www/scripts/queries/profileParseWorker.js");
let query_processor_start_time, raw_total_size, compressed_total_size, upload_count;
+ // Adds a cell to 'row' that shows 'val' as text. The values of the imported queries
+ // come from user supplied profiles, so they must not be interpreted as HTML.
function insertRowVal(row, val) {
- row.insertCell().innerHTML = val;
+ row.insertCell().textContent = val;
}
function setScrollReload() {
@@ -407,15 +409,17 @@ command line parameter.</p>
const row = imported_queries_table.insertRow();
const query_link = document.createElement("a");
query_link.href = `{{ __common__.host-url }}/query_timeline?query_id=${query.id}&imported=true`;
- query_link.innerHTML = query.id;
- insertRowVal(row, query_link.outerHTML);
+ query_link.textContent = query.id;
+ row.insertCell().appendChild(query_link);
insertRowVal(row, query.user);
insertRowVal(row, query.default_db);
insertRowVal(row, query.type);
insertRowVal(row, query.start_time);
insertRowVal(row, query.end_time);
- insertRowVal(row, `${query.bytes_read}
- <hr style="margin-top:0px;margin-bottom:0px;"/>${query.bytes_sent}`);
+ const bytes_separator = document.createElement("hr");
+ bytes_separator.style.marginTop = "0px";
+ bytes_separator.style.marginBottom = "0px";
+ row.insertCell().append(query.bytes_read, bytes_separator, query.bytes_sent);
insertRowVal(row, query.state);
insertRowVal(row, query.rows_fetched);
insertRowVal(row, query.resource_pool);
@@ -427,7 +431,7 @@ command line parameter.</p>
deleteButton.value = "X";
deleteButton.className = "btn btn-primary";
deleteButton.setAttribute("onclick", "deleteProfile(this);");
- insertRowVal(row, deleteButton.outerHTML);
+ row.insertCell().appendChild(deleteButton);
cursor.continue();
}
};
diff --git a/www/query_plan.tmpl b/www/query_plan.tmpl
index a54682d01..ed672a7e2 100644
--- a/www/query_plan.tmpl
+++ b/www/query_plan.tmpl
@@ -139,6 +139,14 @@ const colours = ["#A9A9A9", "#FF8C00", "#8A2BE2", "#A52A2A", "#00008B", "#006400
const cols_by_time = ["#000000", "#1A0500", "#330A00", "#4C0F00", "#661400", "#801A00",
"#991F00", "#B22400", "#CC2900", "#E62E00", "#FF3300", "#FF4719"];
+// Escapes text before it is interpolated into the HTML labels of the plan graph.
+// Plan node labels and details embed user supplied identifiers, e.g. table aliases,
+// which must be rendered as text instead of being interpreted as markup.
+function escapeHtml(text) {
+ return String(text).replace(/&/g, "&").replace(/</g, "<")
+ .replace(/>/g, ">").replace(/"/g, """).replace(/'/g, "'");
+}
+
// Recursively build a list of edges and states that comprise the plan graph
function build(node, parent, edges, states, colour_idx, max_node_time) {
states.push({ "name": node["label"],
@@ -209,15 +217,19 @@ function renderGraph(ignored_arg) {
// Keep a map of names to states for use when processing edges.
const states_by_name = { }
states.forEach((state) => {
- // Build the label for the node from the name and the detail
- let html = "<span>" + state.name + "</span><br/>";
- html += "<span>" + state.detail + "</span><br/>";
- html += "<span>" + state.num_instances + " instance";
+ // Build the label for the node from the name and the detail. Dagre-D3 inserts
+ // this label as HTML, so every interpolated value must be escaped: the label
+ // detail of a scan node contains the table alias, which is arbitrary user
+ // supplied text.
+ let html = "<span>" + escapeHtml(state.name) + "</span><br/>";
+ html += "<span>" + escapeHtml(state.detail) + "</span><br/>";
+ html += "<span>" + escapeHtml(state.num_instances) + " instance";
if (state.num_instances > 1) {
html += "s";
}
html += "</span><br/>";
- html += "<span>Max: " + state.max_time + ", avg: " + state.avg_time + "</span>";
+ html += "<span>Max: " + escapeHtml(state.max_time) + ", avg: "
+ + escapeHtml(state.avg_time) + "</span>";
let style = state.style;
diff --git a/www/query_stmt.tmpl b/www/query_stmt.tmpl
index 391f657ce..fdfdf239a 100644
--- a/www/query_stmt.tmpl
+++ b/www/query_stmt.tmpl
@@ -73,7 +73,9 @@ if (window.location.search.includes("imported")) {
const sql_stmt_body = document.createElement("pre");
sql_stmt_body.className = "code";
const sql_code = sql_stmt_body.appendChild(document.createElement("code"))
- sql_code.innerHTML = sql_query;
+ // The statement comes from a user supplied profile, it must not be interpreted as
+ // HTML. highlightBlock() escapes the text when it adds the highlighting markup.
+ sql_code.textContent = sql_query;
tab_body.innerHTML = "";
tab_body.appendChild(sql_stmt_body);
hljs.highlightBlock(sql_stmt_body);
(impala) 01/03: IMPALA-15345: Resolve '..' segments before trusted_jar_paths check
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
asf-gitbox-commits pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 0281cdad344f11a716f5ea58a9c5dc33939665d1
Author: Michael Smith <mi...@cloudera.com>
AuthorDate: Fri Sep 11 16:16:26 2026 -0700
IMPALA-15345: Resolve '..' segments before trusted_jar_paths check
BackendConfig.isJarPathAllowed() compared the raw driver.url / HDFS
location string against the configured --trusted_jar_paths prefixes
using String#startsWith(). That string is later turned into a Hadoop
Path and resolved by the filesystem, which collapses '.' and '..'
segments. A URL such as '<trusted_dir>/../../secret.jar' therefore
passed the prefix check while actually resolving to a JAR outside the
trusted directory, letting Impala load and execute attacker-controlled
code in impalad.
Resolve both the candidate path and each allowlist entry through
Hadoop's Path class (which performs the same '.'/'..' collapsing)
before comparing, and require the resolved path to be nested inside,
or equal to, a resolved trusted directory rather than merely share a
string prefix.
Testing:
- Added BackendConfigTest cases covering '..'-escape rejection,
legitimate in-directory '..'/'.' usage, sibling-directory-name
spoofing, and malformed allowlist entries.
Change-Id: I4288aba344d7d0cd07c9fb8281fc8d3c0490e432
Assisted-by: Claude Sonnet 5 (GitHub Copilot)
Reviewed-on: http://gerrit.cloudera.org:8080/25036
Reviewed-by: Peter Rozsa <pr...@cloudera.com>
Tested-by: Michael Smith <mi...@cloudera.com>
---
.../org/apache/impala/service/BackendConfig.java | 44 ++++++++++++++++++--
.../apache/impala/service/BackendConfigTest.java | 47 ++++++++++++++++++++++
2 files changed, 88 insertions(+), 3 deletions(-)
diff --git a/fe/src/main/java/org/apache/impala/service/BackendConfig.java b/fe/src/main/java/org/apache/impala/service/BackendConfig.java
index 979ace136..f8b535842 100644
--- a/fe/src/main/java/org/apache/impala/service/BackendConfig.java
+++ b/fe/src/main/java/org/apache/impala/service/BackendConfig.java
@@ -23,6 +23,7 @@ import static org.apache.hadoop.fs.CommonConfigurationKeysPublic
import java.util.Scanner;
import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.Path;
import org.apache.hadoop.security.authentication.util.KerberosName;
import org.apache.impala.analysis.SqlScanner;
import org.apache.impala.thrift.TBackendGflags;
@@ -659,19 +660,56 @@ public class BackendConfig {
}
/**
- * Returns true if 'path' starts with one of the allowed prefixes configured in
- * --trusted_jar_paths. An empty allowlist (the default) disables jar
+ * Returns true if 'path', once resolved the same way Hadoop's Path class resolves
+ * it (collapsing '.' and '..' segments), is inside one of the directories
+ * configured in --trusted_jar_paths. An empty allowlist (the default) disables jar
* loading from table properties entirely.
+ *
+ * 'path' must be resolved (rather than compared as a raw string) because the caller
+ * eventually turns it into a Hadoop Path before reading the file, and Hadoop
+ * resolves '..' segments at that point. Comparing the raw, unresolved string would
+ * let a path like '<trusted_dir>/../../secret.jar' pass the allowlist check while
+ * actually resolving to a location outside the trusted directory.
*/
public boolean isJarPathAllowed(String path) {
String allowlist = backendCfg_.trusted_jar_paths;
if (Strings.isNullOrEmpty(allowlist) || Strings.isNullOrEmpty(path)) return false;
+ String resolvedPath = resolveJarPath(path);
+ if (resolvedPath == null) return false;
try (Scanner scanner = new Scanner(allowlist).useDelimiter(",")) {
while (scanner.hasNext()) {
String prefix = scanner.next().trim();
- if (!prefix.isEmpty() && path.startsWith(prefix)) return true;
+ if (prefix.isEmpty()) continue;
+ String resolvedPrefix = resolveJarPath(prefix);
+ if (resolvedPrefix != null && isWithinTrustedDir(resolvedPath, resolvedPrefix)) {
+ return true;
+ }
}
}
return false;
}
+
+ /**
+ * Resolves 'rawPath' via Hadoop's Path class so that '.' and '..' segments are
+ * collapsed the same way they will be when the path is later used to read a file.
+ * Returns null if 'rawPath' cannot be parsed as a Path.
+ */
+ private static String resolveJarPath(String rawPath) {
+ try {
+ return new Path(rawPath).toString();
+ } catch (IllegalArgumentException e) {
+ return null;
+ }
+ }
+
+ /**
+ * Returns true if resolved path 'path' is exactly the resolved directory
+ * 'trustedDir', or is nested inside it. Both arguments must already be resolved via
+ * resolveJarPath().
+ */
+ private static boolean isWithinTrustedDir(String path, String trustedDir) {
+ if (path.equals(trustedDir)) return true;
+ String trustedDirWithSlash = trustedDir.endsWith("/") ? trustedDir : trustedDir + "/";
+ return path.startsWith(trustedDirWithSlash);
+ }
}
diff --git a/fe/src/test/java/org/apache/impala/service/BackendConfigTest.java b/fe/src/test/java/org/apache/impala/service/BackendConfigTest.java
index f56355010..a4f161887 100644
--- a/fe/src/test/java/org/apache/impala/service/BackendConfigTest.java
+++ b/fe/src/test/java/org/apache/impala/service/BackendConfigTest.java
@@ -111,4 +111,51 @@ public class BackendConfigTest {
BackendConfig cfg = configWith(",");
assertFalse(cfg.isJarPathAllowed("/trusted/foo.jar"));
}
+
+ // --- isJarPathAllowed: path traversal ('..') resolution ---
+
+ @Test
+ public void testIsJarPathAllowed_dotDotEscapesTrustedDir_returnsFalse() {
+ // Raw string starts with the trusted prefix, but resolving '..' segments (the
+ // same way the caller's Hadoop Path will) lands outside the trusted directory.
+ BackendConfig cfg = configWith("/trusted/");
+ assertFalse(cfg.isJarPathAllowed("/trusted/../../secret/foo.jar"));
+ }
+
+ @Test
+ public void testIsJarPathAllowed_dotDotEscapesTrustedDirWithScheme_returnsFalse() {
+ BackendConfig cfg = configWith("hdfs://nn/trusted/");
+ assertFalse(cfg.isJarPathAllowed("hdfs://nn/trusted/../../secret/foo.jar"));
+ }
+
+ @Test
+ public void testIsJarPathAllowed_dotDotStaysWithinTrustedDir_returnsTrue() {
+ // '..' segments that resolve to a location still inside the trusted directory
+ // are legitimate and should be allowed.
+ BackendConfig cfg = configWith("/trusted/");
+ assertTrue(cfg.isJarPathAllowed("/trusted/sub/../foo.jar"));
+ }
+
+ @Test
+ public void testIsJarPathAllowed_dotSegmentIsNoOp_returnsTrue() {
+ BackendConfig cfg = configWith("/trusted/");
+ assertTrue(cfg.isJarPathAllowed("/trusted/./foo.jar"));
+ }
+
+ @Test
+ public void testIsJarPathAllowed_siblingDirWithSamePrefixSpelling_returnsFalse() {
+ // A directory named similarly to the trusted one (but not nested inside it) must
+ // not be treated as trusted just because the raw string starts with the prefix.
+ BackendConfig cfg = configWith("/trusted");
+ assertFalse(cfg.isJarPathAllowed("/trusted-evil/foo.jar"));
+ }
+
+ @Test
+ public void testIsJarPathAllowed_unparsablePrefix_isSkipped() {
+ // A malformed allowlist entry must not be able to match everything; it should be
+ // skipped, and a valid subsequent entry should still be usable.
+ BackendConfig cfg = configWith("::not a valid path::,/trusted/");
+ assertTrue(cfg.isJarPathAllowed("/trusted/foo.jar"));
+ assertFalse(cfg.isJarPathAllowed("/untrusted/foo.jar"));
+ }
}
(impala) 02/03: IMPALA-15448: Executor Webserver JWT/OAuth Auth Removal
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
asf-gitbox-commits pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 384a41b8d1c20946aafbadf94e1e187a7509e91a
Author: jasonmfehr <jf...@cloudera.com>
AuthorDate: Mon Sep 28 11:04:30 2026 -0700
IMPALA-15448: Executor Webserver JWT/OAuth Auth Removal
Removes non-functioning JWT/OAuth authentication from the executor's
debug webservers.
Manually tested using a combination of LDAP and JWT/OAuth auth.
Change-Id: I805461fbb73d579a00d5ab9dbca312199d1d2d9f
Reviewed-on: http://gerrit.cloudera.org:8080/25037
Reviewed-by: Peter Rozsa <pr...@cloudera.com>
Tested-by: Michael Smith <mi...@cloudera.com>
---
be/src/util/jwt-util.cc | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/be/src/util/jwt-util.cc b/be/src/util/jwt-util.cc
index 56cffc78a..ab0c6aff2 100644
--- a/be/src/util/jwt-util.cc
+++ b/be/src/util/jwt-util.cc
@@ -919,7 +919,8 @@ Status JWTHelper::Verify(const JWTDecodedToken* decoded_token) const {
return Status(TErrorCode::JWT_VERIFY_FAILED, "Unsecured JWT");
} else if (jwks_mgr_ == nullptr) {
// Skip to signature validation if JWKS file or url is not specified.
- return Status::OK();
+ return Status(TErrorCode::NOT_IMPLEMENTED_ERROR,
+ "Token auth not supported on this daemon.");
}
JWKSSnapshotPtr jwks = GetJWKS();