Skip to content

Commit f99456b

Browse files
n-marionsrowen
n-marion
authored and
srowen
committed
[SPARK-20393][WEBU UI] Strengthen Spark to prevent XSS vulnerabilities
## What changes were proposed in this pull request? Add stripXSS and stripXSSMap to Spark Core's UIUtils. Calling these functions at any point that getParameter is called against a HttpServletRequest. ## How was this patch tested? Unit tests, IBM Security AppScan Standard no longer showing vulnerabilities, manual verification of WebUI pages. Author: NICHOLAS T. MARION <[email protected]> Closes #17686 from n-marion/xss-fix. (cherry picked from commit b512233) Signed-off-by: Sean Owen <[email protected]>
1 parent fafe283 commit f99456b

File tree

19 files changed

+140
-54
lines changed

19 files changed

+140
-54
lines changed

‎core/src/main/scala/org/apache/spark/deploy/history/HistoryPage.scala‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,9 @@ import org.apache.spark.ui.{UIUtils, WebUIPage}
2626
private[history] class HistoryPage(parent: HistoryServer) extends WebUIPage("") {
2727

2828
def render(request: HttpServletRequest): Seq[Node] = {
29+
// stripXSS is called first to remove suspicious characters used in XSS attacks
2930
val requestedIncomplete =
30-
Option(request.getParameter("showIncomplete")).getOrElse("false").toBoolean
31+
Option(UIUtils.stripXSS(request.getParameter("showIncomplete"))).getOrElse("false").toBoolean
3132

3233
val allAppsSize = parent.getApplicationList().count(_.completed != requestedIncomplete)
3334
val eventLogsUnderProcessCount = parent.getEventLogsUnderProcess()

‎core/src/main/scala/org/apache/spark/deploy/master/ui/ApplicationPage.scala‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@ private[ui] class ApplicationPage(parent: MasterWebUI) extends WebUIPage("app")
3333

3434
/** Executor details for a particular application */
3535
def render(request: HttpServletRequest): Seq[Node] = {
36-
val appId = request.getParameter("appId")
36+
// stripXSS is called first to remove suspicious characters used in XSS attacks
37+
val appId = UIUtils.stripXSS(request.getParameter("appId"))
3738
val state = master.askSync[MasterStateResponse](RequestMasterState)
3839
val app = state.activeApps.find(_.id == appId)
3940
.getOrElse(state.completedApps.find(_.id == appId).orNull)

‎core/src/main/scala/org/apache/spark/deploy/master/ui/MasterPage.scala‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,10 @@ private[ui] class MasterPage(parent: MasterWebUI) extends WebUIPage("") {
5757
private def handleKillRequest(request: HttpServletRequest, action: String => Unit): Unit = {
5858
if (parent.killEnabled &&
5959
parent.master.securityMgr.checkModifyPermissions(request.getRemoteUser)) {
60-
val killFlag = Option(request.getParameter("terminate")).getOrElse("false").toBoolean
61-
val id = Option(request.getParameter("id"))
60+
// stripXSS is called first to remove suspicious characters used in XSS attacks
61+
val killFlag =
62+
Option(UIUtils.stripXSS(request.getParameter("terminate"))).getOrElse("false").toBoolean
63+
val id = Option(UIUtils.stripXSS(request.getParameter("id")))
6264
if (id.isDefined && killFlag) {
6365
action(id.get)
6466
}

‎core/src/main/scala/org/apache/spark/deploy/worker/ui/LogPage.scala‎

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,16 @@ private[ui] class LogPage(parent: WorkerWebUI) extends WebUIPage("logPage") with
3333
private val supportedLogTypes = Set("stderr", "stdout")
3434
private val defaultBytes = 100 * 1024
3535

36+
// stripXSS is called first to remove suspicious characters used in XSS attacks
3637
def renderLog(request: HttpServletRequest): String = {
37-
val appId = Option(request.getParameter("appId"))
38-
val executorId = Option(request.getParameter("executorId"))
39-
val driverId = Option(request.getParameter("driverId"))
40-
val logType = request.getParameter("logType")
41-
val offset = Option(request.getParameter("offset")).map(_.toLong)
42-
val byteLength = Option(request.getParameter("byteLength")).map(_.toInt).getOrElse(defaultBytes)
38+
val appId = Option(UIUtils.stripXSS(request.getParameter("appId")))
39+
val executorId = Option(UIUtils.stripXSS(request.getParameter("executorId")))
40+
val driverId = Option(UIUtils.stripXSS(request.getParameter("driverId")))
41+
val logType = UIUtils.stripXSS(request.getParameter("logType"))
42+
val offset = Option(UIUtils.stripXSS(request.getParameter("offset"))).map(_.toLong)
43+
val byteLength =
44+
Option(UIUtils.stripXSS(request.getParameter("byteLength"))).map(_.toInt)
45+
.getOrElse(defaultBytes)
4346

4447
val logDir = (appId, executorId, driverId) match {
4548
case (Some(a), Some(e), None) =>
@@ -55,13 +58,16 @@ private[ui] class LogPage(parent: WorkerWebUI) extends WebUIPage("logPage") with
5558
pre + logText
5659
}
5760

61+
// stripXSS is called first to remove suspicious characters used in XSS attacks
5862
def render(request: HttpServletRequest): Seq[Node] = {
59-
val appId = Option(request.getParameter("appId"))
60-
val executorId = Option(request.getParameter("executorId"))
61-
val driverId = Option(request.getParameter("driverId"))
62-
val logType = request.getParameter("logType")
63-
val offset = Option(request.getParameter("offset")).map(_.toLong)
64-
val byteLength = Option(request.getParameter("byteLength")).map(_.toInt).getOrElse(defaultBytes)
63+
val appId = Option(UIUtils.stripXSS(request.getParameter("appId")))
64+
val executorId = Option(UIUtils.stripXSS(request.getParameter("executorId")))
65+
val driverId = Option(UIUtils.stripXSS(request.getParameter("driverId")))
66+
val logType = UIUtils.stripXSS(request.getParameter("logType"))
67+
val offset = Option(UIUtils.stripXSS(request.getParameter("offset"))).map(_.toLong)
68+
val byteLength =
69+
Option(UIUtils.stripXSS(request.getParameter("byteLength"))).map(_.toInt)
70+
.getOrElse(defaultBytes)
6571

6672
val (logDir, params, pageName) = (appId, executorId, driverId) match {
6773
case (Some(a), Some(e), None) =>

‎core/src/main/scala/org/apache/spark/ui/UIUtils.scala‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ import scala.util.control.NonFatal
2525
import scala.xml._
2626
import scala.xml.transform.{RewriteRule, RuleTransformer}
2727

28+
import org.apache.commons.lang3.StringEscapeUtils
29+
2830
import org.apache.spark.internal.Logging
2931
import org.apache.spark.ui.scope.RDDOperationGraph
3032

@@ -34,6 +36,8 @@ private[spark] object UIUtils extends Logging {
3436
val TABLE_CLASS_STRIPED = TABLE_CLASS_NOT_STRIPED + " table-striped"
3537
val TABLE_CLASS_STRIPED_SORTABLE = TABLE_CLASS_STRIPED + " sortable"
3638

39+
private val NEWLINE_AND_SINGLE_QUOTE_REGEX = raw"(?i)(\r\n|\n|\r|%0D%0A|%0A|%0D|'|%27)".r
40+
3741
// SimpleDateFormat is not thread-safe. Don't expose it to avoid improper use.
3842
private val dateFormat = new ThreadLocal[SimpleDateFormat]() {
3943
override def initialValue(): SimpleDateFormat =
@@ -527,4 +531,21 @@ private[spark] object UIUtils extends Logging {
527531
origHref
528532
}
529533
}
534+
535+
/**
536+
* Remove suspicious characters of user input to prevent Cross-Site scripting (XSS) attacks
537+
*
538+
* For more information about XSS testing:
539+
* https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet and
540+
* https://www.owasp.org/index.php/Testing_for_Reflected_Cross_site_scripting_(OTG-INPVAL-001)
541+
*/
542+
def stripXSS(requestParameter: String): String = {
543+
if (requestParameter == null) {
544+
null
545+
} else {
546+
// Remove new lines and single quotes, followed by escaping HTML version 4.0
547+
StringEscapeUtils.escapeHtml4(
548+
NEWLINE_AND_SINGLE_QUOTE_REGEX.replaceAllIn(requestParameter, ""))
549+
}
550+
}
530551
}

‎core/src/main/scala/org/apache/spark/ui/exec/ExecutorThreadDumpPage.scala‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,10 @@ private[ui] class ExecutorThreadDumpPage(parent: ExecutorsTab) extends WebUIPage
2828

2929
private val sc = parent.sc
3030

31+
// stripXSS is called first to remove suspicious characters used in XSS attacks
3132
def render(request: HttpServletRequest): Seq[Node] = {
32-
val executorId = Option(request.getParameter("executorId")).map { executorId =>
33+
val executorId =
34+
Option(UIUtils.stripXSS(request.getParameter("executorId"))).map { executorId =>
3335
UIUtils.decodeURLParameter(executorId)
3436
}.getOrElse {
3537
throw new IllegalArgumentException(s"Missing executorId parameter")

‎core/src/main/scala/org/apache/spark/ui/jobs/AllJobsPage.scala‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -220,18 +220,20 @@ private[ui] class AllJobsPage(parent: JobsTab) extends WebUIPage("") {
220220
jobTag: String,
221221
jobs: Seq[JobUIData],
222222
killEnabled: Boolean): Seq[Node] = {
223-
val allParameters = request.getParameterMap.asScala.toMap
223+
// stripXSS is called to remove suspicious characters used in XSS attacks
224+
val allParameters = request.getParameterMap.asScala.toMap.mapValues(_.map(UIUtils.stripXSS))
224225
val parameterOtherTable = allParameters.filterNot(_._1.startsWith(jobTag))
225226
.map(para => para._1 + "=" + para._2(0))
226227

227228
val someJobHasJobGroup = jobs.exists(_.jobGroup.isDefined)
228229
val jobIdTitle = if (someJobHasJobGroup) "Job Id (Job Group)" else "Job Id"
229230

230-
val parameterJobPage = request.getParameter(jobTag + ".page")
231-
val parameterJobSortColumn = request.getParameter(jobTag + ".sort")
232-
val parameterJobSortDesc = request.getParameter(jobTag + ".desc")
233-
val parameterJobPageSize = request.getParameter(jobTag + ".pageSize")
234-
val parameterJobPrevPageSize = request.getParameter(jobTag + ".prevPageSize")
231+
// stripXSS is called first to remove suspicious characters used in XSS attacks
232+
val parameterJobPage = UIUtils.stripXSS(request.getParameter(jobTag + ".page"))
233+
val parameterJobSortColumn = UIUtils.stripXSS(request.getParameter(jobTag + ".sort"))
234+
val parameterJobSortDesc = UIUtils.stripXSS(request.getParameter(jobTag + ".desc"))
235+
val parameterJobPageSize = UIUtils.stripXSS(request.getParameter(jobTag + ".pageSize"))
236+
val parameterJobPrevPageSize = UIUtils.stripXSS(request.getParameter(jobTag + ".prevPageSize"))
235237

236238
val jobPage = Option(parameterJobPage).map(_.toInt).getOrElse(1)
237239
val jobSortColumn = Option(parameterJobSortColumn).map { sortColumn =>

‎core/src/main/scala/org/apache/spark/ui/jobs/JobPage.scala‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,8 @@ private[ui] class JobPage(parent: JobsTab) extends WebUIPage("job") {
187187
val listener = parent.jobProgresslistener
188188

189189
listener.synchronized {
190-
val parameterId = request.getParameter("id")
190+
// stripXSS is called first to remove suspicious characters used in XSS attacks
191+
val parameterId = UIUtils.stripXSS(request.getParameter("id"))
191192
require(parameterId != null && parameterId.nonEmpty, "Missing id parameter")
192193

193194
val jobId = parameterId.toInt

‎core/src/main/scala/org/apache/spark/ui/jobs/JobsTab.scala‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ package org.apache.spark.ui.jobs
2020
import javax.servlet.http.HttpServletRequest
2121

2222
import org.apache.spark.scheduler.SchedulingMode
23-
import org.apache.spark.ui.{SparkUI, SparkUITab}
23+
import org.apache.spark.ui.{SparkUI, SparkUITab, UIUtils}
2424

2525
/** Web UI showing progress status of all jobs in the given SparkContext. */
2626
private[ui] class JobsTab(parent: SparkUI) extends SparkUITab(parent, "jobs") {
@@ -40,7 +40,8 @@ private[ui] class JobsTab(parent: SparkUI) extends SparkUITab(parent, "jobs") {
4040

4141
def handleKillRequest(request: HttpServletRequest): Unit = {
4242
if (killEnabled && parent.securityManager.checkModifyPermissions(request.getRemoteUser)) {
43-
val jobId = Option(request.getParameter("id")).map(_.toInt)
43+
// stripXSS is called first to remove suspicious characters used in XSS attacks
44+
val jobId = Option(UIUtils.stripXSS(request.getParameter("id"))).map(_.toInt)
4445
jobId.foreach { id =>
4546
if (jobProgresslistener.activeJobs.contains(id)) {
4647
sc.foreach(_.cancelJob(id))

‎core/src/main/scala/org/apache/spark/ui/jobs/PoolPage.scala‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,8 @@ private[ui] class PoolPage(parent: StagesTab) extends WebUIPage("pool") {
3131

3232
def render(request: HttpServletRequest): Seq[Node] = {
3333
listener.synchronized {
34-
val poolName = Option(request.getParameter("poolname")).map { poolname =>
34+
// stripXSS is called first to remove suspicious characters used in XSS attacks
35+
val poolName = Option(UIUtils.stripXSS(request.getParameter("poolname"))).map { poolname =>
3536
UIUtils.decodeURLParameter(poolname)
3637
}.getOrElse {
3738
throw new IllegalArgumentException(s"Missing poolname parameter")

0 commit comments

Comments
 (0)