bitstorm / tiny-zip Goto Github PK

If one zips a folder and the path of the zip file is in the folder you want to zip a corrupted zip file will be created in the intetional created zip file.

WARNING: This code has a security leak in it.

ZIPs can contain relative paths. Specifically, you can craft them such that they contain ../../../../../../../etc/passwd for example, as path. It can even use absolute paths and have /etc/passwd as path. Your tool calls destDirPath.resolve(pathTakenStraightFromZipFileWithoutCheckingIt) which means it will write to where-ever the zip file wants it to write to. somePath.resolve(Paths.get("/etc/passwd")) returns a Path object representing /etc/passwd (try it!). This can lead to nasty surprises in the best case, and to security leaks in the worst. Yes, you rarely meet zip files that do this, but then, I assume you rarely come into contact with zip files that are explicitly crafted to hack your system. I'm pretty sure java's own zip library can make these hacky zips with absolute paths / relative paths with double-dots in em, if you need some for testing purposes.

Suggested fix is to check if the resulting path (the return value of the resolve call) is still a sub-path of your target using filePath.startsWith(destDirPath) and if the answer is 'no', to either just abort (and state that you do not support unpacking zips with absolute paths / escape-target-via-double-dots paths, so: throw new IOException("ZIP files with absolute paths (or relative paths with suffient '..' paths inside) are not supported by tinyzip");), or, alternatively, just get the last bit of the path (filePath.getFileName()), and throw that at destDirPath.resolve. Thus, if I want to unpack a zip into /tmp/unzipHere, and I have a zip with /etc/passwd in it, I end up with /tmp/unzipHere/passwd which seems fair enough.

It is possible that a zip contains the same filepath twice (and it becomes easier if you treat /etc/passwd as just passwd to have this occur), but I doubt you need to take any special action there. If you attempt to use this lib to unzip a zip with a duplicated entry in it, the last time that entry shows up, is the file you get, and all earlier entries will not be there (they will have been unzipped, but will then have been overwritten by the later entry).

When using the constructor:

ZipParameters zp = new ZipParameters(false, (percent, currentFile) -> { System.out.println(String.format("%f, done %s.", percent, currentFile)); });

and then calling

TinyZip.zip(zipDestPath, zp, srcFilePath);

The resulting zip file is empty. But if I use the constructor that only takes a BiConsumer, e.x:

ZipParameters zp = new ZipParameters((percent, currentFile) -> { System.out.println(String.format("%f, done %s.", percent, currentFile)); });

That works just fine.