Add unzip util method #906
Conversation
chanseokoh
force-pushed
the
i431-add-ZipUtil
branch
from
6d2242a
to
2f22336
Compare
jib-core/src/main/java/com/google/cloud/tools/jib/zip/ZipUtil.java
Outdated
Show resolved
Hide resolved
jib-core/src/main/java/com/google/cloud/tools/jib/zip/ZipUtil.java
Outdated
Show resolved
Hide resolved
| try (ZipInputStream zipIn = new ZipInputStream(Files.newInputStream(archive))) { | ||
|
|
||
| for (ZipEntry entry = zipIn.getNextEntry(); entry != null; entry = zipIn.getNextEntry()) { | ||
| Path entryPath = destination.resolve(entry.getName()); |
There was a problem hiding this comment.
Could entry.getName() possibly refer to paths preceding destination?
There was a problem hiding this comment.
That's the Zip-Slip vulnerability I mentioned above. I can address that in a later PR.
jib-core/src/main/java/com/google/cloud/tools/jib/zip/ZipUtil.java
Outdated
Show resolved
Hide resolved
jib-core/src/main/java/com/google/cloud/tools/jib/zip/ZipUtil.java
Outdated
Show resolved
Hide resolved
|
|
||
| for (ZipEntry entry = zipIn.getNextEntry(); entry != null; entry = zipIn.getNextEntry()) { | ||
| // TODO: check Zip-Slip vulnerability: https://snyk.io/research/zip-slip-vulnerability#java | ||
| Path entryPath = destination.resolve(entry.getName()); | ||
|
|
||
| if (entry.isDirectory()) { | ||
| Files.createDirectories(entryPath); | ||
| } else { | ||
| try (OutputStream out = Files.newOutputStream(entryPath)) { |
There was a problem hiding this comment.
And BufferedOutputStream here too?
There was a problem hiding this comment.
Looking at the code of ByteStreams.copy, it does use a buffer in a very obvious way, so I'm inclined to leave the code simple. You could still argue that's an implementation detail of ByteStreams though.
|
Zip-Slip fix merged into this PR. |
|
|
||
| String canonicalTarget = entryPath.toFile().getCanonicalPath(); | ||
| if (!canonicalTarget.startsWith(canonicalDestination + File.separator)) { | ||
| throw new IOException("Blocked unzipping files outside destination: " + entry.getName()); |
There was a problem hiding this comment.
Could we include the filename too?
There was a problem hiding this comment.
It's printing entry.getName() which is the file path in the zip. If this error happens, it would look like ../../../evil.attack.
For use in #431.