Skip to content

Commit 1d858ac

Browse files
committed
fix(android): validate ZIP entry paths against destination (Zip Slip)
Prior to this commit, HybridUnzipTask.kt's Android extraction path resolved every entry as `File(destDir, entry.name)` and wrote to that path without checking that the resolved file actually sits inside destDir. A malicious ZIP containing an entry named `../../databases/poi.db` would resolve to `destDir/../../databases/poi.db` and FileOutputStream would happily write to it — the classic Zip Slip vulnerability. Fix: introduce a canonical-path validation helper that's called once per entry in pass 1 (header collection) and again per entry in pass 2 (file write) for defence in depth. The check resolves the candidate path under the canonical destDir and rejects anything that doesn't start with the destDir prefix. Absolute paths, `..` traversal, and exotic separators all canonicalise into something the prefix check catches. Same validation applied to the zip4j password-protected path before delegating to zip4j.extractFile() — zip4j 2.10+ has its own check but defence in depth is cheap and shields against older transitive versions. iOS path is unaffected: SSZipArchive ~> 2.5 validates paths since 2.4.0. Bumps to 0.3.1.
1 parent 66a6acf commit 1d858ac

2 files changed

Lines changed: 41 additions & 1 deletion

File tree

android/src/main/java/com/margelo/nitro/unzip/HybridUnzipTask.kt

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,9 @@ class HybridUnzipTask(
7171
if (!destDir.exists()) {
7272
destDir.mkdirs()
7373
}
74+
// Canonicalize the destination once for Zip Slip validation. Any entry
75+
// whose resolved path doesn't start with this prefix is rejected.
76+
val canonicalDestDir = destDir.canonicalFile
7477

7578
val sourceFile = File(cleanZip)
7679
if (!sourceFile.exists()) {
@@ -84,6 +87,10 @@ class HybridUnzipTask(
8487
ZipInputStream(BufferedInputStream(sourceFile.inputStream(), BUFFER_SIZE)).use { zis ->
8588
var entry = zis.nextEntry
8689
while (entry != null) {
90+
// Reject malicious entries before either pass touches the filesystem.
91+
// Zip Slip: an entry named `../../foo` would resolve outside destDir
92+
// and write to arbitrary app-private paths.
93+
assertSafeEntryPath(canonicalDestDir, entry.name)
8794
if (entry.isDirectory) {
8895
directoriesToCreate.add(entry.name)
8996
} else {
@@ -112,6 +119,10 @@ class HybridUnzipTask(
112119

113120
while (entry != null && isActive && !shouldCancel) {
114121
if (!entry.isDirectory) {
122+
// Defence in depth: re-validate per entry in case the archive
123+
// shuffles between passes (shouldn't, but ZIP central directory
124+
// and local headers can disagree on malformed archives).
125+
assertSafeEntryPath(canonicalDestDir, entry.name)
115126
val entryFile = File(destDir, entry.name)
116127

117128
BufferedOutputStream(FileOutputStream(entryFile), BUFFER_SIZE).use { output ->
@@ -185,6 +196,7 @@ class HybridUnzipTask(
185196
if (!destDir.exists()) {
186197
destDir.mkdirs()
187198
}
199+
val canonicalDestDir = destDir.canonicalFile
188200

189201
val sourceFile = File(cleanZip)
190202
if (!sourceFile.exists()) {
@@ -195,6 +207,13 @@ class HybridUnzipTask(
195207
zipFile.setPassword(password!!.toCharArray())
196208

197209
val fileHeaders = zipFile.fileHeaders
210+
// Zip Slip mitigation — validate every entry's resolved path before
211+
// delegating to zip4j. zip4j 2.10+ has its own check but defence in depth
212+
// is cheap, and it shields against older versions if the resolved
213+
// dependency tree slips.
214+
for (header in fileHeaders) {
215+
assertSafeEntryPath(canonicalDestDir, header.fileName)
216+
}
198217
val totalEntries = fileHeaders.size
199218
var extractedCount = 0
200219
var lastProgressUpdate = System.currentTimeMillis()
@@ -251,5 +270,26 @@ class HybridUnzipTask(
251270
companion object {
252271
private const val BUFFER_SIZE = 65536
253272
private const val PROGRESS_THROTTLE_MS = 1000L
273+
274+
/**
275+
* Zip Slip mitigation — reject any entry whose resolved canonical path
276+
* doesn't sit inside the destination directory. Catches the classic
277+
* `../../foo` payload as well as absolute paths (`/etc/passwd`) and
278+
* exotic separators that resolve through `..` after canonicalisation.
279+
*
280+
* Throws SecurityException with the offending entry name so callers can
281+
* fail loudly (and report to crash analytics) when malformed archives
282+
* appear in production.
283+
*/
284+
internal fun assertSafeEntryPath(canonicalDestDir: File, entryName: String) {
285+
val resolved = File(canonicalDestDir, entryName).canonicalFile
286+
val destPrefix = canonicalDestDir.path + File.separator
287+
if (resolved.path != canonicalDestDir.path &&
288+
!resolved.path.startsWith(destPrefix)) {
289+
throw SecurityException(
290+
"Refusing to extract entry outside destination: $entryName (resolves to ${resolved.path})"
291+
)
292+
}
293+
}
254294
}
255295
}

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "react-native-nitro-unzip",
3-
"version": "0.3.0",
3+
"version": "0.3.1",
44
"description": "High-performance ZIP extraction for React Native, powered by Nitro Modules",
55
"main": "lib/commonjs/index.js",
66
"module": "lib/module/index.js",

0 commit comments

Comments
 (0)