[Bugfix] Address command injection vulnerability in vm-repair extension; validate and quote tag values

src/vm-repair/HISTORY.rst

@@ -2,6 +2,10 @@
 Release History
 ===============
 
+2.2.1
+++++++
+Fixing a command injection vulnerability (MSRC 115198 / VULN-185362). Source VM tag values copied via ``--copy-tags`` could contain shell metacharacters that, on Windows, were interpreted by ``cmd.exe`` and executed as arbitrary commands on the operator's workstation. Tag keys and values are now validated and quoted before being interpolated into the ``az`` command, and ``_call_az_command`` quotes every argument so ``cmd.exe`` treats shell metacharacters as literal text. Minimum fixed version: 2.2.1.
+
 2.2.0
 ++++++
 Adding `--tags` parameter to `vm repair create` and `vm repair repair-and-restore` commands to allow users to tag the repair VM for organizational requirements

src/vm-repair/azext_vm_repair/custom.py

@@ -5,6 +5,7 @@
 
 # pylint: disable=line-too-long, too-many-locals, too-many-statements, broad-except, too-many-branches
 import json
+import shlex
 import timeit
 import traceback
 import requests
@@ -24,6 +25,7 @@
     _fetch_compatible_sku,
     _list_resource_ids_in_rg,
     _get_repair_resource_tag,
+    _validate_tags_for_command,
     _fetch_compatible_windows_os_urn,
     _fetch_matching_windows_os_urn,
     _fetch_run_script_map,
@@ -136,8 +138,21 @@ def create(cmd, vm_name, resource_group_name, repair_password=None, repair_usern
         if sep:
             merged_tags[repair_key] = repair_value
 
+        # Validate tag keys and values before they are placed into the command string.
+        # Tag values can originate from the source VM (via --copy-tags) and are therefore
+        # untrusted. _validate_tags_for_command rejects double quotes, control characters and
+        # the cmd.exe expansion characters '%' and '!' (which cannot be safely escaped on a
+        # 'cmd /c' command line); every other character, including shell metacharacters such
+        # as & | < >, is preserved and passed through literally. See MSRC 115198 / VULN-185362.
+        _validate_tags_for_command(merged_tags)
+
         # Convert to CLI string for passing to az cli later.
-        tag_string = ' '.join(f'{k}={v}' for k, v in merged_tags.items())
+        # Each key=value token is quoted with shlex.quote so values containing spaces or
+        # shell metacharacters survive re-tokenization in _call_az_command as a single
+        # argument. Combined with the Windows cmd.exe quoting in _call_az_command, this
+        # prevents command injection through attacker-controlled source VM tags.
+        # See MSRC 115198 / VULN-185362.
+        tag_string = ' '.join(shlex.quote(f'{tag_key}={tag_value}') for tag_key, tag_value in merged_tags.items())
 
         # initializing the list of created resources.
         created_resources = []

src/vm-repair/azext_vm_repair/repair_utils.py

@@ -18,7 +18,7 @@
 from .encryption_types import Encryption
 from .exceptions import (AzCommandError, WindowsOsNotAvailableError, RunScriptNotFoundForIdError, SkuDoesNotSupportHyperV, SkuNotAvailableError)
 
-from azure.cli.core.azclierror import CLIError
+from azure.cli.core.azclierror import CLIError, InvalidArgumentValueError
 
 REPAIR_MAP_URL = 'https://raw.githubusercontent.com/Azure/repair-script-library/master/map.json'
 
@@ -60,6 +60,61 @@ def _is_gen2(vm):
     return 1
 
 
+def _quote_cmd_arg(arg):
+    """
+    Quote a single argument for safe use on a Windows 'cmd /c' command line.
+
+    The argument is always wrapped in double quotes so that cmd.exe treats shell
+    metacharacters such as & | < > ( ) ^ as literal text instead of operators.
+    Embedded double quotes and any backslashes that precede them are escaped using
+    the Windows CommandLineToArgvW convention so the receiving program parses the
+    original value. This prevents command injection from untrusted values (for
+    example source VM tags) that are interpolated into the command string.
+    See MSRC 115198 / VULN-185362.
+    """
+    result = '"'
+    backslash_count = 0
+    for char in arg:
+        if char == '\\':
+            backslash_count += 1
+        elif char == '"':
+            # Double the backslashes that precede the quote, then escape the quote.
+            result += '\\' * (backslash_count * 2 + 1)
+            result += '"'
+            backslash_count = 0
+        else:
+            result += '\\' * backslash_count
+            result += char
+            backslash_count = 0
+    # Double any trailing backslashes so they do not escape the closing quote.
+    result += '\\' * (backslash_count * 2)
+    result += '"'
+    return result
+
+
+# Characters that cannot be safely carried through a Windows 'cmd /c' command line when
+# interpolated from an untrusted tag value (for example a source VM tag copied via
+# --copy-tags). Double quotes and ASCII control characters break argument tokenization,
+# and '%' / '!' are expanded by cmd.exe as environment / delayed-expansion variables even
+# inside double quotes -- '%' in particular cannot be reliably escaped on a 'cmd /c' line
+# (a leading '^' is preserved as a literal caret and corrupts the value). Such characters
+# are therefore rejected at the boundary rather than escaped. See MSRC 115198 / VULN-185362.
+def _validate_tags_for_command(merged_tags):
+    """
+    Reject tag keys and values that contain characters which are unsafe to interpolate
+    into the 'az' command string. Raises InvalidArgumentValueError on the first offending
+    key or value; returns None when every tag is safe.
+    """
+    for tag_key, tag_value in merged_tags.items():
+        for tag_field in (str(tag_key), str(tag_value)):
+            if any(unsafe_char in tag_field for unsafe_char in ('"', '%', '!')) or \
+                    any(ord(ch) < 32 or ord(ch) == 127 for ch in tag_field):
+                raise InvalidArgumentValueError(
+                    f'Tag keys and values must not contain double quotes, percent signs, '
+                    f'exclamation marks, or control characters. Offending tag: {tag_key}={tag_value}'
+                )
+
+
 def _call_az_command(command_string, run_async=False, secure_params=None):
     """
     Uses subprocess to run a command string. To hide sensitive parameters from logs, add the
@@ -72,18 +127,37 @@ def _call_az_command(command_string, run_async=False, secure_params=None):
     # If command does not start with 'az' then raise exception
     if not tokenized_command or tokenized_command[0] != 'az':
         raise AzCommandError("The command string is not an 'az' command!")
-    # If run on windows, add 'cmd /c'
-    windows_os_name = 'nt'
-    if os.name == windows_os_name:
-        tokenized_command = ['cmd', '/c'] + tokenized_command
 
     # Hide sensitive data such as passwords from logs
     if secure_params:
         for param in secure_params:
             if param:
                 command_string = command_string.replace(param, '********')
     logger.debug("Calling: %s", command_string)
-    process = subprocess.Popen(tokenized_command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True)
+
+    # On Windows, 'az' resolves to a batch file (az.cmd) so the call must be launched
+    # through cmd.exe. Handing the tokenized list to subprocess would let cmd.exe
+    # re-interpret shell metacharacters: subprocess.list2cmdline only quotes tokens that
+    # contain whitespace, so a token such as 'env=ok&echo' would reach cmd.exe unquoted
+    # and the '&' would be parsed as a command separator. To prevent command injection
+    # from untrusted interpolated values (for example source VM tags), build the command
+    # line explicitly and wrap every token in double quotes so cmd.exe treats
+    # metacharacters as literal text.
+    #
+    # The whole command is additionally wrapped in one outer pair of quotes and invoked
+    # with 'cmd /s /c "..."'. Without '/s', cmd.exe strips the first and last quote on the
+    # line (its documented /c behavior), which would unbalance the quoting around the final
+    # argument and re-expose metacharacters. With '/s' and a leading+trailing quote, cmd.exe
+    # strips exactly those outer quotes and parses the remainder verbatim, keeping every
+    # per-token quote balanced. See MSRC 115198 / VULN-185362.
+    windows_os_name = 'nt'
+    if os.name == windows_os_name:
+        quoted_command = ' '.join(_quote_cmd_arg(token) for token in tokenized_command)
+        command_to_run = 'cmd /s /c "' + quoted_command + '"'
+    else:
+        command_to_run = tokenized_command
+
+    process = subprocess.Popen(command_to_run, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True)
 
     # Wait for process to terminate and fetch stdout and stderror
     if not run_async: