This is an automated email from the ASF dual-hosted git repository.
dongjoon pushed a commit to branch branch-3.4
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-3.4 by this push:
new edaa0fd8d096 [SPARK-46893][UI] Remove inline scripts from UI
descriptions
edaa0fd8d096 is described below
commit edaa0fd8d096a3e57918e4b6e437337fcfdc8276
Author: Willi Raschkowski <wraschkow...@palantir.com>
AuthorDate: Mon Jan 29 22:43:21 2024 -0800
[SPARK-46893][UI] Remove inline scripts from UI descriptions
### What changes were proposed in this pull request?
This PR prevents malicious users from injecting inline scripts via job and
stage descriptions.
Spark's Web UI [already checks the security of job and stage
descriptions](https://github.com/apache/spark/blob/a368280708dd3c6eb90bd3b09a36a68bdd096222/core/src/main/scala/org/apache/spark/ui/UIUtils.scala#L528-L545)
before rendering them as HTML (or treating them as plain text). The UI already
disallows `<script>` tags but doesn't protect against attributes with inline
scripts like `onclick` or `onmouseover`.
### Why are the changes needed?
On multi-user clusters, bad users can inject scripts into their job and
stage descriptions. The UI already finds that [worth protecting
against](https://github.com/apache/spark/blob/a368280708dd3c6eb90bd3b09a36a68bdd096222/core/src/main/scala/org/apache/spark/ui/UIUtils.scala#L533-L535).
So this is extending that protection to scripts in attributes.
### Does this PR introduce _any_ user-facing change?
Yes if users relied on inline scripts or attributes in their job or stage
descriptions.
### How was this patch tested?
Added tests.
### Was this patch authored or co-authored using generative AI tooling?
No.
Closes #44933 from rshkv/wr/spark-46893.
Authored-by: Willi Raschkowski <wraschkow...@palantir.com>
Signed-off-by: Dongjoon Hyun <dh...@apple.com>
(cherry picked from commit abd9d27e87b915612e2a89e0d2527a04c7b984e0)
Signed-off-by: Dongjoon Hyun <dh...@apple.com>
---
core/src/main/scala/org/apache/spark/ui/UIUtils.scala | 12 +++++++++---
core/src/test/scala/org/apache/spark/ui/UIUtilsSuite.scala | 14 ++++++++++++++
2 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/core/src/main/scala/org/apache/spark/ui/UIUtils.scala
b/core/src/main/scala/org/apache/spark/ui/UIUtils.scala
index e03324ceb1b7..8685b4fc220a 100644
--- a/core/src/main/scala/org/apache/spark/ui/UIUtils.scala
+++ b/core/src/main/scala/org/apache/spark/ui/UIUtils.scala
@@ -555,8 +555,8 @@ private[spark] object UIUtils extends Logging {
* the whole string will rendered as a simple escaped text.
*
* Note: In terms of security, only anchor tags with root relative links are
supported. So any
- * attempts to embed links outside Spark UI, or other tags like
<script> will cause in
- * the whole description to be treated as plain text.
+ * attempts to embed links outside Spark UI, other tags like <script>,
or inline scripts
+ * like `onclick` will cause in the whole description to be treated as plain
text.
*
* @param desc the original job or stage description string, which
may contain html tags.
* @param basePathUri with which to prepend the relative links; this is used
when plainText is
@@ -576,7 +576,13 @@ private[spark] object UIUtils extends Logging {
// Verify that this has only anchors and span (we are wrapping in span)
val allowedNodeLabels = Set("a", "span", "br")
- val illegalNodes = (xml \\ "_").filterNot(node =>
allowedNodeLabels.contains(node.label))
+ val allowedAttributes = Set("class", "href")
+ val illegalNodes =
+ (xml \\ "_").filterNot { node =>
+ allowedNodeLabels.contains(node.label) &&
+ // Verify we only have href attributes
+ node.attributes.map(_.key).forall(allowedAttributes.contains)
+ }
if (illegalNodes.nonEmpty) {
throw new IllegalArgumentException(
"Only HTML anchors allowed in job descriptions\n" +
diff --git a/core/src/test/scala/org/apache/spark/ui/UIUtilsSuite.scala
b/core/src/test/scala/org/apache/spark/ui/UIUtilsSuite.scala
index 9d040bb4e1ec..4c4f19f61405 100644
--- a/core/src/test/scala/org/apache/spark/ui/UIUtilsSuite.scala
+++ b/core/src/test/scala/org/apache/spark/ui/UIUtilsSuite.scala
@@ -61,6 +61,20 @@ class UIUtilsSuite extends SparkFunSuite {
errorMsg = "Base URL should be prepended to html links",
plainText = false
)
+
+ verify(
+ """<a onclick="alert('oops');"></a>""",
+ <span class="description-input">{"""<a
onclick="alert('oops');"></a>"""}</span>,
+ "Non href attributes should make the description be treated as a string
instead of HTML",
+ plainText = false
+ )
+
+ verify(
+ """<a onmouseover="alert('oops');"></a>""",
+ <span class="description-input">{"""<a
onmouseover="alert('oops');"></a>"""}</span>,
+ "Non href attributes should make the description be treated as a string
instead of HTML",
+ plainText = false
+ )
}
test("makeDescription(plainText = true)") {
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org
For additional commands, e-mail: commits-h...@spark.apache.org
