Skip to content

Commit e896629

Browse files
Validate app store path in history server disk management
Co-authored-by: Holden Karau <holden@pigscanfly.ca>
1 parent c7190ed commit e896629

2 files changed

Lines changed: 26 additions & 2 deletions

File tree

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -266,8 +266,15 @@ private class HistoryServerDiskManager(
266266
}
267267

268268
private[history] def appStorePath(appId: String, attemptId: Option[String]): File = {
269-
val fileName = appId + attemptId.map("_" + _).getOrElse("") + extension
270-
new File(appStoreDir, fileName)
269+
val fileName = Utils.sanitizeDirName(appId) +
270+
attemptId.map("_" + Utils.sanitizeDirName(_)).getOrElse("") + extension
271+
val storePath = new File(appStoreDir, fileName)
272+
// Validate the app store path is valid
273+
if (storePath.getCanonicalFile.getParentFile != appStoreDir.getCanonicalFile) {
274+
throw new IllegalArgumentException(
275+
s"Store path for app $appId / $attemptId escapes the application store directory")
276+
}
277+
storePath
271278
}
272279

273280
private def updateApplicationStoreInfo(

‎core/src/test/scala/org/apache/spark/deploy/history/HistoryServerDiskManagerSuite.scala‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,23 @@ abstract class HistoryServerDiskManagerSuite extends SparkFunSuite with BeforeAn
257257
val manager = new HistoryServerDiskManager(conf, testDir, store, new ManualClock())
258258
assert(manager.appStorePath("appId", None).getName.endsWith(extension))
259259
}
260+
261+
test("appStorePath neutralizes path traversal in log-derived app and attempt ids") {
262+
val conf = new SparkConf().set(HYBRID_STORE_DISK_BACKEND, backend.toString)
263+
val manager = new HistoryServerDiskManager(conf, testDir, store, new ManualClock())
264+
val appsDir = new File(testDir, "apps").getCanonicalFile
265+
266+
// Legitimate cluster-manager-generated ids keep their store name.
267+
assert(manager.appStorePath("app-20260811120000-0001", Some("1")).getName ===
268+
"app-20260811120000-0001_1" + extension)
269+
270+
// Reject the non-standard paths
271+
Seq("../listing", "..", "../../tmp/evil", "a/b").foreach { appId =>
272+
val storePath = manager.appStorePath(appId, Some("../1"))
273+
assert(storePath.getCanonicalFile.getParentFile === appsDir,
274+
s"appId '$appId' escaped the store directory: $storePath")
275+
}
276+
}
260277
}
261278

262279
@ExtendedLevelDBTest

0 commit comments

Comments
 (0)