-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Enhance cpp/overflow-calculated - detect out-of-bounds write caused by passing the buffer size in bytes (using sizeof) instead of the number of elements to wcsftime, allowing the function to overrun the allocated buffer.
#19722
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
base: main
Are you sure you want to change the base?
Enhance cpp/overflow-calculated - detect out-of-bounds write caused by passing the buffer size in bytes (using sizeof) instead of the number of elements to wcsftime, allowing the function to overrun the allocated buffer.
#19722
Conversation
- Pointer case: sizeof(pointer) gives pointer size (e.g., 8 bytes), which is completely wrong
| @@ -0,0 +1,4 @@ | |||
| --- | |||
| category: newQuery | |||
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.
This query did exist previously but did not have any precision so it was not included in any suite.
Also enhanced the query to find additional scenarios - what would be best category?
| * @name Buffer overflow from insufficient space or incorrect size calculation | ||
| * @description A buffer allocated using 'malloc' may not have enough space for a string being copied into it, or wide character functions may receive incorrect size parameters causing buffer overrun. Make sure that buffers contain enough room for strings (including zero terminator) and that size parameters are correctly calculated. | ||
| * @kind problem | ||
| * @precision medium |
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.
This will add to security-extended suite. Review findings in DCA to ensure FP rate is low. FN's have not been evaluated by me - only added coverage for the above scenario only.
| ql/cpp/ql/src/Critical/IncorrectCheckScanf.ql | ||
| ql/cpp/ql/src/Critical/MissingCheckScanf.ql | ||
| ql/cpp/ql/src/Critical/NewFreeMismatch.ql | ||
| ql/cpp/ql/src/Critical/OverflowCalculated.ql |
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.
Hand rolled this, security-and-quality, and not_included_in
This pull request improves the detection of buffer overflow issues in the
OverflowCalculated.qlquery.Improvements to buffer overflow detection:
Enhanced query description: The description of the query has been updated to include scenarios involving incorrect size calculations for wide character functions, improving clarity and scope. Additionally, the precision level was set to "medium" to better reflect the query's behavior. (
cpp/ql/src/Critical/OverflowCalculated.ql, cpp/ql/src/Critical/OverflowCalculated.qlL2-R5)New predicate for wide character size issues: Added the
wideCharSizeofProblempredicate to detect cases where thesizeofoperator is incorrectly used withwcsftime, leading to potential buffer overflows. The predicate checks for miscalculations in thecountparameter by ensuring that the size is expressed in terms ofwchar_telements rather than bytes. (cpp/ql/src/Critical/OverflowCalculated.ql, cpp/ql/src/Critical/OverflowCalculated.qlR44-R64)Updated query logic: Modified the
whereclause in the query to include the newwideCharSizeofProblempredicate, enabling the detection of both traditional space problems and wide character size issues. (cpp/ql/src/Critical/OverflowCalculated.ql, cpp/ql/src/Critical/OverflowCalculated.qlR44-R64)