From 2b540166f742450d0256d6f219c97bd77ff6b6c6 Mon Sep 17 00:00:00 2001 From: TrellixVulnTeam Date: Wed, 16 Nov 2022 12:44:12 +0000 Subject: [PATCH] Adding tarfile member sanitization to extractall() --- redkyn-grader/grader/models/submission.py | 42 +++++++++++++++++++++-- redkyn-grader/grader/test/test_import.py | 42 +++++++++++++++++++++-- 2 files changed, 80 insertions(+), 4 deletions(-) diff --git a/redkyn-grader/grader/models/submission.py b/redkyn-grader/grader/models/submission.py index a9168ff..81f324d 100644 --- a/redkyn-grader/grader/models/submission.py +++ b/redkyn-grader/grader/models/submission.py @@ -127,7 +127,26 @@ def _remove_extension(cls, basename): def _check_tarball(cls, assignment, path, student_id): with tempfile.TemporaryDirectory() as tmpdir: with tarfile.open(path) as tar: - tar.extractall(tmpdir) + def is_within_directory(directory, target): + + abs_directory = os.path.abspath(directory) + abs_target = os.path.abspath(target) + + prefix = os.path.commonprefix([abs_directory, abs_target]) + + return prefix == abs_directory + + def safe_extract(tar, path=".", members=None, *, numeric_owner=False): + + for member in tar.getmembers(): + member_path = os.path.join(path, member.name) + if not is_within_directory(path, member_path): + raise Exception("Attempted Path Traversal in Tar File") + + tar.extractall(path, members, numeric_owner=numeric_owner) + + + safe_extract(tar, tmpdir) try: # Try to unpack our single folder @@ -434,7 +453,26 @@ def unpacked_files(self): """ with tempfile.TemporaryDirectory() as tmpdir: with tarfile.open(self.path, "r") as tar: - tar.extractall(tmpdir) + def is_within_directory(directory, target): + + abs_directory = os.path.abspath(directory) + abs_target = os.path.abspath(target) + + prefix = os.path.commonprefix([abs_directory, abs_target]) + + return prefix == abs_directory + + def safe_extract(tar, path=".", members=None, *, numeric_owner=False): + + for member in tar.getmembers(): + member_path = os.path.join(path, member.name) + if not is_within_directory(path, member_path): + raise Exception("Attempted Path Traversal in Tar File") + + tar.extractall(path, members, numeric_owner=numeric_owner) + + + safe_extract(tar, tmpdir) yield tmpdir @property diff --git a/redkyn-grader/grader/test/test_import.py b/redkyn-grader/grader/test/test_import.py index f0b3d6b..358d2d3 100644 --- a/redkyn-grader/grader/test/test_import.py +++ b/redkyn-grader/grader/test/test_import.py @@ -57,7 +57,26 @@ def check_grader_tarfile(grader_path, student_id, filenames=["main.py"]): tar_path = os.path.join(submission_dir, tar_filename) with tempfile.TemporaryDirectory() as tempdir: with tarfile.open(tar_path, "r:gz") as tar: - tar.extractall(tempdir) + def is_within_directory(directory, target): + + abs_directory = os.path.abspath(directory) + abs_target = os.path.abspath(target) + + prefix = os.path.commonprefix([abs_directory, abs_target]) + + return prefix == abs_directory + + def safe_extract(tar, path=".", members=None, *, numeric_owner=False): + + for member in tar.getmembers(): + member_path = os.path.join(path, member.name) + if not is_within_directory(path, member_path): + raise Exception("Attempted Path Traversal in Tar File") + + tar.extractall(path, members, numeric_owner=numeric_owner) + + + safe_extract(tar, tempdir) name, = os.listdir(tempdir) assert name == student_id @@ -94,7 +113,26 @@ def test_make_student_tarball(clean_dir): with tempfile.TemporaryDirectory() as tempdir: with tarfile.open(student_tarball, "r:gz") as tar: - tar.extractall(tempdir) + def is_within_directory(directory, target): + + abs_directory = os.path.abspath(directory) + abs_target = os.path.abspath(target) + + prefix = os.path.commonprefix([abs_directory, abs_target]) + + return prefix == abs_directory + + def safe_extract(tar, path=".", members=None, *, numeric_owner=False): + + for member in tar.getmembers(): + member_path = os.path.join(path, member.name) + if not is_within_directory(path, member_path): + raise Exception("Attempted Path Traversal in Tar File") + + tar.extractall(path, members, numeric_owner=numeric_owner) + + + safe_extract(tar, tempdir) # Look for folder assert os.listdir(tempdir) == ["jtd111"]