DevSkim icon indicating copy to clipboard operation
DevSkim copied to clipboard

Review Devskim: Finding DS104456(DS104456) for appropriate severity level

Open klawrawkz opened this issue 4 years ago • 2 comments

Hi, my name is klawrawkz. I'm fine. How are you?

In VS Code, I'm plugging away at a Powershell IAC automation routine. I need to invoke commands on various VM resources I'm working with. Thus I write a function, e.g.

Invoke-Command -ComputerName $compName, - ScriptBlock {
       mySwellScript.vbs /reallyAwesomeSwitch
       net accounts /maxpwage:unlimited
       Restart-Computer -Force
      ... continue on with 
      ... this terrific secret sauce of awesomeness 
} -Credential $mySpiffyCredential

VS Code's UI gets all squiggly with it and DevSkim reports the following problem:

Use of restricted functions.
Severity: [Important]
Use of restricted functions.
More Info:
https://github.com/Microsoft/DevSkim/blob/main/guidance/DS104456.md
Devskim: Finding DS104456(DS104456)

When I take a look at the intellisense link that redirects me to the DevSkim repo, the DS104456.md topic document is for all intents and purposes "empty." No guidance. Since there is no other way that I know of to run such commands on remote systems, I have permanently suppressed the "problem." That DevSkim reports a "problem," and has no other information to add, does not seem like super awesome behavior to me. It seems unnecessary. When I read the Invoke-Command guidance I don't find any supporting evidence that DevSkim should report this call as a "problem."

What's your take on this? Is this behavior a bug or something else?

Cheers,

klaw

klawrawkz avatar Feb 12 '21 18:02 klawrawkz

Sorry @klawrawkz - I know our documentation isn't as good as it should be. I'm sorry for that.

As to why this is reported -- since the Invoke-Command is often used in vulnerable (or malicious) code, DevSkim flags it's use, more to ensure careful review than to change it to something else. For example, if mySwellScript.vbs in your sample code was actually a variable passed in by the user of your script (or similar), then it could be bad.

But we might have over-emphasized the danger of this command -- we have a "security review" category that might be more appropriate. @joshbw what do you think?

scovetta avatar Feb 12 '21 18:02 scovetta

@scovetta , understood. The variable idea does seem like a potentially bad thing.

Your idea sounds good to me. Close this and push it over to the security reviewers. :)

Thanks for the follow up.

Cheers, klaw

klawrawkz avatar Feb 18 '21 00:02 klawrawkz