[Security] Incomplete fix for CVE-2023-5245: zip slip via directory entries in FileUtil.extract
- Dominant language
- Scala
- Stars
- 1.5k
- Forks
- 315
- PR merge metrics
- No merged PRs in 30d
Description
Reporting this publicly because private vulnerability reporting is not enabled on this repository, and because it concerns an already-published CVE rather than an undisclosed issue. Happy to move it elsewhere if you have a preferred channel.
The path-containment check added for CVE-2023-5245 is applied only to regular file entries. Directory entries skip it, so a bundle containing a directory entry whose name traverses upward causes `Files.createDirectories()` to run against a path outside the destination directory.
## Affected code
`bundle-ml/src/main/scala/ml/combust/bundle/util/FileUtil.scala`, `FileUtil.extract()`. Confirmed at HEAD `88de54d` (2026-07-21).
```scala
def extract(in: ZipInputStream, dest: Path): Unit = {
Files.createDirectories(dest)
var entry = in.getNextEntry
while (entry != null) {
val filePath = dest.resolve(entry.getName)
if (entry.isDirectory) {
Files.createDirectories(filePath) // <-- no check
} else {
val destCanonical = dest.toRealPath()
val entryCanonical = filePath.toAbsolutePath().normalize()
if (!entryCanonical.startsWith(destCanonical.toString() + FileSystems.getDefault().getSeparator())) {
throw new Exception("Entry is outside of the target dir: " + entry.getName)
}
Using(Files.newOutputStream(filePath)) { out => writeData(in, out) }
}
entry = in.getNextEntry
}
}
```
`dest.resolve(entry.getName)` resolves `..` segments as written. The containment check that would reject them sits inside the `else` branch, so it never runs for an entry whose `isDirectory` flag is set.
## Impact
Arbitrary directory creation outside the destination, at any path the JVM process can write to, triggered by loading an untrusted MLeap bundle.
To be precise about the ceiling: the file-writing branch is correctly guarded, so this does not give arbitrary file write, and `createDirectories()` will not overwrite an existing file. The practical impact is filesystem pollution and, on a constrained volume, inode exhaustion. I am reporting it because it is an incomplete fix of a previously assigned CVE rather than because the impact is high, and because a future change that moves file writes before or outside the current check would turn it into arbitrary file write.
## Reproduction
Verified against a verbatim port of `FileUtil.extract()` at `88de54d`, on OpenJDK 21.0.10 (Linux x86_64). The Scala original calls only `java.nio.file` APIs, so the port preserves the behaviour exactly.
```java
// FileUtil.extract() as it stands at 88de54d
static void extract(ZipInputStream in, Path dest) throws Exception {
Files.createDirectories(dest);
ZipEntry entry = in.getNextEntry();
while (entry != null) {
Path filePath = dest.resolve(entry.getName());
if (entry.isDirectory()) {
Files.createDirectories(filePath); // <-- no check
} else {
Path destCanonical = dest.toRealPath();
Path entryCanonical = filePath.toAbsolutePath().normalize();
if (!entryCanonical.startsWith(
destCanonical.toString() + FileSystems.getDefault().getSeparator())) {
throw new Exception("Entry is outside of the target dir: " + entry.getName());
}
try (OutputStream out = Files.newOutputStream(filePath)) { in.transferTo(out); }
}
entry = in.getNextEntry();
}
}
```
Archive built with a traversing **directory** entry (trailing slash sets the directory flag) plus one benign file:
```java
try (ZipOutputStream zos = new ZipOutputStream(Files.newOutputStream(zip))) {
zos.putNextEntry(new ZipEntry("../OUTSIDE/poc/"));
zos.closeEntry();
zos.putNextEntry(new ZipEntry("bundle.json"));
zos.write("{}".getBytes());
zos.closeEntry();
}
```
Output:
```
dest : /tmp/mleap-poc-.../extract-here
target of escape : /tmp/mleap-poc-.../OUTSIDE/poc
exists before : false
extract() : returned without throwing
exists after : true
>>> ESCAPED: directory created outside the destination
control (file entry, same traversal):
extract() threw: Entry is outside of the target dir: ../OUTSIDE/poc2
poc2 exists : false
```
The control case is the important half: the identical path submitted as a file entry is correctly rejected. Only the directory branch escapes.
## Suggested fix
Hoist the containment check above the branch so it covers both cases:
```scala
val filePath = dest.resolve(entry.getName)
val destCanonical = dest.toRealPath()
val entryCanonical = filePath.toAbsolutePath().normalize()
if (!entryCanonical.startsWith(destCanonical.toString() + FileSystems.getDefault().getSeparator())) {
throw new Exception("Entry is outside of the target dir: " + entry.getName)
}
if (entry.isDirectory) Files.createDirectories(filePath)
else Using(Files.newOutputStream(filePath)) { out => writeData(in, out) }
```
If you'd prefer, I can open the PR directly and you can review the fix alongside this report.
Contributor guide
No contributing guide indexed for this repository
Research direction
Read bundle-ml/src/main/scala/ml/combust/bundle/util/FileUtil.scala, focusing on FileUtil.extract(), then run the supplied directory-entry reproduction. Done means traversing directory entries is rejected without creating paths outside the destination, while the benign file entry still extracts successfully.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- scala
- Domain
- security
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100