Skip to content

Fix OS command injection in WinRweAccess via os.system → subprocess.run(shell=False) - #114

Open
sys-xmlcli wants to merge 4 commits into
intel:mainfrom
sys-xmlcli:PTK0009082
Open

sys-xmlcli wants to merge 4 commits into
intel:mainfrom
sys-xmlcli:PTK0009082

Conversation

@sys-xmlcli

Copy link
Copy Markdown
Contributor

No description provided.

# Built-in imports
import os
import binascii
import subprocess
Comment thread src/xmlcli/access/winrwe/winrwe.py Fixed
Comment thread src/xmlcli/access/winrwe/winrwe.py Fixed
Comment thread src/xmlcli/access/winrwe/winrwe.py Fixed
Comment thread src/xmlcli/access/winrwe/winrwe.py Fixed
Comment thread src/xmlcli/access/winrwe/winrwe.py Fixed
Comment thread src/xmlcli/access/winrwe/winrwe.py Fixed
Comment thread src/xmlcli/access/winrwe/winrwe.py Fixed
Comment thread src/xmlcli/access/winrwe/winrwe.py Fixed
Comment thread src/xmlcli/access/winrwe/winrwe.py Fixed

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Staging fails when the caller’s filename aliases the configured temporary file.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Replaces shell-based RW.exe execution to prevent command injection.

Changes:

  • Uses subprocess.run with structured arguments.
  • Validates RW command paths.
  • Stages caller files through a trusted temporary path.
File Description
src/​xmlcli/​access/​winrwe/​winrwe.py Secures RW.exe invocation and file handling.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/xmlcli/access/winrwe/winrwe.py Outdated

This branch has not been deployed

No deployments
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.

4 participants