Skip to content

[3.9] gh-130577: tarfile now validates archives to ensure member offsets are non-negative (GH-137027) #137177

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: 3.9
Choose a base branch
from

Conversation

gpshead
Copy link
Member

@gpshead gpshead commented Jul 28, 2025

…r 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>
@@ -4234,6 +4235,193 @@ def valueerror_filter(tarinfo, path):
self.expect_exception(TypeError) # errorlevel is not int


class OverwriteTests(archiver_tests.OverwriteTests, unittest.TestCase):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

archiver_tests are not imported and do not exist in 3.9 sources.

Comment on lines +4238 to +4269
class OverwriteTests(archiver_tests.OverwriteTests, unittest.TestCase):
testdir = os.path.join(TEMPDIR, "testoverwrite")

@classmethod
def setUpClass(cls):
p = cls.ar_with_file = os.path.join(TEMPDIR, 'tar-with-file.tar')
cls.addClassCleanup(os_helper.unlink, p)
with tarfile.open(p, 'w') as tar:
t = tarfile.TarInfo('test')
t.size = 10
tar.addfile(t, io.BytesIO(b'newcontent'))

p = cls.ar_with_dir = os.path.join(TEMPDIR, 'tar-with-dir.tar')
cls.addClassCleanup(os_helper.unlink, p)
with tarfile.open(p, 'w') as tar:
tar.addfile(tar.gettarinfo(os.curdir, 'test'))

p = os.path.join(TEMPDIR, 'tar-with-implicit-dir.tar')
cls.ar_with_implicit_dir = p
cls.addClassCleanup(os_helper.unlink, p)
with tarfile.open(p, 'w') as tar:
t = tarfile.TarInfo('test/file')
t.size = 10
tar.addfile(t, io.BytesIO(b'newcontent'))

def open(self, path):
return tarfile.open(path, 'r')

def extractall(self, ar):
ar.extractall(self.testdir, filter='fully_trusted')


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as in the 3.10 backport, this seems to have been mistakenly added as a result of merge error.

@frenzymadness
Copy link
Contributor

Is there anything I can do to move this forward? There is another CVE fix already merged into 3.9, and it'd be great to release them both together.

Comment on lines +4238 to +4269
class OverwriteTests(archiver_tests.OverwriteTests, unittest.TestCase):
testdir = os.path.join(TEMPDIR, "testoverwrite")

@classmethod
def setUpClass(cls):
p = cls.ar_with_file = os.path.join(TEMPDIR, 'tar-with-file.tar')
cls.addClassCleanup(os_helper.unlink, p)
with tarfile.open(p, 'w') as tar:
t = tarfile.TarInfo('test')
t.size = 10
tar.addfile(t, io.BytesIO(b'newcontent'))

p = cls.ar_with_dir = os.path.join(TEMPDIR, 'tar-with-dir.tar')
cls.addClassCleanup(os_helper.unlink, p)
with tarfile.open(p, 'w') as tar:
tar.addfile(tar.gettarinfo(os.curdir, 'test'))

p = os.path.join(TEMPDIR, 'tar-with-implicit-dir.tar')
cls.ar_with_implicit_dir = p
cls.addClassCleanup(os_helper.unlink, p)
with tarfile.open(p, 'w') as tar:
t = tarfile.TarInfo('test/file')
t.size = 10
tar.addfile(t, io.BytesIO(b'newcontent'))

def open(self, path):
return tarfile.open(path, 'r')

def extractall(self, ar):
ar.extractall(self.testdir, filter='fully_trusted')


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case that helps update it:

Suggested change
class OverwriteTests(archiver_tests.OverwriteTests, unittest.TestCase):
testdir = os.path.join(TEMPDIR, "testoverwrite")
@classmethod
def setUpClass(cls):
p = cls.ar_with_file = os.path.join(TEMPDIR, 'tar-with-file.tar')
cls.addClassCleanup(os_helper.unlink, p)
with tarfile.open(p, 'w') as tar:
t = tarfile.TarInfo('test')
t.size = 10
tar.addfile(t, io.BytesIO(b'newcontent'))
p = cls.ar_with_dir = os.path.join(TEMPDIR, 'tar-with-dir.tar')
cls.addClassCleanup(os_helper.unlink, p)
with tarfile.open(p, 'w') as tar:
tar.addfile(tar.gettarinfo(os.curdir, 'test'))
p = os.path.join(TEMPDIR, 'tar-with-implicit-dir.tar')
cls.ar_with_implicit_dir = p
cls.addClassCleanup(os_helper.unlink, p)
with tarfile.open(p, 'w') as tar:
t = tarfile.TarInfo('test/file')
t.size = 10
tar.addfile(t, io.BytesIO(b'newcontent'))
def open(self, path):
return tarfile.open(path, 'r')
def extractall(self, ar):
ar.extractall(self.testdir, filter='fully_trusted')

@vstinner
Copy link
Member

Alternative backport without the OverwriteTests class: #137645

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

7 participants