-
Notifications
You must be signed in to change notification settings - Fork 1.7k
False positives in cpp/user-after-free #19387
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Hey @ajohnston9 thanks for raising this! |
Hi @ajohnston9, Thanks for your report. We are aware of this problem, and unfortunately there's no easy fix for this. I've added a link to this issue to our internal issue tracking this. |
@coadaflorin We haven't yet attempted to modify the query. I understand that fixing this issue in a global sense is difficult, but ideally I'd like to see examples like the ones above captured since they're relatively trivial (re-allocation happens within the same method within a few lines of the Could you also consider dropping the |
@ajohnston9 we'll add this to our to do list. We'll consider both options and publish w/e change we make in the changelogs. |
cpp-user-after-free
seems to have a number of false positives, particular when a pointer isfree
d, re-allocated, and then reused correctly.Consider the following code snippet from this part of OpenSC:
Analysis of the code shows that although free(priv->aid_der.value) is called at line 2939, the pointer priv->aid_der.value is immediately reassigned by a malloc call at line 2940. If the malloc fails, the function returns before the potentially dangerous memcpy at line 2943 is reached. If malloc succeeds, the memcpy operates on the newly allocated buffer, preventing a use-after-free. This finding appears to be a false positive.
Similarly, consider this snippet from this part of Assimp:
Again, the CodeQL finding indicates a potential use-after-free vulnerability where pcScene->mRootNode->mChildren is deleted at line 661 and potentially used later at line 678. Analysis of the code shows that pcScene->mRootNode->mChildren is reallocated with new aiNode*[apcNodes.size()] on line 678 before being accessed in the loop starting on line 679. Therefore, this finding appears to be a false positive, as the memory is valid when accessed.
These findings were generated when running
codeql/cpp-queries
against the given codebases.Recommendation
Revise the query to look for use of a
free
d pointer where the the use of the pointer is done before anymalloc
,new
or similar memory-allocating function is performed. Validate the query against test cases such as these to ensure that proper pointer re-use isn't flagged as a potential UAF.The text was updated successfully, but these errors were encountered: