Skip to content

SECURITY LEAK: library will write to any path you want it to. #1

@rzwitserloot

Description

@rzwitserloot

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).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions