Skip to content

Flow from insecure random number to sensitive sink#346

Open
chanel-y wants to merge 5 commits intomainfrom
users/chanely/insecure-random
Open

Flow from insecure random number to sensitive sink#346
chanel-y wants to merge 5 commits intomainfrom
users/chanely/insecure-random

Conversation

@chanel-y
Copy link
Copy Markdown

No description provided.

chanel-y and others added 2 commits April 9, 2026 09:19
Detects use of Get-Random cmdlet and System.Random class which are
not cryptographically secure. Policy: Microsoft.Security.Cryptography.10017

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Collaborator

@MathiasVP MathiasVP left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of small comments. I'd prefer that we move NextBytes into a MaD model in this PR as I'm afraid we'd otherwise forget about it, but if you have any issues doing that I'm also okay with leaving this as-is.

Comment on lines +23 to +30
// When .NextBytes() is called on a System.Random instance,
// taint flows from the qualifier to the argument (the byte array is filled with random bytes).
exists(DataFlow::CallNode call |
call.matchesName("NextBytes") and
node1 = call.getQualifier() and
node2.(DataFlow::PostUpdateNode).getPreUpdateNode() = call.getArgument(0)
)
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably just add this as a MaD summary model instead of this. Could you add a summary model row to https://github.com/microsoft/codeql/blob/main/powershell/ql/lib/semmle/code/powershell/frameworks/System.model.yml with this instead? It should be something along the lines of:

  - addsTo:
      pack: microsoft/powershell-all
      extensible: summaryModel
    data:
      - ["system.random", "Method[nextbytes]", "Argument[this]", "Argument[0]", "taint"]

Comment thread powershell/ql/test/query-tests/security/cwe-338/InsecureRandomness/test.ps1 Outdated
Comment thread powershell/ql/test/query-tests/security/cwe-338/InsecureRandomness/test.ps1 Outdated
Comment thread powershell/ql/test/query-tests/security/cwe-338/InsecureRandomness/test.ps1 Outdated
Comment thread powershell/ql/test/query-tests/security/cwe-338/InsecureRandomness/test.ps1 Outdated
Comment thread powershell/ql/test/query-tests/security/cwe-338/InsecureRandomness/test.ps1 Outdated
Comment thread powershell/ql/test/query-tests/security/cwe-338/InsecureRandomness/test.ps1 Outdated
Comment thread powershell/ql/test/query-tests/security/cwe-338/InsecureRandomness/test.ps1 Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants