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

Commit e846c88

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

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
@@ -2445,7 +2445,8 @@ def makedir(self, tarinfo, targetpath):
24452445
# later in _extract_member().
24462446
os.mkdir(targetpath, 0o700)
24472447
except FileExistsError:
2448-
pass
2448+
if not os.path.isdir(targetpath):
2449+
raise
24492450

24502451
def makefile(self, tarinfo, targetpath):
24512452
"""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' F438 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
@@ -4013,6 +4014,38 @@ def valueerror_filter(tarinfo, path):
40134014
self.expect_exception(TypeError) # errorlevel is not int
40144015

40154016

4017+
class OverwriteTests(archiver_tests.OverwriteTests, unittest.TestCase):
4018+
testdir = os.path.join(TEMPDIR, "testoverwrite")
4019+
4020+
@classmethod
4021+
def setUpClass(cls):
4022+
p = cls.ar_with_file = os.path.join(TEMPDIR, 'tar-with-file.tar')
4023+
cls.addClassCleanup(os_helper.unlink, p)
4024+
with tarfile.open(p, 'w') as tar:
4025+
t = tarfile.TarInfo('test')
4026+
t.size = 10
4027+
tar.addfile(t, io.BytesIO(b'newcontent'))
4028+
4029+
p = cls.ar_with_dir = os.path.join(TEMPDIR, 'tar-with-dir.tar')
4030+
cls.addClassCleanup(os_helper.unlink, p)
4031+
with tarfile.open(p, 'w') as tar:
4032+
tar.addfile(tar.gettarinfo(os.curdir, 'test'))
4033+
4034+
p = os.path.join(TEMPDIR, 'tar-with-implicit-dir.tar')
4035+
cls.ar_with_implicit_dir = p
4036+
cls.addClassCleanup(os_helper.unlink, p)
4037+
with tarfile.open(p, 'w') as tar:
4038+
t = tarfile.TarInfo('test/file')
4039+
t.size = 10
4040+
tar.addfile(t, io.BytesIO(b'newcontent'))
4041+
4042+
def open(self, path):
4043+
return tarfile.open(path, 'r')
4044+
4045+
def extractall(self, ar):
4046+
ar.extractall(self.testdir, filter='fully_trusted')
4047+
4048+
40164049
def setUpModule():
40174050
os_helper.unlink(TEMPDIR)
40184051
os.makedirs(TEMPDIR)

‎Lib/test/test_zipfile.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
from tempfile import TemporaryFile
2222
from random import randint, random, randbytes
2323

24+
from test import archiver_tests
2425
from test.support import script_helper
2526
from test.support import (
2627
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