github
CodeQL C check for potential memset() removal by compiler dead store elimination is desired
memset() is often used for data sanitization in security sensitive software to harden against information leaks. However, compiler dead store elimination passes can remove that hardening. Recently, I had been late in reviewing a PR against OpenZFS that was intended to harden against information leaks, but used memset() in a way vulnerable to dead store elimination. I caught it upon review, verified that GCC truly was eliminating 4 memset() calls that had been dead stores to stack memory and submitted the following now merged patch to fix it:
https://github.com/openzfs/zfs/commit/d634d20d1be31dfa8cf06ef2dc96285baf81a2fb
Every PR to OpenZFS is scanned by CodeQL. While we are not using the security-and-extended checks, I have skimmed through their reports in my fork and did not see any warnings about this. I run several static analyzers on the codebase and none of them reported this, although the documentation for the commercial static analyzer, PVS Studio, suggests that it is able to report such issues:
https://pvs-studio.com/en/docs/warnings/v570/
It would be nice if CodeQL at least had an optional check to catch this. Please l me know if it already has one that I missed.
There is a query for this: https://github.com/github/codeql/blob/bf6f6eec34beb144f214a743a14b73fb55a7b98f/cpp/ql/src/Security/CWE/CWE-014/MemsetMayBeDeleted.ql Here is the QHelp: https://github.com/github/codeql/blob/bf6f6eec34beb144f214a743a14b73fb55a7b98f/cpp/ql/src/Security/CWE/CWE-014/MemsetMayBeDeleted.qhelp
However, I don't know whether this query can find the problem you described.
Yeah, that looks like the query that should be responsible for looking for this. I've asked our C/C++ analysis experts whether they can see a reason it wouldn't find the problem in this case.
Thanks for the report. We've understood why the existing query didn't alert on this code (the query only looks for calls to memset where the buffer is a local variable in the same function as the call). We will look into how it can be made more sensitive.