Conversation
MathiasVP
left a comment
MathiasVP
left a comment
There was a problem hiding this comment.
LGTM! One comment, but that's not blocking!
| class CmdletBindingParam extends CriticalSource { | ||
| CmdletBindingParam(){ | ||
| exists(Attribute a, Function f | | ||
| a.getName() = "CmdletBinding" and |
There was a problem hiding this comment.
This should probably to case insensitive matching since I'm sure PowerShell happily accepts cMdlEtBinDinG. But I'll fix that in another since we don't have an API for this on Attribute yet.
| exists(Function f, Attribute a, Parameter p | | ||
| p = f.getAParameter() and | ||
| p.getAnAttribute() = a and | ||
| a.getName() = ["ValidateScript", "ValidateSet", "ValidatePattern"] and |
There was a problem hiding this comment.
Again: I'll do another PR to make this case insensitive. Thanks for adding this!
| this instanceof SourceNode and | ||
| not this instanceof EnvironmentVariableSource | ||
| not this instanceof EnvironmentVariableSource and | ||
| not this instanceof InvokeWebRequest |
There was a problem hiding this comment.
Should we only remove it when it's given a constant string literal as a source? Or do you think it's better to totally remove it like you're doing here?
There was a problem hiding this comment.
I think totally remove it. It could be a case if there's flow from user input -> InvokeWebRequest -> command call, but that's more of an SSRF vuln first, which we can model as a separate query
2 participants
Couple of changes in this one suggested by @ewilloner