Skip to content

C++: Add CWE-789 tag to cpp/uncontrolled-allocation-size. #6041

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

Merged
merged 1 commit into from
Jun 8, 2021

Conversation

geoffw0
Copy link
Contributor

@geoffw0 geoffw0 commented Jun 8, 2021

Add the CWE-789 ("Memory Allocation with Excessive Size Value") tag to cpp/uncontrolled-allocation-size ("Overflow in uncontrolled allocation size") in addition to the existing CWE-190 ("Integer Overflow or Wraparound") tag.

I considered increasing the @precision medium of the query to high as well. I think the main issue holding it back is uncertainty about taint sources (such as from getenv, argv), an issue we're going to have to address properly at some point. There are also some cases involving strlen which may not actually be abusable.

@geoffw0 geoffw0 added the C++ label Jun 8, 2021
@geoffw0 geoffw0 requested a review from a team as a code owner June 8, 2021 10:18
@geoffw0 geoffw0 added the no-change-note-required This PR does not need a change note label Jun 8, 2021
@geoffw0
Copy link
Contributor Author

geoffw0 commented Jun 8, 2021

I don't think the "Build/check CSV flow coverage report / build (pull_request)" check failing is relevant to this PR.

@MathiasVP
Copy link
Contributor

I considered increasing the @precision medium of the query to high as well. I think the main issue holding it back is uncertainty about taint sources (such as from getenv, argv), an issue we're going to have to address properly at some point. There are also some cases involving strlen which may not actually be abusable.

I'm surprised about the strlen part. I thought #3592 prevented alerts related to strlen?

@geoffw0
Copy link
Contributor Author

geoffw0 commented Jun 8, 2021

Yes. I saw two results that appeared to go through some kind of strlen call though. I should investigate.

@MathiasVP MathiasVP merged commit 8fb1566 into github:main Jun 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C++ no-change-note-required This PR does not need a change note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants