Skip to content

security: Escape field values that might contain RPM macros#301

Open
gordonmessmer wants to merge 1 commit into
mainfrom
rendersec
Open

security: Escape field values that might contain RPM macros#301
gordonmessmer wants to merge 1 commit into
mainfrom
rendersec

Conversation

@gordonmessmer
Copy link
Copy Markdown
Member

Assisted-by: Claude

Copy link
Copy Markdown
Member

@hroncok hroncok left a comment

Choose a reason for hiding this comment

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

Looks OK. I haven't tested it, and the CI here is dead

Addresses multiple security vulnerabilities where untrusted input could
inject RPM macros and directives during spec file generation.

Vulnerabilities fixed:
1. update_attr() bypass - set_from(..., update=True) bypassed __setattr__
   escaping, allowing PyPI metadata to inject macros
2. Missing field escaping - dirname, sphinx_dir, scripts, py_modules,
   packages, doc_files, doc_license were rendered without escaping
3. Dependency injection - package names in dependencies not escaped
4. Truncate/wordwrap boundary - could split %% leaving dangerous single %

Solution:
- Removed bypassable __setattr__ escaping from package_data.py
- Implemented render-time escaping with new rpm_escape Jinja filter
- Applied escaping at correct points in filter chain:
  * Before replace() operations to preserve template macros
  * Inside module_to_path/package_to_path filters for literal returns
  * Before name_for_python_version for dependency names
  * After truncate/wordwrap for descriptions
- Updated all 6 template variants (fedora, epel6, epel7, mageia, pld, macros)
- Updated test expectations to reflect escaped output
- Added comprehensive security test suite (294 lines)
- Fixed tox.ini to use pytest directly instead of deprecated setup.py test

All user-controlled fields now properly escaped while preserving legitimate
RPM template macros.

This flaw was reported by https://github.com/gouldnicholas

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Gordon Messmer <gmessmer@redhat.com>
@gordonmessmer
Copy link
Copy Markdown
Member Author

@gouldnicholas please review

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants