Harden ClawHub ZIP extraction and add security tests

AndyClaw/src/main/java/org/ethereumphone/andyclaw/extensions/clawhub/ClawHubApi.kt

@@ -14,6 +14,7 @@ import okhttp3.Response
 import okhttp3.ResponseBody
 import java.io.File
 import java.io.IOException
+import java.io.OutputStream
 import java.util.concurrent.TimeUnit
 import java.util.logging.Logger
 import java.util.zip.ZipInputStream
@@ -188,33 +189,118 @@ class ClawHubApi(
 
     /**
      * Extract a ZIP response body into [targetDir], writing files relative
-     * to the target. Skips entries outside the target (zip-slip guard).
+     * to the target.
+     *
+     * Security controls:
+     * - path containment check using canonical paths + separator-safe matching
+     * - max entry count
+     * - max single-entry uncompressed bytes
+     * - max total uncompressed bytes (zip-bomb mitigation)
      */
     private fun extractZip(body: ResponseBody, targetDir: File) {
+        val canonicalTargetDir = targetDir.canonicalFile
+        val canonicalTargetPath = canonicalTargetDir.toPath()
+        var entryCount = 0
+        var totalUncompressedBytes = 0L
+
         ZipInputStream(body.byteStream()).use { zis ->
             var entry = zis.nextEntry
             while (entry != null) {
-                val outFile = File(targetDir, entry.name).canonicalFile
+                entryCount++
+                if (entryCount > MAX_ZIP_ENTRIES) {
+                    throw SecurityException("Zip exceeds entry limit ($MAX_ZIP_ENTRIES)")
+                }
+
+                if (entry.name.length > MAX_ZIP_ENTRY_NAME_CHARS) {
+                    throw SecurityException("Zip entry name too long: ${entry.name.take(80)}")
+                }
+
+                if (entry.size > MAX_ZIP_ENTRY_UNCOMPRESSED_BYTES) {
+                    throw SecurityException(
+                        "Zip entry too large by declared size: ${entry.name}"
+                    )
+                }
 
-                // Zip-slip guard: ensure extracted path stays within targetDir
-                if (!outFile.path.startsWith(targetDir.canonicalPath)) {
+                val outFile = File(canonicalTargetDir, entry.name).canonicalFile
+
+                // Zip-slip guard: ensure extracted path stays within target directory.
+                if (!outFile.toPath().startsWith(canonicalTargetPath)) {
                     throw SecurityException("Zip entry escapes target dir: ${entry.name}")
                 }
 
                 if (entry.isDirectory) {
-                    outFile.mkdirs()
+                    if (!outFile.exists() && !outFile.mkdirs()) {
+                        throw IOException("Failed to create directory for zip entry: ${entry.name}")
+                    }
                 } else {
-                    outFile.parentFile?.mkdirs()
-                    outFile.outputStream().use { out ->
-                        zis.copyTo(out)
+                    val parent = outFile.parentFile
+                    if (parent != null && !parent.exists() && !parent.mkdirs()) {
+                        throw IOException("Failed to create parent directory for ${entry.name}")
+                    }
+
+                    val bytesRemaining = MAX_ZIP_TOTAL_UNCOMPRESSED_BYTES - totalUncompressedBytes
+                    if (bytesRemaining <= 0) {
+                        throw SecurityException(
+                            "Zip exceeds total uncompressed size limit ($MAX_ZIP_TOTAL_UNCOMPRESSED_BYTES bytes)"
+                        )
+                    }
+
+                    val bytesWritten = outFile.outputStream().use { out ->
+                        copyZipEntryWithLimits(
+                            zis = zis,
+                            out = out,
+                            entryName = entry.name,
+                            entryLimitBytes = MAX_ZIP_ENTRY_UNCOMPRESSED_BYTES,
+                            totalRemainingBytes = bytesRemaining,
+                        )
+                    }
+                    totalUncompressedBytes += bytesWritten
+
+                    if (totalUncompressedBytes > MAX_ZIP_TOTAL_UNCOMPRESSED_BYTES) {
+                        throw SecurityException(
+                            "Zip exceeds total uncompressed size limit ($MAX_ZIP_TOTAL_UNCOMPRESSED_BYTES bytes)"
+                        )
                     }
                 }
+
                 zis.closeEntry()
                 entry = zis.nextEntry
             }
         }
     }
 
+    private fun copyZipEntryWithLimits(
+        zis: ZipInputStream,
+        out: OutputStream,
+        entryName: String,
+        entryLimitBytes: Long,
+        totalRemainingBytes: Long,
+    ): Long {
+        val buffer = ByteArray(ZIP_COPY_BUFFER_BYTES)
+        var entryBytes = 0L
+
+        while (true) {
+            val read = zis.read(buffer)
+            if (read <= 0) break
+
+            entryBytes += read
+            if (entryBytes > entryLimitBytes) {
+                throw SecurityException(
+                    "Zip entry exceeds uncompressed size limit ($entryLimitBytes bytes): $entryName"
+                )
+            }
+            if (entryBytes > totalRemainingBytes) {
+                throw SecurityException(
+                    "Zip exceeds total uncompressed size limit ($MAX_ZIP_TOTAL_UNCOMPRESSED_BYTES bytes)"
+                )
+            }
+
+            out.write(buffer, 0, read)
+        }
+
+        return entryBytes
+    }
+
     companion object {
         const val DEFAULT_REGISTRY = "https://clawhub.com"
 
@@ -224,6 +310,13 @@ class ClawHubApi(
         private const val V1_RESOLVE = "/api/v1/resolve"
         private const val V1_DOWNLOAD = "/api/v1/download"
 
+        // ZIP hardening caps to mitigate zip-bomb and path abuse.
+        private const val MAX_ZIP_ENTRIES = 2_000
+        private const val MAX_ZIP_ENTRY_NAME_CHARS = 512
+        private const val MAX_ZIP_ENTRY_UNCOMPRESSED_BYTES = 5L * 1024 * 1024
+        private const val MAX_ZIP_TOTAL_UNCOMPRESSED_BYTES = 25L * 1024 * 1024
+        private const val ZIP_COPY_BUFFER_BYTES = 8 * 1024
+
         private fun defaultClient(): OkHttpClient = OkHttpClient.Builder()
             .connectTimeout(15, TimeUnit.SECONDS)
             .readTimeout(30, TimeUnit.SECONDS)

app/build.gradle.kts

@@ -124,6 +124,7 @@ dependencies {
     // ContactsSDK for ethOS contacts with ETH address support
     implementation("com.github.EthereumPhone:ContactsSDK:0.1.0")
     testImplementation(libs.junit)
+    testImplementation("com.squareup.okhttp3:mockwebserver:4.12.0")
     androidTestImplementation(libs.androidx.junit)
     androidTestImplementation(libs.androidx.espresso.core)
     androidTestImplementation(platform(libs.androidx.compose.bom))

app/src/test/java/org/ethereumphone/andyclaw/extensions/clawhub/ClawHubApiSecurityTest.kt

@@ -0,0 +1,134 @@
+package org.ethereumphone.andyclaw.extensions.clawhub
+
+import kotlinx.coroutines.runBlocking
+import okhttp3.mockwebserver.MockResponse
+import okhttp3.mockwebserver.MockWebServer
+import okio.Buffer
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
+import org.junit.Test
+import java.io.ByteArrayOutputStream
+import java.io.File
+import java.nio.file.Files
+import java.util.zip.ZipEntry
+import java.util.zip.ZipOutputStream
+
+class ClawHubApiSecurityTest {
+
+    @Test
+    fun `downloadAndExtract allows safe archives`() {
+        val zip = buildZip(
+            listOf(
+                "SKILL.md" to "# safe".toByteArray(),
+                "scripts/run.sh" to "echo hi".toByteArray(),
+            )
+        )
+
+        withApiServingZip(zip) { api ->
+            withTempDir { dir ->
+                val ok = api.downloadAndExtract(slug = "safe-skill", targetDir = dir)
+                assertTrue(ok)
+                assertTrue(File(dir, "SKILL.md").isFile)
+                assertEquals("# safe", File(dir, "SKILL.md").readText())
+                assertTrue(File(dir, "scripts/run.sh").isFile)
+            }
+        }
+    }
+
+    @Test
+    fun `downloadAndExtract rejects zip slip paths`() {
+        val zip = buildZip(listOf("../evil.txt" to "pwn".toByteArray()))
+
+        withApiServingZip(zip) { api ->
+            withTempDir { dir ->
+                val ok = api.downloadAndExtract(slug = "evil-skill", targetDir = dir)
+                assertFalse(ok)
+                assertFalse(File(dir.parentFile, "evil.txt").exists())
+            }
+        }
+    }
+
+    @Test
+    fun `downloadAndExtract rejects too many entries`() {
+        val entries = (0..2000).map { idx -> "f$idx.txt" to ByteArray(0) }
+        val zip = buildZip(entries)
+
+        withApiServingZip(zip) { api ->
+            withTempDir { dir ->
+                val ok = api.downloadAndExtract(slug = "too-many", targetDir = dir)
+                assertFalse(ok)
+            }
+        }
+    }
+
+    @Test
+    fun `downloadAndExtract rejects oversized uncompressed entry`() {
+        val oversized = ByteArray(5 * 1024 * 1024 + 1) { 'a'.code.toByte() }
+        val zip = buildZip(listOf("big.bin" to oversized))
+
+        withApiServingZip(zip) { api ->
+            withTempDir { dir ->
+                val ok = api.downloadAndExtract(slug = "oversized", targetDir = dir)
+                assertFalse(ok)
+            }
+        }
+    }
+
+    @Test
+    fun `downloadAndExtract rejects total uncompressed size bomb`() {
+        // 7 entries × 4 MB each = 28 MB total, exceeds the 25 MB total limit
+        val chunk = ByteArray(4 * 1024 * 1024) { 'a'.code.toByte() }
+        val entries = (1..7).map { idx -> "chunk$idx.bin" to chunk }
+        val zip = buildZip(entries)
+
+        withApiServingZip(zip) { api ->
+            withTempDir { dir ->
+                val ok = api.downloadAndExtract(slug = "total-bomb", targetDir = dir)
+                assertFalse(ok)
+            }
+        }
+    }
+
+    private fun withApiServingZip(zipBytes: ByteArray, block: suspend (ClawHubApi) -> Unit) {
+        runBlocking {
+            val server = MockWebServer()
+            server.enqueue(
+                MockResponse()
+                    .setResponseCode(200)
+                    .setBody(Buffer().write(zipBytes))
+            )
+            server.start()
+            try {
+                val api = ClawHubApi(registryUrl = server.url("/").toString())
+                block(api)
+            } finally {
+                server.shutdown()
+            }
+        }
+    }
+
+    private fun buildZip(entries: List<Pair<String, ByteArray>>): ByteArray {
+        val out = ByteArrayOutputStream()
+        ZipOutputStream(out).use { zip ->
+            for ((name, content) in entries) {
+                val entry = ZipEntry(name)
+                zip.putNextEntry(entry)
+                if (!name.endsWith("/")) {
+                    zip.write(content)
+                }
+                zip.closeEntry()
+            }
+        }
+        return out.toByteArray()
+    }
+
+    private fun withTempDir(block: (File) -> Unit) {
+        val dir = Files.createTempDirectory("clawhub-api-test-").toFile()
+        try {
+            block(dir)
+        } finally {
+            dir.deleteRecursively()
+        }
+    }
+}