Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion .cirrus.yml
Original file line number Diff line number Diff line change
Expand Up @@ -194,10 +194,14 @@ analysis_linux_task:
qa_task:
alias: qa
matrix:
- name: "Test Python 3.9"
- name: "Test Python 3.9.18"
eks_container:
docker_arguments:
PYTHON_VERSION: 3.9.18
- name: "Test Python 3.9.6"
eks_container:
docker_arguments:
PYTHON_VERSION: 3.9.6
- name: "Test Python 3.10"
eks_container:
docker_arguments:
Expand Down
3 changes: 1 addition & 2 deletions src/pysonar_scanner/jre.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,7 @@ def __extract_jre(self, file_path: pathlib.Path, unzip_dir: pathlib.Path):
with zipfile.ZipFile(file_path, "r") as zip_ref:
zip_ref.extractall(unzip_dir)
elif file_path.suffix in [".gz", ".tgz"]:
with tarfile.open(file_path, "r:gz") as tar_ref:
tar_ref.extractall(unzip_dir, filter="data")
utils.extract_tar(file_path, unzip_dir)
else:
raise UnsupportedArchiveFormat(
f"Received JRE is packaged as an unsupported archive format: {file_path.suffix}"
Expand Down
10 changes: 10 additions & 0 deletions src/pysonar_scanner/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
import hashlib
import pathlib
import platform
import sys
import tarfile
import typing
from enum import Enum

Expand Down Expand Up @@ -89,3 +91,11 @@ def get_arch() -> Arch:

def filter_none_values(dictionary: dict) -> dict:
return {k: v for k, v in dictionary.items() if v is not None}


def extract_tar(path: pathlib.Path, target_dir: pathlib.Path):
with tarfile.open(path, "r:gz") as tar_ref:
if sys.version_info >= (3, 12):
tar_ref.extractall(target_dir, filter="data")
else:
tar_ref.extractall(target_dir)
Copy link
Copy Markdown
Contributor Author

@Seppli11 Seppli11 May 26, 2025

Choose a reason for hiding this comment

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

There is a security risk of tar bombs here.
I see two possible attacks:

  1. An attacker compromises the SQ server and sends a malicious tar
  2. Since checking the checksum of the tar and actually untarring it are done in separate steps, an attacker could change the content of the tar between the two steps. However, for this, an attacker would have to be able to execute arbitrary commands

Overall, the security risk seems low to me, but it is still a risk.

Link to the sonar security rule: https://next.sonarqube.com/sonarqube/security_hotspots?id=SonarSource_sonar-scanner-python&hotspots=9ec9486e-0271-4c5f-820f-b780ab5863d0&pullRequest=201

30 changes: 29 additions & 1 deletion tests/unit/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import unittest.mock
import pyfakefs.fake_filesystem_unittest as pyfakefs

from pysonar_scanner.utils import Arch, Os, get_arch, get_os, remove_trailing_slash, calculate_checksum
from pysonar_scanner.utils import Arch, Os, get_arch, get_os, remove_trailing_slash, calculate_checksum, extract_tar


class TestUtils(unittest.TestCase):
Expand Down Expand Up @@ -132,3 +132,31 @@ def test_calculate_checksum(self):
calculate_checksum(BytesIO(b"test test")),
"03ffdf45276dd38ffac79b0e9c6c14d89d9113ad783d5922580f4c66a3305591",
)


class TestExtractTar(unittest.TestCase):
def setUp(self):
self.test_path = pathlib.Path("/fake/path/archive.tar.gz")
self.test_target_dir = pathlib.Path("/fake/target/dir")

@unittest.mock.patch("tarfile.open")
@unittest.mock.patch("sys.version_info", (3, 12, 0))
def test_extract_tar_python_3_12_or_higher(self, mock_open):
mock_tar = unittest.mock.MagicMock()
mock_open.return_value.__enter__.return_value = mock_tar

extract_tar(self.test_path, self.test_target_dir)

mock_open.assert_called_once_with(self.test_path, "r:gz")
mock_tar.extractall.assert_called_once_with(self.test_target_dir, filter="data")

@unittest.mock.patch("tarfile.open")
@unittest.mock.patch("sys.version_info", (3, 11, 0))
def test_extract_tar_python_older_than_3_12(self, mock_open):
mock_tar = unittest.mock.MagicMock()
mock_open.return_value.__enter__.return_value = mock_tar

extract_tar(self.test_path, self.test_target_dir)

mock_open.assert_called_once_with(self.test_path, "r:gz")
mock_tar.extractall.assert_called_once_with(self.test_target_dir)