-
Notifications
You must be signed in to change notification settings - Fork 1.7k
CPP: Add query for CWE-754: Improper Check for Unusual or Exceptional Conditions when using functions scanf #8246
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @ihsinme. Thanks for your contribution. Some comments below.
cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.qhelp
Outdated
Show resolved
Hide resolved
cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.qhelp
Outdated
Show resolved
Hide resolved
cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.ql
Outdated
Show resolved
Hide resolved
cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.ql
Outdated
Show resolved
Hide resolved
cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.ql
Outdated
Show resolved
Hide resolved
cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.ql
Outdated
Show resolved
Hide resolved
cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.ql
Outdated
Show resolved
Hide resolved
cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.ql
Outdated
Show resolved
Hide resolved
cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.ql
Outdated
Show resolved
Hide resolved
cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.ql
Outdated
Show resolved
Hide resolved
thanks for the comments. |
Co-authored-by: Jeroen Ketema <93738568+jketema@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates, especially making the tests cover more cases.
cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp
Outdated
Show resolved
Hide resolved
cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp
Outdated
Show resolved
Hide resolved
cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp
Outdated
Show resolved
Hide resolved
cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp
Show resolved
Hide resolved
cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp
Show resolved
Hide resolved
cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp
Outdated
Show resolved
Hide resolved
cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp
Outdated
Show resolved
Hide resolved
cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp
Outdated
Show resolved
Hide resolved
Co-authored-by: Jeroen Ketema <93738568+jketema@users.noreply.github.com>
cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.qhelp
Outdated
Show resolved
Hide resolved
cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.ql
Outdated
Show resolved
Hide resolved
cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.ql
Outdated
Show resolved
Hide resolved
cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.ql
Outdated
Show resolved
Hide resolved
cpp/ql/src/experimental/Security/CWE/CWE-754/ImproperCheckReturnValueScanf.ql
Show resolved
Hide resolved
cpp/ql/test/experimental/query-tests/Security/CWE/CWE-754/semmle/tests/test.cpp
Outdated
Show resolved
Hide resolved
Co-authored-by: Jeroen Ketema <93738568+jketema@users.noreply.github.com>
I want to thank you for your corrections. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution and addressing my comments.
Not for this PR, but you might want to have a look at the data flow library https://codeql.github.com/docs/codeql-language-guides/analyzing-data-flow-in-cpp/. I think that a rewrite based on that library might help to improve the quality of the analysis of what happens after a scanf
call.
the query looks for situations of working with
scanf
functions in situations where the developer has not left the possibility of controlling the correctness of work. There is no check of the returned value, the filled variable is not initialized and its value is not evaluated after the function has run. this leads to a situation where, continuing to work with a filled variable, the developer runs the risk of working with a randomly filled value at the declaration stage.CVE-2019-15900
I'm working on a real fix for this issue.