Skip to content

Commit 3c714ca

Browse files
JLLeitschuhTeamModerne
JLLeitschuh
and
TeamModerne
committed
vuln-fix: Zip Slip Vulnerability
This fixes a Zip-Slip vulnerability. This change does one of two things. This change either 1. Inserts a guard to protect against Zip Slip. OR 2. Replaces `dir.getCanonicalPath().startsWith(parent.getCanonicalPath())`, which is vulnerable to partial path traversal attacks, with the more secure `dir.getCanonicalFile().toPath().startsWith(parent.getCanonicalFile().toPath())`. For number 2, consider `"/usr/outnot".startsWith("/usr/out")`. The check is bypassed although `/outnot` is not under the `/out` directory. It's important to understand that the terminating slash may be removed when using various `String` representations of the `File` object. For example, on Linux, `println(new File("/var"))` will print `/var`, but `println(new File("/var", "/")` will print `/var/`; however, `println(new File("/var", "/").getCanonicalPath())` will print `/var`. Weakness: CWE-22: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') Severity: High CVSSS: 7.4 Detection: CodeQL (https://codeql.github.com/codeql-query-help/java/java-zipslip/) & OpenRewrite (https://public.moderne.io/recipes/org.openrewrite.java.security.ZipSlip) Reported-by: Jonathan Leitschuh <[email protected]> Signed-off-by: Jonathan Leitschuh <[email protected]> Bug-tracker: JLLeitschuh/security-research#16 Co-authored-by: Moderne <[email protected]>
1 parent b62590b commit 3c714ca

File tree

2 files changed

+329
-321
lines changed

2 files changed

+329
-321
lines changed

‎ZipExtractor-Core/src/main/java/com/dscalzi/zipextractor/core/provider/JarProvider.java

Lines changed: 143 additions & 139 deletions
Original file line numberDiff line numberDiff line change
@@ -17,142 +17,146 @@
1717
*/
1818

1919
package com.dscalzi.zipextractor.core.provider;
20-
21-
import com.dscalzi.zipextractor.core.TaskInterruptedException;
22-
import com.dscalzi.zipextractor.core.ZTask;
23-
import com.dscalzi.zipextractor.core.managers.MessageManager;
24-
import com.dscalzi.zipextractor.core.util.ICommandSender;
25-
26-
import java.io.File;
27-
import java.io.FileInputStream;
28-
import java.io.FileOutputStream;
29-
import java.io.IOException;
30-
import java.nio.file.AccessDeniedException;
31-
import java.util.ArrayList;
32-
import java.util.Collections;
33-
import java.util.List;
34-
import java.util.jar.JarEntry;
35-
import java.util.jar.JarInputStream;
36-
import java.util.regex.Pattern;
37-
import java.util.zip.ZipException;
38-
39-
public class JarProvider implements TypeProvider {
40-
41-
// Shared pattern by JarProviders
42-
public static final Pattern PATH_END = Pattern.compile("\\.jar$");
43-
protected static final List<String> SUPPORTED = new ArrayList<>(Collections.singletonList("jar"));
44-
45-
@Override
46-
public List<String> scanForExtractionConflicts(ICommandSender sender, File src, File dest, boolean silent) {
47-
48-
List<String> existing = new ArrayList<>();
49-
final MessageManager mm = MessageManager.inst();
50-
if(!silent)
51-
mm.scanningForConflics(sender);
52-
try (FileInputStream fis = new FileInputStream(src); JarInputStream jis = new JarInputStream(fis);) {
53-
JarEntry je = jis.getNextJarEntry();
54-
55-
while (je != null) {
56-
if (Thread.interrupted())
57-
throw new TaskInterruptedException();
58-
59-
File newFile = new File(dest + File.separator + je.getName());
60-
if (newFile.exists()) {
61-
existing.add(je.getName());
62-
}
63-
je = jis.getNextJarEntry();
64-
}
65-
66-
jis.closeEntry();
67-
} catch (TaskInterruptedException e) {
68-
mm.taskInterruption(sender, ZTask.EXTRACT);
69-
} catch (IOException e) {
70-
e.printStackTrace();
71-
}
72-
73-
return existing;
74-
}
75-
76-
@Override
77-
public boolean canDetectPipedConflicts() {
78-
return false;
79-
}
80-
81-
@Override
82-
public boolean extract(ICommandSender sender, File src, File dest, boolean log, boolean pipe) {
83-
final MessageManager mm = MessageManager.inst();
84-
byte[] buffer = new byte[1024];
85-
mm.startingProcess(sender, ZTask.EXTRACT, src.getName());
86-
try (FileInputStream fis = new FileInputStream(src); JarInputStream jis = new JarInputStream(fis);) {
87-
JarEntry je = jis.getNextJarEntry();
88-
89-
while(je != null) {
90-
if (Thread.interrupted())
91-
throw new TaskInterruptedException();
92-
93-
File newFile = new File(dest + File.separator + je.getName());
94-
if (log)
95-
mm.info("Extracting : " + newFile.getAbsoluteFile());
96-
File parent = newFile.getParentFile();
97-
if (!parent.exists() && !parent.mkdirs()) {
98-
throw new IllegalStateException("Couldn't create dir: " + parent);
99-
}
100-
if (je.isDirectory()) {
101-
newFile.mkdir();
102-
je = jis.getNextJarEntry();
103-
continue;
104-
}
105-
try (FileOutputStream fos = new FileOutputStream(newFile)) {
106-
int len;
107-
while ((len = jis.read(buffer)) > 0) {
108-
fos.write(buffer, 0, len);
109-
}
110-
}
111-
je = jis.getNextJarEntry();
112-
}
113-
jis.closeEntry();
114-
if(!pipe)
115-
mm.extractionComplete(sender, dest);
116-
return true;
117-
} catch (AccessDeniedException e) {
118-
mm.fileAccessDenied(sender, ZTask.EXTRACT, e.getMessage());
119-
return false;
120-
} catch (ZipException e) {
121-
mm.extractionFormatError(sender, src, "Jar");
122-
return false;
123-
} catch (TaskInterruptedException e) {
124-
mm.taskInterruption(sender, ZTask.EXTRACT);
125-
return false;
126-
} catch (IOException e) {
127-
e.printStackTrace();
128-
mm.genericOperationError(sender, src, ZTask.EXTRACT);
129-
return false;
130-
}
131-
}
132-
133-
@Override
134-
public boolean validForExtraction(File src) {
135-
return PATH_END.matcher(src.getAbsolutePath()).find();
136-
}
137-
138-
@Override
139-
public boolean srcValidForCompression(File src) {
140-
return false; // Compression to Jars is not supported.
141-
}
142-
143-
@Override
144-
public boolean destValidForCompression(File dest) {
145-
return false; // Compression to Jars is not supported.
146-
}
147-
148-
@Override
149-
public List<String> supportedExtractionTypes() {
150-
return SUPPORTED;
151-
}
152-
153-
@Override
154-
public List<String> canCompressTo() {
155-
return new ArrayList<>();
156-
}
157-
158-
}
20+
21+
import com.dscalzi.zipextractor.core.TaskInterruptedException;
22+
import com.dscalzi.zipextractor.core.ZTask;
23+
import com.dscalzi.zipextractor.core.managers.MessageManager;
24+
import com.dscalzi.zipextractor.core.util.ICommandSender;
25+
26+
import java.io.File;
27+
import java.io.FileInputStream;
28+
import java.io.FileOutputStream;
29+
import java.io.IOException;
30+
import java.nio.file.AccessDeniedException;
31+
import java.util.ArrayList;
32+
import java.util.Collections;
33+
import java.util.List;
34+
import java.util.jar.JarEntry;
35+
import java.util.jar.JarInputStream;
36+
import java.util.regex.Pattern;
37+
import java.util.zip.ZipException;
38+
39+
public class JarProvider implements TypeProvider {
40+
41+
// Shared pattern by JarProviders
42+
public static final Pattern PATH_END = Pattern.compile("\\.jar$");
43+
protected static final List<String> SUPPORTED = new ArrayList<>(Collections.singletonList("jar"));
44+
45+
@Override
46+
public List<String> scanForExtractionConflicts(ICommandSender sender, File src, File dest, boolean silent) {
47+
48+
List<String> existing = new ArrayList<>();
49+
final MessageManager mm = MessageManager.inst();
50+
if(!silent)
51+
mm.scanningForConflics(sender);
52+
try (FileInputStream fis = new FileInputStream(src); JarInputStream jis = new JarInputStream(fis);) {
53+
JarEntry je = jis.getNextJarEntry();
54+
55+
while (je != null) {
56+
if (Thread.interrupted())
57+
throw new TaskInterruptedException();
58+
59+
File newFile = new File(dest + File.separator + je.getName());
60+
if (newFile.exists()) {
61+
existing.add(je.getName());
62+
}
63+
je = jis.getNextJarEntry();
64+
}
65+
66+
jis.closeEntry();
67+
} catch (TaskInterruptedException e) {
68+
mm.taskInterruption(sender, ZTask.EXTRACT);
69+
} catch (IOException e) {
70+
e.printStackTrace();
71+
}
72+
73+
return existing;
74+
}
75+
76+
@Override
77+
public boolean canDetectPipedConflicts() {
78+
return false;
79+
}
80+
81+
@Override
82+
public boolean extract(ICommandSender sender, File src, File dest, boolean log, boolean pipe) {
83+
final MessageManager mm = MessageManager.inst();
84+
byte[] buffer = new byte[1024];
85+
mm.startingProcess(sender, ZTask.EXTRACT, src.getName());
86+
try (FileInputStream fis = new FileInputStream(src); JarInputStream jis = new JarInputStream(fis);) {
87+
JarEntry je = jis.getNextJarEntry();
88+
89+
while(je != null) {
90+
if (Thread.interrupted())
91+
throw new TaskInterruptedException();
92+
93+
File newFile = new File(dest, je.getName());
94+
95+
if (!newFile.toPath().normalize().startsWith(dest.toPath())) {
96+
throw new RuntimeException("Bad zip entry");
97+
}
98+
if (log)
99+
mm.info("Extracting : " + newFile.getAbsoluteFile());
100+
File parent = newFile.getParentFile();
101+
if (!parent.exists() && !parent.mkdirs()) {
102+
throw new IllegalStateException("Couldn't create dir: " + parent);
103+
}
104+
if (je.isDirectory()) {
105+
newFile.mkdir();
106+
je = jis.getNextJarEntry();
107+
continue;
108+
}
109+
try (FileOutputStream fos = new FileOutputStream(newFile)) {
110+
int len;
111+
while ((len = jis.read(buffer)) > 0) {
112+
fos.write(buffer, 0, len);
113+
}
114+
}
115+
je = jis.getNextJarEntry();
116+
}
117+
jis.closeEntry();
118+
if(!pipe)
119+
mm.extractionComplete(sender, dest);
120+
return true;
121+
} catch (AccessDeniedException e) {
122+
mm.fileAccessDenied(sender, ZTask.EXTRACT, e.getMessage());
123+
return false;
124+
} catch (ZipException e) {
125+
mm.extractionFormatError(sender, src, "Jar");
126+
return false;
127+
} catch (TaskInterruptedException e) {
128+
mm.taskInterruption(sender, ZTask.EXTRACT);
129+
return false;
130+
} catch (IOException e) {
131+
e.printStackTrace();
132+
mm.genericOperationError(sender, src, ZTask.EXTRACT);
133+
return false;
134+
}
135+
}
136+
137+
@Override
138+
public boolean validForExtraction(File src) {
139+
return PATH_END.matcher(src.getAbsolutePath()).find();
140+
}
141+
142+
@Override
143+
public boolean srcValidForCompression(File src) {
144+
return false; // Compression to Jars is not supported.
145+
}
146+
147+
@Override
148+
public boolean destValidForCompression(File dest) {
149+
return false; // Compression to Jars is not supported.
150+
}
151+
152+
@Override
153+
public List<String> supportedExtractionTypes() {
154+
return SUPPORTED;
155+
}
156+
157+
@Override
158+
public List<String> canCompressTo() {
159+
return new ArrayList<>();
160+
}
161+
162+
}

0 commit comments

Comments
 (0)