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)