Common/Sanitizer: block data-constructor RCE via AST backstop (issue #22173, supersedes #22253)

Tasks/AzureFileCopyV1/task.json

@@ -14,7 +14,7 @@
     "version": {
         "Major": 1,
         "Minor": 276,
-        "Patch": 0
+        "Patch": 1
     },
     "demands": [
         "azureps"

Tasks/AzureFileCopyV1/task.loc.json

@@ -14,7 +14,7 @@
   "version": {
     "Major": 1,
     "Minor": 276,
-    "Patch": 0
+    "Patch": 1
   },
   "demands": [
     "azureps"

Tasks/AzureFileCopyV2/task.json

@@ -14,7 +14,7 @@
   "version": {
     "Major": 2,
     "Minor": 276,
-    "Patch": 0
+    "Patch": 1
   },
   "demands": [
     "azureps"

Tasks/AzureFileCopyV2/task.loc.json

@@ -14,7 +14,7 @@
   "version": {
     "Major": 2,
     "Minor": 276,
-    "Patch": 0
+    "Patch": 1
   },
   "demands": [
     "azureps"

Tasks/AzureFileCopyV3/task.json

@@ -14,7 +14,7 @@
   "version": {
     "Major": 3,
     "Minor": 276,
-    "Patch": 0
+    "Patch": 1
   },
   "demands": [
     "azureps"

Tasks/AzureFileCopyV3/task.loc.json

@@ -14,7 +14,7 @@
   "version": {
     "Major": 3,
     "Minor": 276,
-    "Patch": 0
+    "Patch": 1
   },
   "demands": [
     "azureps"

Tasks/AzureFileCopyV4/task.json

@@ -14,7 +14,7 @@
   "version": {
     "Major": 4,
     "Minor": 276,
-    "Patch": 0
+    "Patch": 2
   },
   "demands": [
     "azureps"

Tasks/AzureFileCopyV4/task.loc.json

@@ -14,7 +14,7 @@
   "version": {
     "Major": 4,
     "Minor": 276,
-    "Patch": 0
+    "Patch": 2
   },
   "demands": [
     "azureps"

Tasks/AzureFileCopyV5/task.json

@@ -14,7 +14,7 @@
   "version": {
     "Major": 5,
     "Minor": 276,
-    "Patch": 0
+    "Patch": 2
   },
   "demands": [
     "azureps"

Tasks/AzureFileCopyV5/task.loc.json

@@ -14,7 +14,7 @@
   "version": {
     "Major": 5,
     "Minor": 276,
-    "Patch": 0
+    "Patch": 2
   },
   "demands": [
     "azureps"

Tasks/AzureFileCopyV6/task.json

@@ -14,7 +14,7 @@
   "version": {
     "Major": 6,
     "Minor": 276,
-    "Patch": 0
+    "Patch": 2
   },
   "demands": [
     "azureps"

Tasks/AzureFileCopyV6/task.loc.json

@@ -14,7 +14,7 @@
   "version": {
     "Major": 6,
     "Minor": 276,
-    "Patch": 0
+    "Patch": 2
   },
   "demands": [
     "azureps"

Tasks/AzurePowerShellV2/task.json

@@ -18,7 +18,7 @@
     "version": {
         "Major": 2,
         "Minor": 276,
-        "Patch": 0
+        "Patch": 1
     },
     "demands": [
         "azureps"

Tasks/AzurePowerShellV2/task.loc.json

@@ -18,7 +18,7 @@
   "version": {
     "Major": 2,
     "Minor": 276,
-    "Patch": 0
+    "Patch": 1
   },
   "demands": [
     "azureps"

Tasks/AzurePowerShellV3/task.json

@@ -18,7 +18,7 @@
     "version": {
         "Major": 3,
         "Minor": 276,
-        "Patch": 0
+        "Patch": 1
     },
     "releaseNotes": "Added support for Fail on standard error and ErrorActionPreference",
     "demands": [

Tasks/AzurePowerShellV3/task.loc.json

@@ -18,7 +18,7 @@
   "version": {
     "Major": 3,
     "Minor": 276,
-    "Patch": 0
+    "Patch": 1
   },
   "releaseNotes": "ms-resource:loc.releaseNotes",
   "demands": [

Tasks/AzurePowerShellV4/task.json

@@ -18,7 +18,7 @@
   "version": {
     "Major": 4,
     "Minor": 276,
-    "Patch": 0
+    "Patch": 2
   },
   "releaseNotes": "Added support for Az Module and cross platform agents.",
   "groups": [

Tasks/AzurePowerShellV4/task.loc.json

@@ -18,7 +18,7 @@
   "version": {
     "Major": 4,
     "Minor": 276,
-    "Patch": 0
+    "Patch": 2
   },
   "releaseNotes": "ms-resource:loc.releaseNotes",
   "groups": [

Tasks/AzurePowerShellV5/task.json

@@ -18,7 +18,7 @@
   "version": {
     "Major": 5,
     "Minor": 276,
-    "Patch": 0
+    "Patch": 2
   },
   "releaseNotes": "Added support for Az Module and cross platform agents.",
   "groups": [

Tasks/AzurePowerShellV5/task.loc.json

@@ -18,7 +18,7 @@
   "version": {
     "Major": 5,
     "Minor": 276,
-    "Patch": 0
+    "Patch": 2
   },
   "releaseNotes": "ms-resource:loc.releaseNotes",
   "groups": [

Tasks/Common/Sanitizer/ArgumentsSanitizer.ps1

@@ -30,14 +30,26 @@ function Get-SanitizerActivateStatus {
 
 # This is a wrapper for Get-SanitizedArguments to handle feature flags in one place
 # It will return sanitized arguments string if feature flag is enabled
-function Protect-ScriptArguments([string]$inputArgs, [string]$taskName) {
+function Protect-ScriptArguments([string]$inputArgs, [string]$taskName, [switch]$AllowDataConstructors) {
     $script:taskName = $taskName
 
+    # When data constructors are permitted, run the structural AST backstop on the
+    # RAW arguments first. This module only validates - it does not rewrite what the
+    # task runs - so the raw string is exactly what PowerShell parses at the
+    # dot-source sink. The relaxed character allow-list intentionally permits
+    # @ { } [ ], which re-enables expressions that evaluate at bind time (a hashtable
+    # value, array element, cast or sub-expression). Test-SanitizerArgumentAst
+    # rejects those while still allowing pure data literals such as @{ Port = 8080 }.
+    $astSafe = $true
+    if ($AllowDataConstructors) {
+        $astSafe = Test-SanitizerArgumentAst $inputArgs
+    }
+
     $expandedArgs, $expandTelemetry = Expand-EnvVariables $inputArgs;
 
-    $sanitizedArgs, $sanitizeTelemetry = Get-SanitizedArguments -InputArgs $expandedArgs
+    $sanitizedArgs, $sanitizeTelemetry = Get-SanitizedArguments -InputArgs $expandedArgs -AllowDataConstructors:$AllowDataConstructors
 
-    if ($sanitizedArgs -eq $inputArgs) {
+    if (($sanitizedArgs -eq $inputArgs) -and $astSafe) {
         Write-Debug 'Arguments passed sanitization without change.'
     }
     else {
@@ -46,10 +58,16 @@ function Protect-ScriptArguments([string]$inputArgs, [string]$taskName) {
             if ($null -ne $sanitizeTelemetry) {
                 $telemetry += $sanitizeTelemetry;
             }
+            if (-not $astSafe) {
+                if ($null -eq $telemetry) {
+                    $telemetry = @{}
+                }
+                $telemetry.astBackstopRejected = $true
+            }
             Publish-Telemetry $telemetry;
         }
 
-        if ($sanitizedArgs -ne $expandedArgs) {
+        if (($sanitizedArgs -ne $expandedArgs) -or (-not $astSafe)) {
             $message = (Get-VstsLocString -Key 'PS_ScriptArgsSanitized');
 
             if ($featureFlags.activate) {
@@ -69,7 +87,7 @@ function Protect-ScriptArguments([string]$inputArgs, [string]$taskName) {
 # public functions - end
 
 # !ATTENTION: don't write any console output in this method, because it will break result
-function Get-SanitizedArguments([string]$inputArgs) {
+function Get-SanitizedArguments([string]$inputArgs, [switch]$AllowDataConstructors) {
     $removedSymbolSign = '_#removed#_';
     $argsSplitSymbols = '``';
     [string[][]]$matchesChunks = @()
@@ -78,7 +96,18 @@ function Get-SanitizedArguments([string]$inputArgs) {
     ## ('?<!`') - checking if before character no backtick.
     ## ([^\w` _'"-=\/:\.*,+~?%\n#]) - checking if character is allowed. Insead replacing to #removed#
     ## (?!true|false) - checking if after characters sequence no $true or $false.
-    $regex = '(?<!`)([^\w\\` _''"\-=\/:\.*,+~?%\n#])(?!true|false)'
+    ##
+    ## When -AllowDataConstructors is set (Group A tasks, via the dispatcher) the
+    ## data-constructor characters @ { } [ ] are additionally allowed so legitimate
+    ## hashtable / array arguments are not mangled (regression issue #22173). The
+    ## code execution those characters could otherwise re-enable (e.g. @{ k = cmd })
+    ## is blocked structurally by Test-SanitizerArgumentAst, not by this allow-list.
+    if ($AllowDataConstructors) {
+        $regex = '(?<!`)([^\w\\` _''"\-=\/:\.*,+~?%\n#@{}\[\]])(?!true|false)'
+    }
+    else {
+        $regex = '(?<!`)([^\w\\` _''"\-=\/:\.*,+~?%\n#])(?!true|false)'
+    }
 
     # We're splitting by ``, removing all suspicious characters and then join
     $argsArr = $inputArgs -split $argsSplitSymbols;
@@ -105,6 +134,85 @@ function Get-SanitizedArguments([string]$inputArgs) {
     return $($resultArgs, $telemetry);
 }
 
+# Structural backstop for the relaxed (-AllowDataConstructors) path.
+#
+# A character allow-list alone cannot tell a data literal from code: once
+# @ { } [ ] are permitted, an argument such as @{ k = New-Item ... }, @( cmd ),
+# @{ k = $(...) } or @{ k = [type]::Member() } passes the regex yet is an
+# *evaluated expression* at the dot-source sink. This was verified empirically
+# against the real '. <script> <args>' / '& <script> <args>' sinks: a hashtable
+# value, array element, sub-expression or cast *inside* a data constructor runs,
+# whereas the same tokens at top-level argument position are inert literal
+# strings.
+#
+# This function parses the raw arguments exactly as the sink does - as the
+# argument list of a command invocation - and rejects anything that is not a
+# plain data literal:
+#   * a parse error,
+#   * a script block, member access / method call, type-cast, the -as conversion
+#     operator, or a bare type reference,
+#   * a nested command (more than the single placeholder CommandAst), which
+#     covers commands embedded in a hashtable value, array element, or a
+#     chained statement.
+# Pure data literals (@{ Port = 8080 }, @('a','b')), variables including
+# $env:VAR, quoted strings and numbers are accepted.
+#
+# Returns $true when the arguments are safe, $false when a dangerous construct
+# is present.
+function Test-SanitizerArgumentAst([string]$inputArgs) {
+    if ([string]::IsNullOrWhiteSpace($inputArgs)) {
+        return $true
+    }
+
+    $tokens = $null
+    $parseErrors = $null
+    # A literal placeholder command name keeps the parse focused on the argument
+    # expressions and mirrors how the arguments reach the sink.
+    $ast = [System.Management.Automation.Language.Parser]::ParseInput(
+        "& placeholder $inputArgs", [ref]$tokens, [ref]$parseErrors)
+
+    if ($parseErrors -and $parseErrors.Count -gt 0) {
+        return $false
+    }
+
+    # InvokeMemberExpressionAst derives from MemberExpressionAst, so the single
+    # MemberExpressionAst check covers both property getters and method calls.
+    # ConvertExpressionAst is the [type]$x / [type]'x' cast; the -as conversion
+    # operator (a BinaryExpressionAst with the 'As' operator) is the semantically
+    # equivalent form and likewise invokes the target type's constructor /
+    # type-converter at the sink - verified to execute with both a [type] literal
+    # and a string/variable right operand - so it must be rejected too. A bare
+    # TypeExpressionAst (a type reference used as a value inside a data constructor)
+    # is never needed in pure data and is blocked for good measure; top-level type
+    # literals passed as plain arguments do not parse as TypeExpressionAst and
+    # remain allowed.
+    $dangerous = $ast.FindAll({
+            param($node)
+            ($node -is [System.Management.Automation.Language.ScriptBlockExpressionAst]) -or
+            ($node -is [System.Management.Automation.Language.MemberExpressionAst]) -or
+            ($node -is [System.Management.Automation.Language.ConvertExpressionAst]) -or
+            ($node -is [System.Management.Automation.Language.TypeExpressionAst]) -or
+            (($node -is [System.Management.Automation.Language.BinaryExpressionAst]) -and
+             ($node.Operator -eq [System.Management.Automation.Language.TokenKind]::As))
+        }, $true)
+    if ($dangerous -and $dangerous.Count -gt 0) {
+        return $false
+    }
+
+    # Exactly one CommandAst is expected - our placeholder. Any additional
+    # CommandAst means a command nested inside a data constructor or a chained
+    # statement.
+    $commandAsts = $ast.FindAll({
+            param($node)
+            $node -is [System.Management.Automation.Language.CommandAst]
+        }, $true)
+    if ($commandAsts.Count -gt 1) {
+        return $false
+    }
+
+    return $true
+}
+
 function Publish-Telemetry($telemetry) {
     $area = 'TaskHub'
     $feature = $script:taskName

Tasks/Common/Sanitizer/Invoke-ScriptArgumentSanitization.ps1

@@ -129,7 +129,14 @@ function Invoke-ScriptArgumentSanitization {
     $caughtMessage  = $null
     $caughtStack    = $null
     try {
-        $null = Protect-ScriptArguments -InputArgs $InputArgs -TaskName $TaskName
+        # The dispatcher is the opt-in entry point for the new per-task sanitizer
+        # consumers (AzurePowerShellV2-V5, ServiceFabricPowerShellV1). They run
+        # user FilePath scripts whose arguments legitimately include hashtables
+        # and arrays, so data constructors are permitted here; the AST backstop
+        # inside Protect-ScriptArguments blocks any that would execute. Legacy
+        # direct callers (AzureFileCopy, PowerShell, etc.) keep the strict
+        # character allow-list by not passing this switch.
+        $null = Protect-ScriptArguments -InputArgs $InputArgs -TaskName $TaskName -AllowDataConstructors
     }
     catch {
         $sanitizerThrew = $true

Tasks/Common/Sanitizer/Tests/L0.ts

@@ -70,4 +70,28 @@ describe('Security Suite', function () {
             psr.run(path.join(__dirname, 'L0Invoke-ScriptArgumentSanitization.ps1'), done);
         });
     }
+
+    if (psm.testSupported()) {
+        it('Test-SanitizerArgumentAst classifies data literals vs. executable expressions', (done) => {
+            psr.run(path.join(__dirname, 'L0Test-SanitizerArgumentAst.ps1'), done);
+        });
+    }
+
+    if (psm.testSupported()) {
+        it('Protect-ScriptArguments allows legitimate data constructors on the relaxed path', (done) => {
+            psr.run(path.join(__dirname, 'L0Protect-ScriptArguments.AllowsDataConstructors.ps1'), done);
+        });
+    }
+
+    if (psm.testSupported()) {
+        it('Protect-ScriptArguments blocks data-constructor injection via the AST backstop', (done) => {
+            psr.run(path.join(__dirname, 'L0Protect-ScriptArguments.BlocksDataConstructorInjection.ps1'), done);
+        });
+    }
+
+    if (psm.testSupported()) {
+        it('Get-SanitizedArguments keeps the strict path unchanged for legacy callers', (done) => {
+            psr.run(path.join(__dirname, 'L0Get-SanitizedArguments.GroupBIsolation.ps1'), done);
+        });
+    }
 });