Skip to content

Custom rules that don't supply optional properties cause null dereference #1714

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.

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

Closed
rjmholt opened this issue Sep 16, 2021 · 2 comments · Fixed by #1715
Closed

Custom rules that don't supply optional properties cause null dereference #1714

rjmholt opened this issue Sep 16, 2021 · 2 comments · Fixed by #1715

Comments

@rjmholt
Copy link
Contributor

rjmholt commented Sep 16, 2021

Before submitting a bug report:

  • Make sure you are able to repro it on the latest released version
  • Perform a quick search for existing issues to check if this bug has already been reported

Steps to reproduce

Custom rule module:

<#
.SYNOPSIS
    Static methods are not allowed in constrained language mode.
.DESCRIPTION
    Static methods are not allowed in constrained language mode.
    To fix a violation of this rule, use a cmdlet or function instead of a static method.
.EXAMPLE
    Test-StaticMethod -CommandAst $CommandAst
.INPUTS
    [System.Management.Automation.Language.ScriptBlockAst]
.OUTPUTS
    [PSCustomObject[]]
.NOTES
    Reference: Output, CLM info.
#>
function Test-StaticMethod
{
    [CmdletBinding()]
    [OutputType([PSCustomObject[]])]
    Param
    (
        [Parameter(Mandatory = $true)]
        [ValidateNotNullOrEmpty()]
        [System.Management.Automation.Language.ScriptBlockAst]
        $ScriptBlockAst
    )

    Process
    {
        try
        {
            # Gets methods
        
            $invokedMethods = $ScriptBlockAst.FindAll({$args[0] -is [System.Management.Automation.Language.CommandExpressionAst] -and $args[0].Expression -match "^\[.*\]::" },$true)
            foreach ($invokedMethod in $invokedMethods)
            {
                [PSCustomObject]@{Message  = Avoid Using Static Methods;
                                  Extent   = $invokedMethod.Extent;
                                  RuleName = $PSCmdlet.MyInvocation.InvocationName;
                                  Severity = Warning}
            }
        }
        catch
        {
            $PSCmdlet.ThrowTerminatingError($PSItem)
        }
    }
}

Target script:

write-host "test"
$a = "asdf"
$a = $a.replace('s','ssssssss')
[math]::abs(-1)
Function ASDF1234{
    "asdf"
} 

Script analyzer invocation:

Invoke-ScriptAnalyzer -Path 'C:\<path>\CLMTest.ps1' -CustomRulePath 'C:\<path>\MyCustom.psm1' 

Expected behavior


RuleName                            Severity     ScriptName Line  Message
--------                            --------     ---------- ----  -------
MyCustom\Test-StaticMethod          Warning      CLMTest.ps 4     Avoid Using Static Methods                                        

Actual behavior

Invoke-ScriptAnalyzer: Object reference not set to an instance of an object.

Exception             :
    Type       : System.NullReferenceException
    TargetSite :
        Name          : GetExternalRecord
        DeclaringType : Microsoft.Windows.PowerShell.ScriptAnalyzer.ScriptAnalyzer
        MemberType    : Method
        Module        : Microsoft.Windows.PowerShell.ScriptAnalyzer.dll
    Message    : Object reference not set to an instance of an object.
    Source     : Microsoft.Windows.PowerShell.ScriptAnalyzer
    HResult    : -2147467261
    StackTrace :
   at Microsoft.Windows.PowerShell.ScriptAnalyzer.ScriptAnalyzer.GetExternalRecord(Ast ast, Token[]
token, ExternalRule[] rules, String filePath) in C:\Users\Robert
Holt\Documents\Dev\Microsoft\PSScriptAnalyzer\Engine\ScriptAnalyzer.cs:line 1291
TargetObject          : Microsoft.Windows.PowerShell.ScriptAnalyzer.ScriptAnalyzer
CategoryInfo          : NotSpecified: (Microsoft.Windows.P…yzer.ScriptAnalyzer:ScriptAnalyzer)
[Invoke-ScriptAnalyzer], NullReferenceException
FullyQualifiedErrorId :
80004003,Microsoft.Windows.PowerShell.ScriptAnalyzer.Commands.InvokeScriptAnalyzerCommand
InvocationInfo        :
    MyCommand        : Invoke-ScriptAnalyzer
    ScriptLineNumber : 1
    OffsetInLine     : 1
    HistoryId        : 9
    Line             : Invoke-ScriptAnalyzer -Path .\CLMTest.ps1 -CustomRulePath .\MyCustom.psm1
    PositionMessage  : At line:1 char:1
                       + Invoke-ScriptAnalyzer -Path .\CLMTest.ps1 -CustomRulePath .\MyCustom. …
                       + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    InvocationName   : Invoke-ScriptAnalyzer
    CommandOrigin    : Internal
ScriptStackTrace      : at <ScriptBlock>, <No file>: line 1
PipelineIterationInfo :

The issue lies here:

severity = (DiagnosticSeverity)Enum.Parse(typeof(DiagnosticSeverity), psobject.Properties["Severity"].Value.ToString());
message = psobject.Properties["Message"].Value.ToString();
extent = (IScriptExtent)psobject.Properties["Extent"].Value;
ruleName = psobject.Properties["RuleName"].Value.ToString();
ruleSuppressionID = psobject.Properties["RuleSuppressionID"].Value?.ToString();
suggestedCorrections = (IEnumerable<CorrectionExtent>)psobject.Properties["SuggestedCorrections"].Value;

When properties are not in the returned object, they surface as null and we need to defend against that.

@ghost ghost added the Needs: Triage 🔍 label Sep 16, 2021
@rjmholt
Copy link
Contributor Author

rjmholt commented Sep 16, 2021

Curiously this code hasn't changed in 6 years, which makes me wonder whether a change has occurred in PowerShell instead...

@rjmholt
Copy link
Contributor Author

rjmholt commented Sep 16, 2021

Curiously this code hasn't changed in 6 years, which makes me wonder whether a change has occurred in PowerShell instead...

Reproduces in Windows PowerShell as well, so looks like no

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant