selmf / unarr

A decompression library for rar, tar, zip and 7z archives
GNU Lesser General Public License v3.0
73 stars 13 forks source link

Path traversal vulnerability #22

Open gen2brain opened 2 years ago

gen2brain commented 2 years ago

I maintain Golang bindings for unarr at https://github.com/gen2brain/go-unarr and there is this issue reported with tar archives https://github.com/gen2brain/go-unarr/issues/21, along with this CVE https://github.com/advisories/GHSA-v9j4-cp63-qv62. Unfortunately, I don't have a sample of such an archive or a method to create such an archive. I see there are such samples at https://github.com/jwilk/traversal-archives but not sure if these can be used to reproduce the issue.

selmf commented 2 years ago

Hi and thank you for the notice. You don't really need a prepared sample to test this issue. The problem is that unarr extracts the paths as they are recorded in the archives, so if somebody crafts an archive containing a path which leads to directory traversal and you pass this on to the filesystem, bad things can happen.

The fix for this is pretty straightforward. You need to check if the paths are valid before you write to them. If they are not, you either need to sanitize them or you mark the file to as corrupt (the path is, after all, non-spec and the archive was probably handcrafted as an attack) and you refuse to extract them.

I probably should also do something on my side to prevent such issues from ocuring, but this needs a bit of research and consideration so I don't screw things up.

gen2brain commented 2 years ago

Thanks, I pushed a fix here https://github.com/gen2brain/go-unarr/commit/239ec404d348280b50bbf671327709e8857fc5f4, I preferred to sanitize the entry name. It is easy to create an archive for testing:

import sys, tarfile

def main(argv=sys.argv):
        tf = tarfile.open("test.tar", "w")
        tf.add("/etc/protocols", "test/../../../../../../../../../../../tmp/test.txt")
        tf.close()
        return

if __name__ == '__main__':
        main()