Skip to content

Commit 898ac93

Browse files
aeurielesngpshead
andcommitted
[3.10] pythongh-130577: tarfile now validates archives to ensure member offsets are non-negative (pythonGH-137027)
(cherry picked from commit 7040aa5) Co-authored-by: Alexander Urieles <aeurielesn@users.noreply.github.com> Co-authored-by: Gregory P. Smith <greg@krypto.org>
1 parent 1df5d00 commit 898ac93

File tree

3 files changed

+194
-0
lines changed

3 files changed

+194
-0
lines changed

Lib/tarfile.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1613,6 +1613,9 @@ def _block(self, count):
16131613
"""Round up a byte count by BLOCKSIZE and return it,
16141614
e.g. _block(834) => 1024.
16151615
"""
1616+
# Only non-negative offsets are allowed
1617+
if count < 0:
1618+
raise InvalidHeaderError("invalid offset")
16161619
blocks, remainder = divmod(count, BLOCKSIZE)
16171620
if remainder:
16181621
blocks += 1

Lib/test/test_tarfile.py

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ def sha256sum(data):
4949
xzname = os.path.join(TEMPDIR, "testtar.tar.xz")
5050
tmpname = os.path.join(TEMPDIR, "tmp.tar")
5151
dotlessname = os.path.join(TEMPDIR, "testtar")
52+
SPACE = b" "
5253

5354
sha256_regtype = (
5455
"e09e4bc8b3c9d9177e77256353b36c159f5f040531bbd4b024a8f9b9196c71ce"
@@ -4273,6 +4274,193 @@ def valueerror_filter(tarinfo, path):
42734274
self.expect_exception(TypeError) # errorlevel is not int
42744275

42754276

4277+
class OverwriteTests(archiver_tests.OverwriteTests, unittest.TestCase):
4278+
testdir = os.path.join(TEMPDIR, "testoverwrite")
4279+
4280+
@classmethod
4281+
def setUpClass(cls):
4282+
p = cls.ar_with_file = os.path.join(TEMPDIR, 'tar-with-file.tar')
4283+
cls.addClassCleanup(os_helper.unlink, p)
4284+
with tarfile.open(p, 'w') as tar:
4285+
t = tarfile.TarInfo('test')
4286+
t.size = 10
4287+
tar.addfile(t, io.BytesIO(b'newcontent'))
4288+
4289+
p = cls.ar_with_dir = os.path.join(TEMPDIR, 'tar-with-dir.tar')
4290+
cls.addClassCleanup(os_helper.unlink, p)
4291+
with tarfile.open(p, 'w') as tar:
4292+
tar.addfile(tar.gettarinfo(os.curdir, 'test'))
4293+
4294+
p = os.path.join(TEMPDIR, 'tar-with-implicit-dir.tar')
4295+
cls.ar_with_implicit_dir = p
4296+
cls.addClassCleanup(os_helper.unlink, p)
4297+
with tarfile.open(p, 'w') as tar:
4298+
t = tarfile.TarInfo('test/file')
4299+
t.size = 10
4300+
tar.addfile(t, io.BytesIO(b'newcontent'))
4301+
4302+
def open(self, path):
4303+
return tarfile.open(path, 'r')
4304+
4305+
def extractall(self, ar):
4306+
ar.extractall(self.testdir, filter='fully_trusted')
4307+
4308+
4309+
class OffsetValidationTests(unittest.TestCase):
4310+
tarname = tmpname
4311+
invalid_posix_header = (
4312+
# name: 100 bytes
4313+
tarfile.NUL * tarfile.LENGTH_NAME
4314+
# mode, space, null terminator: 8 bytes
4315+
+ b"000755" + SPACE + tarfile.NUL
4316+
# uid, space, null terminator: 8 bytes
4317+
+ b"000001" + SPACE + tarfile.NUL
4318+
# gid, space, null terminator: 8 bytes
4319+
+ b"000001" + SPACE + tarfile.NUL
4320+
# size, space: 12 bytes
4321+
+ b"\xff" * 11 + SPACE
4322+
# mtime, space: 12 bytes
4323+
+ tarfile.NUL * 11 + SPACE
4324+
# chksum: 8 bytes
4325+
+ b"0011407" + tarfile.NUL
4326+
# type: 1 byte
4327+
+ tarfile.REGTYPE
4328+
# linkname: 100 bytes
4329+
+ tarfile.NUL * tarfile.LENGTH_LINK
4330+
# magic: 6 bytes, version: 2 bytes
4331+
+ tarfile.POSIX_MAGIC
4332+
# uname: 32 bytes
4333+
+ tarfile.NUL * 32
4334+
# gname: 32 bytes
4335+
+ tarfile.NUL * 32
4336+
# devmajor, space, null terminator: 8 bytes
4337+
+ tarfile.NUL * 6 + SPACE + tarfile.NUL
4338+
# devminor, space, null terminator: 8 bytes
4339+
+ tarfile.NUL * 6 + SPACE + tarfile.NUL
4340+
# prefix: 155 bytes
4341+
+ tarfile.NUL * tarfile.LENGTH_PREFIX
4342+
# padding: 12 bytes
4343+
+ tarfile.NUL * 12
4344+
)
4345+
invalid_gnu_header = (
4346+
# name: 100 bytes
4347+
tarfile.NUL * tarfile.LENGTH_NAME
4348+
# mode, null terminator: 8 bytes
4349+
+ b"0000755" + tarfile.NUL
4350+
# uid, null terminator: 8 bytes
4351+
+ b"0000001" + tarfile.NUL
4352+
# gid, space, null terminator: 8 bytes
4353+
+ b"0000001" + tarfile.NUL
4354+
# size, space: 12 bytes
4355+
+ b"\xff" * 11 + SPACE
4356+
# mtime, space: 12 bytes
4357+
+ tarfile.NUL * 11 + SPACE
4358+
# chksum: 8 bytes
4359+
+ b"0011327" + tarfile.NUL
4360+
# type: 1 byte
4361+
+ tarfile.REGTYPE
4362+
# linkname: 100 bytes
4363+
+ tarfile.NUL * tarfile.LENGTH_LINK
4364+
# magic: 8 bytes
4365+
+ tarfile.GNU_MAGIC
4366+
# uname: 32 bytes
4367+
+ tarfile.NUL * 32
4368+
# gname: 32 bytes
4369+
+ tarfile.NUL * 32
4370+
# devmajor, null terminator: 8 bytes
4371+
+ tarfile.NUL * 8
4372+
# devminor, null terminator: 8 bytes
4373+
+ tarfile.NUL * 8
4374+
# padding: 167 bytes
4375+
+ tarfile.NUL * 167
4376+
)
4377+
invalid_v7_header = (
4378+
# name: 100 bytes
4379+
tarfile.NUL * tarfile.LENGTH_NAME
4380+
# mode, space, null terminator: 8 bytes
4381+
+ b"000755" + SPACE + tarfile.NUL
4382+
# uid, space, null terminator: 8 bytes
4383+
+ b"000001" + SPACE + tarfile.NUL
4384+
# gid, space, null terminator: 8 bytes
4385+
+ b"000001" + SPACE + tarfile.NUL
4386+
# size, space: 12 bytes
4387+
+ b"\xff" * 11 + SPACE
4388+
# mtime, space: 12 bytes
4389+
+ tarfile.NUL * 11 + SPACE
4390+
# chksum: 8 bytes
4391+
+ b"0010070" + tarfile.NUL
4392+
# type: 1 byte
4393+
+ tarfile.REGTYPE
4394+
# linkname: 100 bytes
4395+
+ tarfile.NUL * tarfile.LENGTH_LINK
4396+
# padding: 255 bytes
4397+
+ tarfile.NUL * 255
4398+
)
4399+
valid_gnu_header = tarfile.TarInfo("filename").tobuf(tarfile.GNU_FORMAT)
4400+
data_block = b"\xff" * tarfile.BLOCKSIZE
4401+
4402+
def _write_buffer(self, buffer):
4403+
with open(self.tarname, "wb") as f:
4404+
f.write(buffer)
4405+
4406+
def _get_members(self, ignore_zeros=None):
4407+
with open(self.tarname, "rb") as f:
4408+
with tarfile.open(
4409+
mode="r", fileobj=f, ignore_zeros=ignore_zeros
4410+
) as tar:
4411+
return tar.getmembers()
4412+
4413+
def _assert_raises_read_error_exception(self):
4414+
with self.assertRaisesRegex(
4415+
tarfile.ReadError, "file could not be opened successfully"
4416+
):
4417+
self._get_members()
4418+
4419+
def test_invalid_offset_header_validations(self):
4420+
for tar_format, invalid_header in (
4421+
("posix", self.invalid_posix_header),
4422+
("gnu", self.invalid_gnu_header),
4423+
("v7", self.invalid_v7_header),
4424+
):
4425+
with self.subTest(format=tar_format):
4426+
self._write_buffer(invalid_header)
4427+
self._assert_raises_read_error_exception()
4428+
4429+
def test_early_stop_at_invalid_offset_header(self):
4430+
buffer = self.valid_gnu_header + self.invalid_gnu_header + self.valid_gnu_header
4431+
self._write_buffer(buffer)
4432+
members = self._get_members()
4433+
self.assertEqual(len(members), 1)
4434+
self.assertEqual(members[0].name, "filename")
4435+
self.assertEqual(members[0].offset, 0)
4436+
4437+
def test_ignore_invalid_archive(self):
4438+
# 3 invalid headers with their respective data
4439+
buffer = (self.invalid_gnu_header + self.data_block) * 3
4440+
self._write_buffer(buffer)
4441+
members = self._get_members(ignore_zeros=True)
4442+
self.assertEqual(len(members), 0)
4443+
4444+
def test_ignore_invalid_offset_headers(self):
4445+
for first_block, second_block, expected_offset in (
4446+
(
4447+
(self.valid_gnu_header),
4448+
(self.invalid_gnu_header + self.data_block),
4449+
0,
4450+
),
4451+
(
4452+
(self.invalid_gnu_header + self.data_block),
4453+
(self.valid_gnu_header),
4454+
1024,
4455+
),
4456+
):
4457+
self._write_buffer(first_block + second_block)
4458+
members = self._get_members(ignore_zeros=True)
4459+
self.assertEqual(len(members), 1)
4460+
self.assertEqual(members[0].name, "filename")
4461+
self.assertEqual(members[0].offset, expected_offset)
4462+
4463+
42764464
def setUpModule():
42774465
os_helper.unlink(TEMPDIR)
42784466
os.makedirs(TEMPDIR)
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
:mod:`tarfile` now validates archives to ensure member offsets are
2+
non-negative. (Contributed by Alexander Enrique Urieles Nieto in
3+
:gh:`130577`.)

0 commit comments

Comments
 (0)