diff --git a/src/wormhole/cli/cmd_receive.py b/src/wormhole/cli/cmd_receive.py index 7725305..79a6221 100644 --- a/src/wormhole/cli/cmd_receive.py +++ b/src/wormhole/cli/cmd_receive.py @@ -284,29 +284,30 @@ class TwistedReceiver: self._msg(u"Received file written to %s" % os.path.basename(self.abs_destname)) + def _extract_file(self, zf, info, extract_dir): + """ + the zipfile module does not restore file permissions + so we'll do it manually + """ + out_path = os.path.join( extract_dir, info.filename ) + out_path = os.path.abspath( out_path ) + if not out_path.startswith( extract_dir ): + raise ValueError( "malicious zipfile, %s outside of extract_dir %s" + % (info.filename, extract_dir) ) + + zf.extract( info.filename, path=extract_dir ) + + # not sure why zipfiles store the perms 16 bits away but they do + perm = info.external_attr >> 16 + os.chmod( out_path, perm ) + def _write_directory(self, f): - def extract_file( zf, info, extract_dir ): - """ - the zipfile module does not restore file permissions - so we'll do it manually - """ - out_path = os.path.join( extract_dir, info.filename ) - out_path = os.path.abspath( out_path ) - if not out_path.startswith( extract_dir ): - raise ValueError( "malicious zipfile, %s outside of extract_dir %s" - % (info.filename, extract_dir) ) - - zf.extract( info.filename, path=extract_dir ) - - # not sure why zipfiles store the perms 16 bits away but they do - perm = info.external_attr >> 16 - os.chmod( out_path, perm ) self._msg(u"Unpacking zipfile..") with self.args.timing.add("unpack zip"): with zipfile.ZipFile(f, "r", zipfile.ZIP_DEFLATED) as zf: for info in zf.infolist(): - extract_file( zf, info, self.abs_destname ) + self._extract_file( zf, info, self.abs_destname ) self._msg(u"Received files written to %s/" % os.path.basename(self.abs_destname)) diff --git a/src/wormhole/test/test_scripts.py b/src/wormhole/test/test_scripts.py index 5bf89be..6ccd5f1 100644 --- a/src/wormhole/test/test_scripts.py +++ b/src/wormhole/test/test_scripts.py @@ -1,5 +1,6 @@ from __future__ import print_function import os, sys, re, io, zipfile, six +import mock from twisted.trial import unittest from twisted.python import procutils, log from twisted.internet.utils import getProcessOutputAndValue @@ -613,3 +614,44 @@ class Cleanup(ServerBase, unittest.TestCase): self.assertEqual(len(cids), 0) self.flushLoggedErrors(WrongPasswordError) +class ExtractFile(unittest.TestCase): + def test_filenames(self): + args = mock.Mock() + args.relay_url = u"" + ef = cmd_receive.TwistedReceiver(args)._extract_file + extract_dir = os.path.abspath(self.mktemp()) + + zf = mock.Mock() + zi = mock.Mock() + zi.filename = "ok" + zi.external_attr = 5 << 16 + expected = os.path.join(extract_dir, "ok") + with mock.patch.object(cmd_receive.os, "chmod") as chmod: + ef(zf, zi, extract_dir) + self.assertEqual(zf.extract.mock_calls, + [mock.call(zi.filename, path=extract_dir)]) + self.assertEqual(chmod.mock_calls, [mock.call(expected, 5)]) + + zf = mock.Mock() + zi = mock.Mock() + zi.filename = "../haha" + e = self.assertRaises(ValueError, ef, zf, zi, extract_dir) + self.assertIn("malicious zipfile", str(e)) + + zf = mock.Mock() + zi = mock.Mock() + zi.filename = "haha//root" # abspath squashes this, hopefully zipfile + # does too + zi.external_attr = 5 << 16 + expected = os.path.join(extract_dir, "haha/root") + with mock.patch.object(cmd_receive.os, "chmod") as chmod: + ef(zf, zi, extract_dir) + self.assertEqual(zf.extract.mock_calls, + [mock.call(zi.filename, path=extract_dir)]) + self.assertEqual(chmod.mock_calls, [mock.call(expected, 5)]) + + zf = mock.Mock() + zi = mock.Mock() + zi.filename = "/etc/passwd" + e = self.assertRaises(ValueError, ef, zf, zi, extract_dir) + self.assertIn("malicious zipfile", str(e))