tomasilluminati commented on code in PR #776:
URL: https://github.com/apache/commons-compress/pull/776#discussion_r3572149521
##########
src/main/java/org/apache/commons/compress/archivers/extractor/Extractor.java:
##########
@@ -217,11 +217,27 @@ final void setBeforeLeafWrite(final Runnable hook) {
}
/**
- * Resolves {@code name} against the extraction root and rejects any
result that escapes it (the lexical zip-slip guard).
+ * Tests whether {@code path} is contained within the canonical extraction
root, comparing component by component so a
+ * sibling that merely shares a name prefix (for example {@code root-old}
beside {@code root}) is not treated as contained.
+ */
+ private boolean isWithinRoot(final Path path) {
+ return path.startsWith(rootDirectory);
+ }
+
+ /**
+ * Resolves {@code name} against the extraction root and applies the
lexical zip-slip guard.
+ *
+ * @return the resolved path within the root, or {@code null} if {@code
name} resolves to the root itself (for example
+ * {@code a/..}), which carries nothing to materialize; the caller
skips such entries rather than writing at or
+ * replacing the root.
+ * @throws ArchiveException if the resolved path escapes the extraction
root.
*/
private Path resolveWithinRoot(final String name) throws ArchiveException {
final Path resolved = rootDirectory.resolve(name).normalize();
- if (!resolved.startsWith(rootDirectory)) {
+ if (resolved.equals(rootDirectory)) {
Review Comment:
I traced it, on Windows a name of only dots or spaces (like "...") aliases
the root and the lexical equals here does not see it, but it stays contained,
the entry never resolves outside the root, and the no-follow CREATE_NEW write
hits the existing root directory and throws rather than replacing it, so it
fails closed. I documented it as a known corner case instead of a per-entry
Files.isSameFile, which would be two extra syscalls on every entry to guard
something already fail-closed and not an escape. If you'd rather have these
rejected outright, a cheap lexical reject of pure dot/space names (no I/O)
would do it, happy to add that if you prefer it in this PR. And please link
your OpenJDK issue here when it is filed, I'll reference it. @garydgregory
@ppkarwasz
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
