Update powershell sanitizer regex (#1199)

LeftTwixWand · web-flow · commit 6a88be695588 · 2023-10-03T15:54:36.000+02:00
* Updated regex to allow more characters Copy of microsoft/azure-pipelines-tasks#19028 * Bump extension and tasks versions
diff --git a/Extensions/IISWebAppDeploy/Src/Tasks/IISWebAppDeploy/IISWebAppDeployV1/task.json b/Extensions/IISWebAppDeploy/Src/Tasks/IISWebAppDeploy/IISWebAppDeployV1/task.json
@@ -7,7 +7,7 @@
   "version": {
     "Major": 1,
     "Minor": 5,
-    "Patch": 5
+    "Patch": 6
   },
   "demands": [
   ],
diff --git a/Extensions/IISWebAppDeploy/Src/Tasks/IISWebAppDeploy/IISWebAppDeployV2/task.json b/Extensions/IISWebAppDeploy/Src/Tasks/IISWebAppDeploy/IISWebAppDeployV2/task.json
@@ -16,7 +16,7 @@
   "version": {
     "Major": 2,
     "Minor": 1,
-    "Patch": 5
+    "Patch": 6
   },
   "demands": [
   ],
diff --git a/Extensions/IISWebAppDeploy/Src/Tasks/IISWebAppMgmt/IISWebAppMgmtV1/task.json b/Extensions/IISWebAppDeploy/Src/Tasks/IISWebAppMgmt/IISWebAppMgmtV1/task.json
@@ -16,7 +16,7 @@
   "version": {
     "Major": 1,
     "Minor": 4,
-    "Patch": 5
+    "Patch": 6
   },
   "demands": [
   ],
diff --git a/Extensions/IISWebAppDeploy/Src/Tasks/IISWebAppMgmt/IISWebAppMgmtV2/task.json b/Extensions/IISWebAppDeploy/Src/Tasks/IISWebAppMgmt/IISWebAppMgmtV2/task.json
@@ -16,7 +16,7 @@
   "version": {
     "Major": 2,
     "Minor": 2,
-    "Patch": 5
+    "Patch": 6
   },
   "demands": [
   ],
diff --git a/Extensions/IISWebAppDeploy/Src/Tasks/IISWebAppMgmt/IISWebAppMgmtV3/task.json b/Extensions/IISWebAppDeploy/Src/Tasks/IISWebAppMgmt/IISWebAppMgmtV3/task.json
@@ -16,7 +16,7 @@
   "version": {
     "Major": 3,
     "Minor": 1,
-    "Patch": 5
+    "Patch": 6
   },
   "demands": [
   ],
diff --git a/Extensions/IISWebAppDeploy/Src/Tasks/SqlDacpacDeploy/SqlDacpacDeployV1/task.json b/Extensions/IISWebAppDeploy/Src/Tasks/SqlDacpacDeploy/SqlDacpacDeployV1/task.json
@@ -16,7 +16,7 @@
   "version": {
     "Major": 1,
     "Minor": 4,
-    "Patch": 6
+    "Patch": 7
   },
   "demands": [
   ],
diff --git a/Extensions/IISWebAppDeploy/Src/Tasks/SqlDacpacDeploy/SqlDacpacDeployV2/task.json b/Extensions/IISWebAppDeploy/Src/Tasks/SqlDacpacDeploy/SqlDacpacDeployV2/task.json
@@ -16,7 +16,7 @@
   "version": {
     "Major": 2,
     "Minor": 1,
-    "Patch": 6
+    "Patch": 7
   },
   "demands": [
   ],
diff --git a/Extensions/IISWebAppDeploy/Src/vss-extension.json b/Extensions/IISWebAppDeploy/Src/vss-extension.json
@@ -2,7 +2,7 @@
   "manifestVersion": 1,
   "extensionId": "iiswebapp",
   "name": "IIS Web App Deployment Using WinRM",
-  "version": "1.6.7",
+  "version": "1.6.8",
   "publisher": "ms-vscs-rm",
   "description": "Using WinRM connect to the host Computer, to deploy a Web project using Web Deploy or a SQL DB using sqlpackage.exe.",
   "public": true,
diff --git a/TaskModules/powershell/Sanitizer/ArgumentsSanitizer.ps1 b/TaskModules/powershell/Sanitizer/ArgumentsSanitizer.ps1
@@ -60,7 +60,7 @@ function Get-SanitizedArguments([string]$inputArgs) {
 
     # regex rule for removing symbols and telemetry.
     # '?<!`' - checking if before character no backtick. '([allowedchars])' - checking if character is allowed. Otherwise, replace to $removedSymbolSign
-    $regex = '(?<!\\)([^a-zA-Z0-9\\ _''"\-=/:.])';
+    $regex = '(?<!`)([^a-zA-Z0-9\\` _''"\-=\/:\.*,+~?%\n])';
 
     # We're splitting by ``, removing all suspicious characters and then join
     $argsArr = $inputArgs -split $argsSplitSymbols;
@@ -71,8 +71,6 @@ function Get-SanitizedArguments([string]$inputArgs) {
             $matchesChunks += , $matches;
             $argsArr[$i] = $argsArr[$i] -replace $regex, $removedSymbolSign;
         }
-
-        $argsArr[$i] = $argsArr[$i] -replace $regex, $removedSymbolSign;
     }
 
     $resultArgs = $argsArr -join $argsSplitSymbols;
diff --git a/TaskModules/powershell/Sanitizer/Strings/resources.resjson/de-DE/resources.resjson b/TaskModules/powershell/Sanitizer/Strings/resources.resjson/de-DE/resources.resjson
@@ -0,0 +1,2 @@
+{
+}
diff --git a/TaskModules/powershell/Sanitizer/Strings/resources.resjson/en-US/resources.resjson b/TaskModules/powershell/Sanitizer/Strings/resources.resjson/en-US/resources.resjson
@@ -1,4 +1,4 @@
 {
-  "loc.messages.PS_ScriptArgsSanitized": "Detected characters in arguments that may not be executed correctly by the shell.  Please escape special characters using backtick (`). More information is available here: <https://aka.ms/ado/75787>",
+  "loc.messages.PS_ScriptArgsSanitized": "Detected characters in arguments that may not be executed correctly by the shell.  Please escape special characters using backtick (`). More information is available here: https://aka.ms/ado/75787",
   "loc.messages.PS_ScriptArgsNotSanitized": "Arguments passed sanitization without change."
 }
diff --git a/TaskModules/powershell/Sanitizer/Strings/resources.resjson/es-ES/resources.resjson b/TaskModules/powershell/Sanitizer/Strings/resources.resjson/es-ES/resources.resjson
@@ -0,0 +1,2 @@
+{
+}
diff --git a/TaskModules/powershell/Sanitizer/Strings/resources.resjson/fr-FR/resources.resjson b/TaskModules/powershell/Sanitizer/Strings/resources.resjson/fr-FR/resources.resjson
@@ -0,0 +1,2 @@
+{
+}
diff --git a/TaskModules/powershell/Sanitizer/Strings/resources.resjson/it-IT/resources.resjson b/TaskModules/powershell/Sanitizer/Strings/resources.resjson/it-IT/resources.resjson
@@ -0,0 +1,2 @@
+{
+}
diff --git a/TaskModules/powershell/Sanitizer/Strings/resources.resjson/ja-JP/resources.resjson b/TaskModules/powershell/Sanitizer/Strings/resources.resjson/ja-JP/resources.resjson
@@ -0,0 +1,2 @@
+{
+}
diff --git a/TaskModules/powershell/Sanitizer/Strings/resources.resjson/ko-KR/resources.resjson b/TaskModules/powershell/Sanitizer/Strings/resources.resjson/ko-KR/resources.resjson
@@ -0,0 +1,2 @@
+{
+}
diff --git a/TaskModules/powershell/Sanitizer/Strings/resources.resjson/ru-RU/resources.resjson b/TaskModules/powershell/Sanitizer/Strings/resources.resjson/ru-RU/resources.resjson
@@ -0,0 +1,2 @@
+{
+}
diff --git a/TaskModules/powershell/Sanitizer/Strings/resources.resjson/zh-CN/resources.resjson b/TaskModules/powershell/Sanitizer/Strings/resources.resjson/zh-CN/resources.resjson
@@ -0,0 +1,2 @@
+{
+}
diff --git a/TaskModules/powershell/Sanitizer/Strings/resources.resjson/zh-TW/resources.resjson b/TaskModules/powershell/Sanitizer/Strings/resources.resjson/zh-TW/resources.resjson
@@ -0,0 +1,2 @@
+{
+}
diff --git a/TaskModules/powershell/Sanitizer/Tests/L0.ts b/TaskModules/powershell/Sanitizer/Tests/L0.ts
@@ -1,12 +1,9 @@
 /// <reference path="../../../../definitions/mocha.d.ts"/>
 /// <reference path="../../../../definitions/node.d.ts"/>
-/// <reference path="../../../../definitions/Q.d.ts"/>
 
-import Q = require('q');
-import assert = require('assert');
 import path = require('path');
 
-var psm = require('../../../Tests/lib/psRunner');
+var psm = require('../../../../Tests/lib/psRunner');
 var psr = null;
 
 describe('Security Suite', function () {
@@ -49,4 +46,16 @@ describe('Security Suite', function () {
             psr.run(path.join(__dirname, 'L0Get-SanitizedArgumentsArray.DoesNotBreakExistingBashFormats.ps1'), done);
         });
     }
-});
+
+    if (psm.testSupported()) {
+        it('Protect-ScriptArguments should pass correctly', (done) => {
+            psr.run(path.join(__dirname, 'L0Protect-ScriptArguments.Passes.ps1'), done);
+        });
+    }
+
+    if (psm.testSupported()) {
+        it('Protect-ScriptArguments should throw', (done) => {
+            psr.run(path.join(__dirname, 'L0Protect-ScriptArguments.Throws.ps1'), done);
+        });
+    }
+});
diff --git a/TaskModules/powershell/Sanitizer/Tests/L0Get-SanitizedArgumentsArray.DoesNotBreakExistingBashFormats.ps1 b/TaskModules/powershell/Sanitizer/Tests/L0Get-SanitizedArgumentsArray.DoesNotBreakExistingBashFormats.ps1
@@ -21,5 +21,5 @@ foreach ($argument in $bashArgumentsFormats) {
     $sanitizedArguments = Get-SanitizedArguments -InputArgs $argument
 
     # Assert
-    Assert-AreEqual $sanitizedArguments $argument
-}
+    Assert-AreEqual -Actual $sanitizedArguments -Expected $argument
+}
diff --git a/TaskModules/powershell/Sanitizer/Tests/L0Get-SanitizedArgumentsArray.DoesNotBreakExistingCmdFormats.ps1 b/TaskModules/powershell/Sanitizer/Tests/L0Get-SanitizedArgumentsArray.DoesNotBreakExistingCmdFormats.ps1
@@ -20,5 +20,5 @@ foreach ($argument in $cmdArgumentsFormats) {
     $sanitizedArguments = Get-SanitizedArguments -InputArgs $argument
 
     # Assert
-    Assert-AreEqual $sanitizedArguments $argument
+    Assert-AreEqual -Actual $sanitizedArguments -Expected $argument
 }
diff --git a/TaskModules/powershell/Sanitizer/Tests/L0Get-SanitizedArgumentsArray.DoesNotBreakExistingPowerShellFormats.ps1 b/TaskModules/powershell/Sanitizer/Tests/L0Get-SanitizedArgumentsArray.DoesNotBreakExistingPowerShellFormats.ps1
@@ -21,5 +21,5 @@ foreach ($argument in $powershellArgumentsFormats) {
     $sanitizedArguments = Get-SanitizedArguments -InputArgs $argument
 
     # Assert
-    Assert-AreEqual $sanitizedArguments $argument
+    Assert-AreEqual -Actual $sanitizedArguments -Expected $argument
 }
diff --git a/TaskModules/powershell/Sanitizer/Tests/L0Get-SanitizedArgumentsArray.ReplacesForbiddenCharacters.ps1 b/TaskModules/powershell/Sanitizer/Tests/L0Get-SanitizedArgumentsArray.ReplacesForbiddenCharacters.ps1
@@ -14,5 +14,5 @@ $sanitizedArguments = Get-SanitizedArguments -InputArgs $arguments
 
 # Assert
 
-# We need to use $sanitizedArguments[1] because $sanitizedArguments contains buffer with Write-Output message from the function execution. 
-Assert-AreEqual $sanitizedArguments[1] "start notepad.exe _#removed#_ echo 'hello' _#removed#_ calc.exe"
+# We need to use $sanitizedArguments[1] because $sanitizedArguments contains buffer with Write-Output message from the function execution.
+Assert-AreEqual -Expected "start notepad.exe _#removed#_ echo 'hello' _#removed#_ calc.exe" -Actual $sanitizedArguments
diff --git a/TaskModules/powershell/Sanitizer/Tests/L0Protect-ScriptArguments.Passes.ps1 b/TaskModules/powershell/Sanitizer/Tests/L0Protect-ScriptArguments.Passes.ps1
@@ -0,0 +1,41 @@
+[CmdletBinding()]
+param()
+
+Set-Item -Path env:AZP_75787_ENABLE_NEW_LOGIC -Value 'true'
+
+. $PSScriptRoot\..\..\..\..\Tests\lib\Initialize-Test.ps1
+. $PSScriptRoot\..\ArgumentsSanitizer.ps1
+
+$inputArgsSuites = @(
+    "/parameter",                               # Traditional way to pass parameters in CMD
+    "-parameter",                               # Modern applications accept parameters with a hyphen
+    "--parameter",                              # Many modern applications accept parameters with double hyphen
+    "parameter=value",                          # Format for passing values to parameters
+    "parameter value.txt",                      # Argument with dot in the middle
+    "-parameter",                               # Single hyphen followed by a single letter or digit (POSIX style)
+    "-parameter value",                         # When the parameter needs a value
+    "--parameter",                              # Double hyphen followed by a word (GNU style)
+    "--parameter=value",                        # Value directly attached to the parameter with an equals sign
+    "parameter=value",                          # Used to pass environment variables to a command
+    "parameter value.txt",                      # Argument with dot in the middle
+    "-Parameter Value",                         # Most common form
+    "-Parameter:Value",                         # Colon connects the parameter and its value
+    "/p:Parameter=Value",                       # Specific syntax for tools like MSBuild or NuGet
+    "--Parameter Value",                        # Used by cmdlets or scripts for cross-platform compatibility
+    "--Parameter=Value",                        # Used by cross-platform tools
+    "parameter value.txt"                       # Argument with dot in the middle
+    'a A 1 \ ` _ '' " - = / : . * , + ~ ? %',   # Just each allowed symbol
+    '',
+    'test 1',
+    'test `; whoami `&`& echo test',
+    "line 1 `n line 2"
+)
+
+foreach ($inputArgs in $inputArgsSuites) {
+    try {
+        Protect-ScriptArguments $inputArgs
+    }
+    catch {
+        throw "Error occured with '$inputArgs' input args: $($_.Exception.Message)"
+    }
+}
diff --git a/TaskModules/powershell/Sanitizer/Tests/L0Protect-ScriptArguments.Throws.ps1 b/TaskModules/powershell/Sanitizer/Tests/L0Protect-ScriptArguments.Throws.ps1
@@ -0,0 +1,22 @@
+[CmdletBinding()]
+param()
+
+Set-Item -Path env:AZP_75787_ENABLE_NEW_LOGIC -Value 'true'
+
+. $PSScriptRoot\..\..\..\..\Tests\lib\Initialize-Test.ps1
+. $PSScriptRoot\..\ArgumentsSanitizer.ps1
+
+$inputArgsSuites = @(
+    'test; whoami',
+    'test && whoami',
+    'echo "$(rm ./somedir)"',
+    'test | whoami'
+)
+
+$expectedMsg = Get-VstsLocString -Key 'PS_ScriptArgsSanitized'
+
+foreach ($inputArgs in $inputArgsSuites) {
+    Assert-Throws {
+        Protect-ScriptArguments $inputArgs
+    } -MessagePattern $expectedMsg
+}
diff --git a/TaskModules/powershell/Sanitizer/Tests/L0Split-Arguments.ps1 b/TaskModules/powershell/Sanitizer/Tests/L0Split-Arguments.ps1
@@ -33,5 +33,5 @@ for ($i = 0; $i -lt $argumentsFormats.Length; $i++) {
     [string[]]$splitArguments = Split-Arguments -arguments $argumentsFormats[$i]
 
     # Assert
-    Assert-AreEqual $splitArguments $expectedOutputs[$i]
-}
+    Assert-AreEqual -Expected $splitArguments -Actual $expectedOutputs[$i]
+}
diff --git a/TaskModules/powershell/Sanitizer/module.json b/TaskModules/powershell/Sanitizer/module.json
@@ -1,6 +1,6 @@
 {
     "messages": {
-        "PS_ScriptArgsSanitized": "Detected characters in arguments that may not be executed correctly by the shell.  Please escape special characters using backtick (`). More information is available here: <https://aka.ms/ado/75787>",
+        "PS_ScriptArgsSanitized": "Detected characters in arguments that may not be executed correctly by the shell.  Please escape special characters using backtick (`). More information is available here: https://aka.ms/ado/75787",
         "PS_ScriptArgsNotSanitized": "Arguments passed sanitization without change."
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -60,7 +60,7 @@ function Get-SanitizedArguments([string]$inputArgs) {`
`60`	`60`
`61`	`61`	`# regex rule for removing symbols and telemetry.`
`62`	`62`	# '?<!`' - checking if before character no backtick. '([allowedchars])' - checking if character is allowed. Otherwise, replace to $removedSymbolSign
`63`		`- $regex = '(?<!\\)([^a-zA-Z0-9\\ _''"\-=/:.])';`
	`63`	+ $regex = '(?<!`)([^a-zA-Z0-9\\` _''"\-=\/:\.*,+~?%\n])';
`64`	`64`
`65`	`65`	# We're splitting by ``, removing all suspicious characters and then join
`66`	`66`	`$argsArr = $inputArgs -split $argsSplitSymbols;`
`@@ -71,8 +71,6 @@ function Get-SanitizedArguments([string]$inputArgs) {`
`71`	`71`	`$matchesChunks += , $matches;`
`72`	`72`	`$argsArr[$i] = $argsArr[$i] -replace $regex, $removedSymbolSign;`
`73`	`73`	`}`
`74`		`-`
`75`		`- $argsArr[$i] = $argsArr[$i] -replace $regex, $removedSymbolSign;`
`76`	`74`	`}`
`77`	`75`
`78`	`76`	`$resultArgs = $argsArr -join $argsSplitSymbols;`