[Bugfix] Address command injection vulnerability in vm-repair extension; validate and quote tag values#10057
Open
EdwinBernal1 wants to merge 7 commits into
Open
[Bugfix] Address command injection vulnerability in vm-repair extension; validate and quote tag values#10057EdwinBernal1 wants to merge 7 commits into
EdwinBernal1 wants to merge 7 commits into
Conversation
|
Validation for Breaking Change Starting...
Thanks for your contribution! |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR mitigates a Windows-specific command injection risk in the vm-repair extension by ensuring untrusted tag data (from --copy-tags / --tags) cannot be re-interpreted by cmd.exe when az vm create is executed via a command string.
Changes:
- Validate and POSIX-quote each
key=valuetag token before interpolating tags into theaz vm createcommand string. - Harden
_call_az_commandon Windows to invokecmd /s /c "..."with per-tokenCommandLineToArgvW-compatible quoting via a new_quote_cmd_arghelper. - Add regression tests (including Windows-only
cmd.exeround-trip tests), and bump extension version/history to2.2.1.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/vm-repair/setup.py | Bumps extension version to 2.2.1. |
| src/vm-repair/HISTORY.rst | Adds 2.2.1 release notes describing the security fix. |
| src/vm-repair/azext_vm_repair/repair_utils.py | Adds _quote_cmd_arg and updates _call_az_command to safely build Windows cmd /s /c invocation. |
| src/vm-repair/azext_vm_repair/custom.py | Validates tag key/value content and quotes each key=value token before embedding into the command string. |
| src/vm-repair/azext_vm_repair/tests/latest/test_command_injection.py | Adds unit + Windows-only integration-style tests for quoting and injection prevention. |
Collaborator
|
VM |
…amation marks, and control characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
az vm repair create/az vm repair repair-and-restorebuild the underlyingaz vm createcall as a command string and, on Windows, execute it throughcmd.exe. Tag values copied from the source VM (--copy-tags) or supplied via--tagswere interpolated into that string without escaping. A tag valuecontaining shell metacharacters (for example
&,|,>,<,^) was thereforere-interpreted by
cmd.exeinstead of being treated as literal text.Because a tag value only requires
Microsoft.Resources/tags/writeon the source VMto set, an operator running the command on Windows against an affected VM could have
attacker-influenced tag content executed on their workstation. Linux was not affected
(the argument vector is passed directly to the process with no shell).
This was reported through MSRC and is being addressed as a security fix.
Cause
custom.pyflattened the merged tag dictionary into a singlekey=value key=valuestring and interpolated it into theaz vm createcommandwith no quoting.
repair_utils._call_az_commandtokenized the string withshlex.split(POSIXmode, which preserves
cmd.exemetacharacters) and, on Windows, prepended['cmd', '/c']and passed the list tosubprocess.Popen.subprocess.list2cmdlineonly quotes tokens that contain whitespace, so a token such as
env=ok&echoreached
cmd.exeunquoted and the&was parsed as a command separator.Fix
Defense in depth across both layers:
Neutralize tags at the boundary (
custom.py) — tag keys/values are validated(rejecting double quotes and control characters that cannot be represented as a
single shell argument) and each
key=valuetoken is quoted withshlex.quotebefore it is interpolated into the command. This also fixes a pre-existing
functional bug where tag values containing spaces were split into multiple tokens.
Harden command execution (
repair_utils._call_az_command) — on Windows thecommand is now built explicitly: every token is wrapped in double quotes
(with
CommandLineToArgvW-correct backslash/quote escaping via the new_quote_cmd_arghelper) and the whole command is invoked ascmd /s /c "<quoted tokens>".The
/sflag plus a single outer pair of quotes is required: without/s,cmd.exestrips the first and last quote on the line (its documented/cbehavior), which would unbalance the quoting around the final argument and
re-expose metacharacters. With
/s,cmd.exestrips exactly the outer quotes andparses the remainder verbatim, keeping every per-token quote balanced. The POSIX
path is unchanged (argument list passed to
subprocess, no shell).Testing
Added
azext_vm_repair/tests/latest/test_command_injection.py:_quote_cmd_argcovering metacharacters (& | < > ^ ( ) && ||),embedded double quotes, single/multiple trailing backslashes, backslash-before-quote,
internal backslashes, empty/space-only values, and normal paths.
cmd /s /c "..."with each tokenquoted, and that the POSIX path passes an argument list with no shell.
cmd.exeround-trip tests that execute theexact production construction and assert each payload is received by the target
program verbatim, plus an injection probe that fails if a metacharacter payload is
able to create a marker file.
azcommand strings are still rejected.All tests pass; the previously affected payloads are delivered as literal,
single arguments with no command execution.
History
vm-repairextension version to2.2.1.2.2.1entry toHISTORY.rst.This checklist is used to make sure that common guidelines for a pull request are followed.
Related command
az vm repair createandaz vm repair repair-and-restore(shared--tags/--copy-tagshandling and the internal_call_az_commandhelper).General Guidelines
azdev style <YOUR_EXT>locally? (pip install azdevrequired)python scripts/ci/test_index.py -qlocally? (azdevrequired; see.azure-pipelines/templates/azdev_setup.ymlfor the install command untilazdev==0.2.11b1is on PyPI)For new extensions:
About Extension Publish
There is a pipeline to automatically build, upload and publish extension wheels.
Once your pull request is merged into main branch, a new pull request will be created to update
src/index.jsonautomatically.You only need to update the version information in file setup.py and historical information in file HISTORY.rst in your PR but do not modify
src/index.json.