diff --git a/core/src/main/scala/org/apache/spark/deploy/history/LogPage.scala b/core/src/main/scala/org/apache/spark/deploy/history/LogPage.scala
index 1b809b80ee328..9b14e76db37a7 100644
--- a/core/src/main/scala/org/apache/spark/deploy/history/LogPage.scala
+++ b/core/src/main/scala/org/apache/spark/deploy/history/LogPage.scala
@@ -56,9 +56,9 @@ private[history] class LogPage(conf: SparkConf) extends WebUIPage("logPage") wit
End of Log
- val logParams = "?self&logType=%s".format(logType)
- val jsOnload = "window.onload = " +
- s"initLogPage('$logParams', $curLogLength, $startByte, $endByte, $logLength, $byteLength);"
+ val logParams = "?self&logType=%s".format(UIUtils.encodeLogParam(logType))
+ val jsOnload = UIUtils.logPageOnloadScript(
+ logParams, curLogLength, startByte, endByte, logLength, byteLength)
val content =
++
diff --git a/core/src/main/scala/org/apache/spark/deploy/master/ui/LogPage.scala b/core/src/main/scala/org/apache/spark/deploy/master/ui/LogPage.scala
index 780f8a2b6c53a..7da8736222026 100644
--- a/core/src/main/scala/org/apache/spark/deploy/master/ui/LogPage.scala
+++ b/core/src/main/scala/org/apache/spark/deploy/master/ui/LogPage.scala
@@ -55,9 +55,9 @@ private[ui] class LogPage(parent: MasterWebUI) extends WebUIPage("logPage") with
End of Log
- val logParams = "?self&logType=%s".format(logType)
- val jsOnload = "window.onload = " +
- s"initLogPage('$logParams', $curLogLength, $startByte, $endByte, $logLength, $byteLength);"
+ val logParams = "?self&logType=%s".format(UIUtils.encodeLogParam(logType))
+ val jsOnload = UIUtils.logPageOnloadScript(
+ logParams, curLogLength, startByte, endByte, logLength, byteLength)
val content =
++
diff --git a/core/src/main/scala/org/apache/spark/deploy/worker/ui/LogPage.scala b/core/src/main/scala/org/apache/spark/deploy/worker/ui/LogPage.scala
index adfe9670f371f..4c3c27c0d9f91 100644
--- a/core/src/main/scala/org/apache/spark/deploy/worker/ui/LogPage.scala
+++ b/core/src/main/scala/org/apache/spark/deploy/worker/ui/LogPage.scala
@@ -71,11 +71,16 @@ private[ui] class LogPage(parent: WorkerWebUI) extends WebUIPage("logPage") with
val byteLength = Option(request.getParameter("byteLength")).map(_.toInt)
.getOrElse(defaultBytes)
+ // The identifiers below flow both into a filesystem path (logDir, kept raw and guarded by
+ // Utils.isInDirectory in getLog) and into the query string of the generated /log requests
+ // (params). URL-encode the query-string copy so request-supplied values cannot inject extra
+ // parameters or truncate the request.
val (logDir, params, pageName) = (appId, executorId, driverId, self) match {
case (Some(a), Some(e), None, None) =>
- (s"${workDir.getPath}/$a/$e/", s"appId=$a&executorId=$e", s"$a/$e")
+ (s"${workDir.getPath}/$a/$e/",
+ s"appId=${UIUtils.encodeLogParam(a)}&executorId=${UIUtils.encodeLogParam(e)}", s"$a/$e")
case (None, None, Some(d), None) =>
- (s"${workDir.getPath}/$d/", s"driverId=$d", d)
+ (s"${workDir.getPath}/$d/", s"driverId=${UIUtils.encodeLogParam(d)}", d)
case (None, None, None, Some(_)) =>
(s"${sys.env.getOrElse("SPARK_LOG_DIR", workDir.getPath)}/", "self", "worker")
case _ =>
@@ -105,9 +110,9 @@ private[ui] class LogPage(parent: WorkerWebUI) extends WebUIPage("logPage") with
End of Log
- val logParams = "?%s&logType=%s".format(params, logType)
- val jsOnload = "window.onload = " +
- s"initLogPage('$logParams', $curLogLength, $startByte, $endByte, $logLength, $byteLength);"
+ val logParams = "?%s&logType=%s".format(params, UIUtils.encodeLogParam(logType))
+ val jsOnload = UIUtils.logPageOnloadScript(
+ logParams, curLogLength, startByte, endByte, logLength, byteLength)
val content =
++
diff --git a/core/src/main/scala/org/apache/spark/ui/DriverLogPage.scala b/core/src/main/scala/org/apache/spark/ui/DriverLogPage.scala
index 7b427cffcc5e7..9cd2fe50715f0 100644
--- a/core/src/main/scala/org/apache/spark/ui/DriverLogPage.scala
+++ b/core/src/main/scala/org/apache/spark/ui/DriverLogPage.scala
@@ -70,9 +70,9 @@ private[ui] class DriverLogPage(
End of Log
- val logParams = "/?logType=%s".format(logType)
- val jsOnload = "window.onload = " +
- s"initLogPage('$logParams', $curLogLength, $startByte, $endByte, $logLength, $byteLength);"
+ val logParams = "/?logType=%s".format(UIUtils.encodeLogParam(logType))
+ val jsOnload = UIUtils.logPageOnloadScript(
+ logParams, curLogLength, startByte, endByte, logLength, byteLength)
val content =
++
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 eedec979f7158..c0d36ef3a8acf 100644
--- a/core/src/main/scala/org/apache/spark/ui/UIUtils.scala
+++ b/core/src/main/scala/org/apache/spark/ui/UIUtils.scala
@@ -19,7 +19,7 @@ package org.apache.spark.ui
import java.{util => ju}
import java.lang.{Long => JLong}
-import java.net.URLDecoder
+import java.net.{URLDecoder, URLEncoder}
import java.nio.charset.StandardCharsets.UTF_8
import java.time.{Instant, ZoneId}
import java.time.format.DateTimeFormatter
@@ -32,6 +32,7 @@ import scala.xml.transform.{RewriteRule, RuleTransformer}
import jakarta.servlet.http.HttpServletRequest
import jakarta.ws.rs.core.{MediaType, MultivaluedMap, Response}
+import org.apache.commons.text.StringEscapeUtils
import org.eclipse.jetty.server.Request
import org.glassfish.jersey.internal.util.collection.MultivaluedStringMap
@@ -751,6 +752,33 @@ private[spark] object UIUtils extends Logging {
def getTimeZoneOffset() : Int =
TimeZone.getDefault().getOffset(System.currentTimeMillis()) / 1000 / 60
+ /**
+ * URL-encode a single request-supplied value before embedding it in a log page query string.
+ * This keeps characters such as '&', '=', '#' and whitespace from changing the structure of the
+ * subsequent `/log` requests (parameter injection / query truncation).
+ */
+ def encodeLogParam(value: String): String =
+ URLEncoder.encode(Option(value).getOrElse("null"), UTF_8)
+
+ /**
+ * Build the inline `window.onload = initLogPage(...)` bootstrap script shared by the log pages
+ * (history / master / worker / driver). Individual parameter values in `logParams` must already
+ * be URL-encoded via [[encodeLogParam]] so the query string is unambiguous; the assembled string
+ * is additionally escaped for the surrounding JavaScript string literal so request-supplied
+ * values cannot break out of the inline <script> block.
+ */
+ def logPageOnloadScript(
+ logParams: String,
+ curLogLength: Long,
+ startByte: Long,
+ endByte: Long,
+ logLength: Long,
+ byteLength: Long): String = {
+ "window.onload = " +
+ s"initLogPage('${StringEscapeUtils.escapeEcmaScript(logParams)}', " +
+ s"$curLogLength, $startByte, $endByte, $logLength, $byteLength);"
+ }
+
/**
* Return the correct Href after checking if master is running in the
* reverse proxy mode or not.
diff --git a/core/src/test/scala/org/apache/spark/deploy/history/LogPageSuite.scala b/core/src/test/scala/org/apache/spark/deploy/history/LogPageSuite.scala
new file mode 100644
index 0000000000000..f37eac47cceac
--- /dev/null
+++ b/core/src/test/scala/org/apache/spark/deploy/history/LogPageSuite.scala
@@ -0,0 +1,42 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file 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 KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.deploy.history
+
+import jakarta.servlet.http.HttpServletRequest
+import org.mockito.Mockito.{mock, when}
+
+import org.apache.spark.{SparkConf, SparkFunSuite}
+
+class LogPageSuite extends SparkFunSuite {
+
+ test("render encodes the logType parameter embedded in the inline script") {
+ val page = new LogPage(new SparkConf(false))
+ val request = mock(classOf[HttpServletRequest])
+ // A value that would break out of the single-quoted JavaScript string literal, close the
+ // script element, or start a new statement if it were emitted into the page verbatim.
+ when(request.getParameter("logType")).thenReturn("stdout');alert('xss')")
+ val html = page.render(request).mkString
+
+ // The value is URL-encoded, so the quotes, angle brackets and slash that would break out of
+ // the inline ' would break out of the inline ")
+ when(request.getParameter("executorId")).thenReturn("0")
+ when(request.getParameter("logType")).thenReturn("stdout")
+ val html = logPage.render(request).mkString
+
+ // The value is percent-encoded, so it stays a single opaque appId value and cannot inject a
+ // parameter or close the script element.
+ assert(html.contains("appId=app%26byteLength%3D0%3C%2Fscript%3E&executorId=0"))
+ assert(!html.contains("app&byteLength=0"))
+ }
+ }
+
/** Write the specified string to the file. */
private def write(f: File, s: String): Unit = {
val writer = new FileWriter(f)