8000 [3.12] gh-114959: tarfile: do not ignore errors when extract a direct… · python/cpython@8ed20bc · GitHub
[go: up one dir, main page]
Skip to content

Commit 8ed20bc

Browse files
miss-islingtonserhiy-storchaka
miss-islington
and
serhiy-storchaka
authored
[3.12] gh-114959: tarfile: do not ignore errors when extract a directory on top of a file (GH-114960) (GH-114963)
Also, add tests common to tarfile and zipfile. (cherry picked from commit 96bce03) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
1 parent c86a9e6 commit 8ed20bc

File tree

5 files changed

+220
-1
lines changed

5 files changed

+220
-1
lines changed

‎Lib/tarfile.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2449,7 +2449,8 @@ def makedir(self, tarinfo, targetpath):
24492449
# later in _extract_member().
24502450
os.mkdir(targetpath, 0o700)
24512451
except FileExistsError:
2452-
pass
2452+
if not os.path.isdir(targetpath):
2453+
raise
24532454

24542455
def makefile(self, tarinfo, targetpath):
24552456
"""Make a file called targetpath.

‎Lib/test/archiver_tests.py

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
"""Tests common to tarfile and zipfile."""
2+
3+
import os
4+
import sys
5+
6+
from test.support import os_helper
7+
8+
class OverwriteTests:
9+
10+
def setUp(self):
11+
os.makedirs(self.testdir)
12+
self.addCleanup(os_helper.rmtree, self.testdir)
13+
14+
def create_file(self, path, content=b''):
15+
with open(path, 'wb') as f:
16+
f.write(content)
17+
18+
def open(self, path):
19+
raise NotImplementedError
20+
21+
def extractall(self, ar):
22+
raise NotImplementedError
23+
24+
25+
def test_overwrite_file_as_file(self):
26+
target = os.path.join(self.testdir, 'test')
27+
self.create_file(target, b'content')
28+
with self.open(self.ar_with_file) as ar:
29+
self.extractall(ar)
30+
self.assertTrue(os.path.isfile(target))
31+
with open(target, 'rb') as f:
32+
self.assertEqual(f.read(), b'newcontent')
33+
34+
def test_overwrite_dir_as_dir(self):
35+
target = os.path.join(self.testdir, 'test')
36+
os.mkdir(target)
37+
with self.open(self.ar_with_dir) as ar:
38+
self.extractall(ar)
39+
self.assertTrue(os.path.isdir(target))
40+
41+
def test_overwrite_dir_as_implicit_dir(self):
42+
target = os.path.join(self.testdir, 'test')
43+
os.mkdir(target)
44+
with self.open(self.ar_with_implicit_dir) as ar:
45+
self.extractall(ar)
46+
self.assertTrue(os.path.isdir(target))
47+
self.assertTrue(os.path.isfile(os.path.join(target, 'file')))
48+
with open(os.path.join(target, 'file'), 'rb') as f:
49+
self.assertEqual(f.read(), b'newcontent')
50+
51+
def test_overwrite_dir_as_file(self):
52+
target = os.path.join(self.testdir, 'test')
53+
os.mkdir(target)
54+
with self.open(self.ar_with_file) as ar:
55+
with self.assertRaises(PermissionError if sys.platform == 'win32'
56+
else IsADirectoryError):
57+
self.extractall(ar)
58+
self.assertTrue(os.path.isdir(target))
59+
60+
def test_overwrite_file_as_dir(self):
61+
target = os.path.join(self.testdir, 'test')
62+
self.create_file(target, b'content')
63+
with self.open(self.ar_with_dir) as ar:
64+
with self.assertRaises(FileExistsError):
65+
self.extractall(ar)
66+
self.assertTrue(os.path.isfile(target))
67+
with open(target, 'rb') as f:
68+
self.assertEqual(f.read(), b'content')
69+
70+
def test_overwrite_file_as_implicit_dir(self):
71+
target = os.path.join(self.testdir, 'test')
72+
self.create_file(target, b'content')
73+
with self.open(self.ar_with_implicit_dir) as ar:
74+
with self.assertRaises(FileNotFoundError if sys.platform == 'win32'
75+
else NotADirectoryError):
76+
self.extractall(ar)
77+
self.assertTrue(os.path.isfile(target))
78+
with open(target, 'rb') as f:
79+
self.assertEqual(f.read(), b'content')
80+
81+
@os_helper.skip_unless_symlink
82+
def test_overwrite_file_symlink_as_file(self):
83+
# XXX: It is potential security vulnerability.
84+
target = os.path.join(self.testdir, 'test')
85+
target2 = os.path.join(self.testdir, 'test2')
86+
self.create_file(target2, b'content')
87+
os.symlink('test2', target)
88+
with self.open(self.ar_with_file) as ar:
89+
self.extractall(ar)
90+
self.assertTrue(os.path.islink(target))
91+
self.assertTrue(os.path.isfile(target2))
92+
with open(target2, 'rb') as f:
93+
self.assertEqual(f.read(), b'newcontent')
94+
95+
@os_helper.skip_unless_symlink
96+
def test_overwrite_broken_file_symlink_as_file(self):
97+
# XXX: It is potential security vulnerability.
98+
target = os.path.join(self.testdir, 'test')
99+
target2 = os.path.join(self.testdir, 'test2')
100+
os.symlink('test2', target)
101+
with self.open(self.ar_with_file) as ar:
102+
self.extractall(ar)
103+
self.assertTrue(os.path.islink(target))
104+
self.assertTrue(os.path.isfile(target2))
105+
with open(target2, 'rb') as f:
106+
self.assertEqual(f.read(), b'newcontent')
107+
108+
@os_helper.skip_unless_symlink
109+
def test_overwrite_dir_symlink_as_dir(self):
110+
# XXX: It is potential security vulnerability.
111+
target = os.path.join(self.testdir, 'test')
112+
target2 = os.path.join(self.testdir, 'test2')
113+
os.mkdir(target2)
114+
os.symlink('test2', target, target_is_directory=True)
115+
with self.open(self.ar_with_dir) as ar:
116+
self.extractall(ar)
117+
self.assertTrue(os.path.islink(target))
118+
self.assertTrue(os.path.isdir(target2))
119+
120+
@os_helper.skip_unless_symlink
121+
def test_overwrite_dir_symlink_as_implicit_dir(self):
122+
# XXX: It is potential security vulnerability.
123+
target = os.path.join(self.testdir, 'test')
124+
target2 = os.path.join(self.testdir, 'test2')
125+
os.mkdir(target2)
126+
os.symlink('test2', target, target_is_directory=True)
127+
with self.open(self.ar_with_implicit_dir) as ar:
128+
self.extractall(ar)
129+
self.assertTrue(os.path.islink(target))
130+
self.assertTrue(os.path.isdir(target2))
131+
self.assertTrue(os.path.isfile(os.path.join(target2, 'file')))
132+
with open(os.path.join(target2, 'file'), 'rb') as f:
133+
self.assertEqual(f.read(), b'newcontent')
134+
135+
@os_helper.skip_unless_symlink
136+
def test_overwrite_broken_dir_symlink_as_dir(self):
137+
target = os.path.join(self.testdir, 'test')
138+
target2 = os.path.join(self.testdir, 'test2')
139+
os.symlink('test2', target, target_is_directory=True)
140+
with self.open(self.ar_with_dir) as ar:
141+
with self.assertRaises(FileExistsError):
142+
self.extractall(ar)
143+
self.assertTrue(os.path.islink(target))
144+
self.assertFalse(os.path.exists(target2))
145+
146+
@os_helper.skip_unless_symlink
147+
def test_overwrite_broken_dir_symlink_as_implicit_dir(self):
148+
target = os.path.join(self.testdir, 'test')
149+
target2 = os.path.join(self.testdir, 'test2')
150+
os.symlink('test2', target, target_is_directory=True)
151+
with self.open(self.ar_with_implicit_dir) as ar:
152+
with self.assertRaises(FileExistsError):
153+
self.extractall(ar)
154+
self.assertTrue(os.path.islink(target))
155+
self.assertFalse(os.path.exists(target2))

‎Lib/test/test_tarfile.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import unittest.mock
1616
import tarfile
1717

18+
from test import archiver_tests
1819
from test import support
1920
from test.support import os_helper
2021
from test.support import script_helper
@@ -4089,6 +4090,38 @@ def valueerror_filter(tarinfo, path):
40894090
self.expect_exception(TypeError) # errorlevel is not int
40904091

40914092

4093+
class OverwriteTests(archiver_tests.OverwriteTests, unittest.TestCase):
4094+
testdir = os.path.join(TEMPDIR, "testoverwrite")
4095+
4096+
@classmethod
4097+
def setUpClass(cls):
4098+
p = cls.ar_with_file = os.path.join(TEMPDIR, 'tar-with-file.tar')
4099+
cls.addClassCleanup(os_helper.unlink, p)
4100+
with tarfile.open(p, 'w') as tar:
4101+
t = tarfile.TarInfo('test')
4102+
t.size = 10
4103+
tar.addfile(t, io.BytesIO(b'newcontent'))
4104+
4105+
p = cls.ar_with_dir = os.path.join(TEMPDIR, 'tar-with-dir.tar')
4106+
cls.addClassCleanup(os_helper.unlink, p)
4107+
with tarfile.open(p, 'w') as tar:
4108+
tar.addfile(tar.gettarinfo(os.curdir, 'test'))
4109+
4110+
p = os.path.join(TEMPDIR, 'tar-with-implicit-dir.tar')
4111+
cls.ar_with_implicit_dir = p
4112+
cls.addClassCleanup(os_helper.unlink, p)
4113+
with tarfile.open(p, 'w') as tar:
4114+
t = tarfile.TarInfo('test/file')
4115+
t.size = 10
4116+
tar.addfile(t, io.BytesIO(b'newcontent'))
4117+
4118+
def open(self, path):
4119+
return tarfile.open(path, 'r')
4120+
4121+
def extractall(self, ar):
4122+
ar.extractall(self.testdir, filter='fully_trusted')
4123+
4124+
40924125
def setUpModule():
40934126
os_helper.unlink(TEMPDIR)
40944127
os.makedirs(TEMPDIR)

‎Lib/test/test_zipfile/test_core.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
from tempfile import TemporaryFile
1919
from random import randint, random, randbytes
2020

21+
from test import archiver_tests
2122
from test.support import script_helper
2223
from test.support import (
2324
findfile, requires_zlib, requires_bz2, requires_lzma,
@@ -1687,6 +1688,33 @@ def _test_extract_hackers_arcnames(self, hacknames):
16871688
unlink(TESTFN2)
16881689

16891690

1691+
class OverwriteTests(archiver_tests.OverwriteTests, unittest.TestCase):
1692+
testdir = TESTFN
1693+
1694+
@classmethod
1695+
def setUpClass(cls):
1696+
p = cls.ar_with_file = TESTFN + '-with-file.zip'
1697+
cls.addClassCleanup(unlink, p)
1698+
with zipfile.ZipFile(p, 'w') as zipfp:
1699+
zipfp.writestr('test', b'newcontent')
1700+
1701+
p = cls.ar_with_dir = TESTFN + '-with-dir.zip'
1702+
cls.addClassCleanup(unlink, p)
1703+
with zipfile.ZipFile(p, 'w') as zipfp:
1704+
zipfp.mkdir('test')
1705+
1706+
p = cls.ar_with_implicit_dir = TESTFN + '-with-implicit-dir.zip'
1707+
cls.addClassCleanup(unlink, p)
1708+
with zipfile.ZipFile(p, 'w') as zipfp:
1709+
zipfp.writestr('test/file', b'newcontent')
1710+
1711+
def open(self, path):
1712+
return zipfile.ZipFile(path, 'r')
1713+
1714+
def extractall(self, ar):
1715+
ar.extractall(self.testdir)
1716+
1717+
16901718
class OtherTests(unittest.TestCase):
16911719
def test_open_via_zip_info(self):
16921720
# Create the ZIP archive

‎Misc/NEWS.d/next/Library/2024-02-03-16-59-25.gh-issue-114959.dCfAG2.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
:mod:`tarfile` no longer ignores errors when trying to extract a directory on
2+
top of a file.

0 commit comments

Comments
 (0)
0