Skip to content

Reduce dependency on Invoke-Expression to improve security #529

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
arudell wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Reduce dependency on Invoke-Expression to improve security #529

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions src/modules/SdnDiag.Common.psm1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -795,8 +795,7 @@ function Start-EtwTraceSession {
)

try {
$logmanCmd = "logman create trace $TraceName -ow -o $TraceFile -nb 16 16 -bs 1024 -mode Circular -f bincirc -max $MaxTraceSize -ets"
$result = Invoke-Expression -Command $logmanCmd
$result = & logman create trace $TraceName -ow -o $TraceFile -nb 16 16 -bs 1024 -mode Circular -f bincirc -max $MaxTraceSize -ets

# Session create failure error need to be reported to user to be aware, this means we have one trace session missing.
# Provider add failure might be ignored and exposed via verbose trace/log file only to debug.
Expand All @@ -808,8 +807,7 @@ function Start-EtwTraceSession {
}

foreach ($provider in $TraceProviders) {
$logmanCmd = 'logman update trace $TraceName -p "$provider" 0xffffffffffffffff 0xff -ets'
$result = Invoke-Expression -Command $logmanCmd
$result = & logman Update trace $TraceName -p $provider 0xffffffffffffffff 0xff -ets
"Added provider {0} with result {1}" -f $provider, "$result" | Trace-Output -Level:Verbose
}
}
Expand Down Expand Up @@ -958,8 +956,7 @@ function Stop-EtwTraceSession {
)

try {
$logmanCmd = "logman stop $TraceName -ets"
$result = Invoke-Expression -Command $logmanCmd
$result = & logman stop $TraceName -ets
if ("$result".Contains("Error")) {
"Stop session {0} failed with error {1}" -f $TraceName, "$result" | Trace-Output -Level:Warning
}
Expand Down
4 changes: 2 additions & 2 deletions src/modules/SdnDiag.Gateway.psm1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ function Disable-SdnRasGatewayTracing {

try {
# since there has not been a time when this as returned an error, just invoking the expression and not doing any error handling
Invoke-Expression -Command "netsh ras set tracing * disabled"
& netsh ras set tracing * disabled

Start-Sleep -Seconds 5
$files = Get-Item -Path "$($config.properties.commonPaths.rasGatewayTraces)\*" -Include '*.log','*.etl'
Expand Down Expand Up @@ -155,7 +155,7 @@ function Enable-SdnRasGatewayTracing {
}

# enable ras tracing
$expression = Invoke-Expression -Command "netsh ras set tracing * enabled"
$expression = & netsh ras set tracing * enabled
if($expression -ilike "*Unable to start ETW*"){
$msg = $expression[1]
throw New-Object -TypeName System.Exception($msg)
Expand Down
8 changes: 3 additions & 5 deletions src/modules/SdnDiag.Server.psm1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -351,8 +351,7 @@
throw New-Object System.NullReferenceException("No endpoint listening on port 6641. Ensure NCHostAgent service is running.")
}

$cmdline = "ovsdb-client.exe dump tcp:127.0.0.1:6641 -f json {0}" -f $Table
$databaseResults = Invoke-Expression $cmdline | ConvertFrom-Json
$databaseResults = & ovsdb-client.exe dump tcp:127.0.0.1:6641 -f json $Table | ConvertFrom-Json

if($null -eq $databaseResults){
$msg = "Unable to retrieve OVSDB results`n`t{0}" -f $_
Expand Down Expand Up @@ -480,7 +479,7 @@
$hnvDiag | ForEach-Object {
try {
$cmd = $_
Invoke-Expression -Command $cmd | Export-ObjectToFile -FilePath $outDir -Name $cmd -FileType txt -Format List
& $cmd | Export-ObjectToFile -FilePath $outDir -Name $cmd -FileType txt -Format List
}
catch {
"Failed to execute {0}" -f $cmd | Trace-Output -Level:Error
Expand Down Expand Up @@ -3336,8 +3335,7 @@
# DestinationIP: Destination IP address or direction of the traffic relative to the direction
# DestinationPort: Destination port or direction of the traffic relative to the direction
# flags: 1 = TCP SYN, 2 = Monitoring Ping
$cmd = "vfpctrl /port $PortName /process-tuples '$protocolId $SourceIP $SourcePort $DestinationIP $DestinationPort $Direction 1'"
Invoke-Expression $cmd
& vfpctrl /port $PortName /process-tuples "$protocolID $SourceIP $SourcePort $DestinationIP $DestinationPort $Direction 1"
}
catch {
$_ | Trace-Exception
Expand Down
4 changes: 2 additions & 2 deletions tests/online/RunTests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ param(
$Global:PesterOnlineTests = @{
}

$Global:PesterOnlineTests.ConfigData = [hashtable] (Invoke-Expression (Get-Content -Path $ConfigurationFile | Out-String))
$Global:PesterOnlineTests.ConfigData = [hashtable] (Get-Content -Path $ConfigurationFile | Out-String)

$Global:PesterOnlineTests.NcRestCredential = [System.Management.Automation.PSCredential]::Empty
#$ncAdminCredential = [System.Management.Automation.PSCredential]::Empty
Expand Down Expand Up @@ -37,4 +37,4 @@ if($testResult.Result -ne "Passed")
}

# Exit code 0 indicate success
return $testFailed
return $testFailed