Skip to content

Commit

Permalink
Updated regex to allow more characters
Browse files Browse the repository at this point in the history
  • Loading branch information
LeftTwixWand committed Oct 3, 2023
1 parent 70c2754 commit 633e310
Show file tree
Hide file tree
Showing 20 changed files with 106 additions and 18 deletions.
4 changes: 1 addition & 3 deletions TaskModules/powershell/Sanitizer/ArgumentsSanitizer.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
{
}
Original file line number Diff line number Diff line change
@@ -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."
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
{
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
{
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
{
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
{
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
{
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
{
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
{
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
{
}
19 changes: 14 additions & 5 deletions TaskModules/powershell/Sanitizer/Tests/L0.ts
Original file line number Diff line number Diff line change
@@ -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 () {
Expand Down Expand Up @@ -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);
});
}
});
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,5 @@ foreach ($argument in $bashArgumentsFormats) {
$sanitizedArguments = Get-SanitizedArguments -InputArgs $argument

# Assert
Assert-AreEqual $sanitizedArguments $argument
}
Assert-AreEqual -Actual $sanitizedArguments -Expected $argument
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,5 @@ foreach ($argument in $cmdArgumentsFormats) {
$sanitizedArguments = Get-SanitizedArguments -InputArgs $argument

# Assert
Assert-AreEqual $sanitizedArguments $argument
Assert-AreEqual -Actual $sanitizedArguments -Expected $argument
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,5 @@ foreach ($argument in $powershellArgumentsFormats) {
$sanitizedArguments = Get-SanitizedArguments -InputArgs $argument

# Assert
Assert-AreEqual $sanitizedArguments $argument
Assert-AreEqual -Actual $sanitizedArguments -Expected $argument
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
@@ -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)"
}
}
Original file line number Diff line number Diff line change
@@ -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
}
4 changes: 2 additions & 2 deletions TaskModules/powershell/Sanitizer/Tests/L0Split-Arguments.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -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]
}
2 changes: 1 addition & 1 deletion TaskModules/powershell/Sanitizer/module.json
Original file line number Diff line number Diff line change
@@ -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."
}
}

0 comments on commit 633e310

Please sign in to comment.