github / codeql

CodeQL: the libraries and queries that power security researchers around the world, as well as code scanning in GitHub Advanced Security
https://codeql.github.com
MIT License
7.38k stars 1.48k forks source link

False positive: Ruby: Kernel Open when File existence guard is present #16943

Open JLLeitschuh opened 2 weeks ago

JLLeitschuh commented 2 weeks ago

Description of the false positive

When IO.read is guarded by a check like File.exists?, isn't that a valid guard against injecting the | character into Kernel.open? I don't imagine that many systems out there have files lying around named |.txt.

Code samples or links to source code

https://github.com/github/codeql/blob/81593ece5aa7701ec0b103932f84ff65ae506e0b/ruby/ql/lib/codeql/ruby/security/KernelOpenQuery.qll#L83C1-L87

URL to the alert on GitHub code scanning (optional)

ginsbach commented 2 weeks ago

Thank you for the report! We do not prioritise false positives at the moment, but we track them internally and will keep them in mind for future work.

JLLeitschuh commented 2 weeks ago

Seems like an interesting choice, given that, from what I've heard, many orgs purchase SAST tools based upon the FP rate compared to other SAST tools.

Perhaps at least updating the documentation stating that a File.exists? check is a valid guard, but doing so won't resolve the alert?

ginsbach commented 2 weeks ago

Apologies if I sounded dismissive, that wasn't my intention. We do value false positive reports, but for now we have other product priorities. Therefore, we do not immediately act on false positive reports but instead track them for later consideration.

That being said, I have forwarded your documentation suggestion to the relevant team.