ADFA-5005: Fix SDK bootstrap crash extracting android-sdk.zip

app/src/main/java/com/itsaky/androidide/assets/AssetsInstallationHelper.kt

@@ -245,6 +245,14 @@ object AssetsInstallationHelper {
  Files.createDirectories(destDir)
  // Normalize and make destDir absolute for secure path validation
  val normalizedDestDir = destDir.toAbsolutePath().normalize()
+ val realDestDir = normalizedDestDir.toRealPath()
+
+ // Zip entries are commonly clustered by directory (e.g. dozens of files
+ // under the same build-tools/<version>/ prefix); cache the last-verified
+ // parent so consecutive entries under it skip a redundant toRealPath() call.
+ // Nothing below can turn an already-verified real directory into a symlink
+ // mid-run, so caching by lexical parent equality is safe.
+ var lastVerifiedParent: Path? = null
 
  ZipInputStream(srcStream.buffered()).useEntriesEach { zipInput, entry ->
  // Validate entry name doesn't contain dangerous patterns
@@ -260,9 +268,28 @@ object AssetsInstallationHelper {
  throw IllegalStateException("Entry is outside of the target dir: ${entry.name}")
  }
 
+ // The checks above are lexical (entry name only) and don't catch a symlink
+ // already present on disk (e.g. destDir merged/reused across installer
+ // runs). Reject writing through an existing symlink up front, then
+ // re-check containment against the real, on-disk path once created.
+ if (Files.isSymbolicLink(destFile)) {
+ throw IllegalStateException("Refusing to extract over an existing symlink: ${entry.name}")
+ }
+
  if (entry.isDirectory) {
  Files.createDirectories(destFile)
+ if (!destFile.toRealPath().startsWith(realDestDir)) {
+ throw IllegalStateException("Entry escapes the target dir via symlink: ${entry.name}")
+ }
  } else {
+ Files.createDirectories(destFile.parent)
+ if (destFile.parent != lastVerifiedParent) {
+ if (!destFile.parent.toRealPath().startsWith(realDestDir)) {
+ throw IllegalStateException("Entry parent escapes the target dir via symlink: ${entry.name}")
+ }
+ lastVerifiedParent = destFile.parent
+ }
+
  Files.newOutputStream(destFile).use { dest ->
  zipInput.copyTo(dest)
  }

app/src/main/java/com/itsaky/androidide/assets/BundledAssetsInstaller.kt

@@ -29,7 +29,6 @@ import java.io.FileNotFoundException
 import java.io.IOException
 import java.nio.file.Files
 import java.nio.file.Path
-import java.util.zip.ZipInputStream
 import kotlin.io.path.ExperimentalPathApi
 import kotlin.io.path.deleteRecursively
 
@@ -174,27 +173,7 @@ data object BundledAssetsInstaller : BaseAssetsInstaller() {
  val assetPath = ToolsManager.getCommonAsset("$entryName.br")
  assets.open(assetPath).use { assetStream ->
  BrotliInputStream(assetStream).use { brotliStream ->
- ZipInputStream(brotliStream).use { pluginZip ->
- var pluginEntry = pluginZip.nextEntry
- while (pluginEntry != null) {
- if (!pluginEntry.isDirectory) {
- val targetPath = pluginDirPath.resolve(pluginEntry.name).normalize()
- // Security check: prevent path traversal attacks
- if (!targetPath.startsWith(pluginDirPath)) {
- throw IllegalStateException(
- "Zip entry '${pluginEntry.name}' would escape target directory",
- )
- }
- val targetFile = targetPath.toFile()
- targetFile.parentFile?.mkdirs()
- logger.debug("Extracting '{}' to {}", pluginEntry.name, targetFile)
- targetFile.outputStream().use { output ->
- pluginZip.copyTo(output)
- }
- }
- pluginEntry = pluginZip.nextEntry
- }
- }
+ AssetsInstallationHelper.extractZipToDir(brotliStream, pluginDirPath)
  }
  }
  logger.debug("Completed extracting plugin artifacts")

app/src/main/java/com/itsaky/androidide/assets/SplitAssetsInstaller.kt

@@ -23,7 +23,6 @@ import java.io.FileNotFoundException
 import java.nio.file.Files
 import java.nio.file.Path
 import java.util.zip.ZipFile
-import java.util.zip.ZipInputStream
 import kotlin.io.path.ExperimentalPathApi
 import kotlin.io.path.deleteRecursively
 import kotlin.system.measureTimeMillis
@@ -167,27 +166,7 @@ data object SplitAssetsInstaller : BaseAssetsInstaller() {
  }
  Files.createDirectories(pluginDirPath)
 
- ZipInputStream(zipInput).use { pluginZip ->
- var pluginEntry = pluginZip.nextEntry
- while (pluginEntry != null) {
- if (!pluginEntry.isDirectory) {
- val targetPath = pluginDirPath.resolve(pluginEntry.name).normalize()
- // Security check: prevent path traversal attacks
- if (!targetPath.startsWith(pluginDirPath)) {
- throw IllegalStateException(
- "Zip entry '${pluginEntry.name}' would escape target directory",
- )
- }
- val targetFile = targetPath.toFile()
- targetFile.parentFile?.mkdirs()
- logger.debug("Extracting '{}' to {}", pluginEntry.name, targetFile)
- targetFile.outputStream().use { output ->
- pluginZip.copyTo(output)
- }
- }
- pluginEntry = pluginZip.nextEntry
- }
- }
+ AssetsInstallationHelper.extractZipToDir(zipInput, pluginDirPath)
  logger.debug("Completed extracting plugin artifacts")
  }
 

app/src/test/java/com/itsaky/androidide/assets/AssetsInstallationHelperTest.kt

@@ -7,11 +7,23 @@ import io.mockk.every
 import io.mockk.mockk
 import io.mockk.mockkObject
 import kotlinx.coroutines.runBlocking
+import org.junit.Assert.assertEquals
 import org.junit.Assert.assertFalse
+import org.junit.Assert.assertThrows
 import org.junit.Assert.assertTrue
 import org.junit.Before
 import org.junit.Test
+import java.io.ByteArrayInputStream
+import java.io.ByteArrayOutputStream
 import java.io.FileNotFoundException
+import java.io.IOException
+import java.nio.file.FileVisitResult
+import java.nio.file.Files
+import java.nio.file.Path
+import java.nio.file.SimpleFileVisitor
+import java.nio.file.attribute.BasicFileAttributes
+import java.util.zip.ZipEntry
+import java.util.zip.ZipOutputStream
 
 class AssetsInstallationHelperTest {
 	private val ctx: Context = mockk(relaxed = true)
@@ -48,4 +60,94 @@ class AssetsInstallationHelperTest {
  (failure.cause?.cause) is FileNotFoundException,
  )
  }
+
+	@Test
+	fun `extractZipToDir creates parent directories for nested entries with no directory entries`() {
+ val destDir = Files.createTempDirectory("extract-zip-to-dir-test")
+ try {
+ val content = "test notice content"
+ val zipBytes =
+ ByteArrayOutputStream().use { baos ->
+ ZipOutputStream(baos).use { zos ->
+ // No directory entries, matching how android-sdk.zip is packaged.
+ zos.putNextEntry(ZipEntry("build-tools/35.0.0/NOTICE.txt"))
+ zos.write(content.toByteArray())
+ zos.closeEntry()
+ }
+ baos.toByteArray()
+ }
+
+ AssetsInstallationHelper.extractZipToDir(ByteArrayInputStream(zipBytes), destDir)
+
+ val extracted = destDir.resolve("build-tools/35.0.0/NOTICE.txt")
+ assertTrue("Expected extracted file to exist", Files.exists(extracted))
+ assertEquals(content, String(Files.readAllBytes(extracted)))
+ } finally {
+ destDir.toFile().deleteRecursively()
+ }
+	}
+
+	@Test
+	fun `extractZipToDir rejects a file entry whose pre-existing symlinked grandparent escapes destDir`() {
+ val destDir = Files.createTempDirectory("extract-zip-to-dir-test")
+ val outsideDir = Files.createTempDirectory("extract-zip-to-dir-outside")
+ try {
+ Files.createSymbolicLink(destDir.resolve("linked"), outsideDir)
+
+ val content = "escaping content"
+ val zipBytes =
+ ByteArrayOutputStream().use { baos ->
+ ZipOutputStream(baos).use { zos ->
+ // Two levels below the symlink ("linked/sub/nested.txt", no directory
+ // entries), not one: for a one-level entry ("linked/nested.txt"),
+ // destFile.parent IS the symlink, so Files.createDirectories() throws
+ // FileAlreadyExistsException (NOFOLLOW_LINKS rejects the existing
+ // symlink-to-dir) before the toRealPath() guard below it ever runs. One
+ // level deeper, createDirectories() silently traverses the symlink to
+ // create "sub" for real inside outsideDir, and only then does the
+ // toRealPath() check on destFile.parent fire -- which is what this test
+ // exercises.
+ zos.putNextEntry(ZipEntry("linked/sub/nested.txt"))
+ zos.write(content.toByteArray())
+ zos.closeEntry()
+ }
+ baos.toByteArray()
+ }
+
+ assertThrows(IllegalStateException::class.java) {
+ AssetsInstallationHelper.extractZipToDir(ByteArrayInputStream(zipBytes), destDir)
+ }
+ } finally {
+ outsideDir.deleteRecursivelyWithoutFollowingLinks()
+ destDir.deleteRecursivelyWithoutFollowingLinks()
+ }
+	}
+
+	// Deletes a directory tree without following symlinks it contains, unlike
+	// File.deleteRecursively(). Files.walkFileTree() doesn't follow symlinks unless
+	// FileVisitOption.FOLLOW_LINKS is passed (it isn't here), so a symlink is visited
+	// as a leaf via visitFile() -- deleting it unlinks the link itself, never the
+	// target it points to. Needed because the symlink test above symlinks out of destDir.
+	private fun Path.deleteRecursivelyWithoutFollowingLinks() {
+ Files.walkFileTree(
+ this,
+ object : SimpleFileVisitor<Path>() {
+ override fun visitFile(
+ file: Path,
+ attrs: BasicFileAttributes,
+ ): FileVisitResult {
+ Files.delete(file)
+ return FileVisitResult.CONTINUE
+ }
+
+ override fun postVisitDirectory(
+ dir: Path,
+ exc: IOException?,
+ ): FileVisitResult {
+ Files.delete(dir)
+ return FileVisitResult.CONTINUE
+ }
+ },
+ )
+	}
 }