Skip to content

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. #19721

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

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 23 additions & 3 deletions cpp/ql/src/Critical/OverflowCalculated.ql
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
/**
* @name Buffer not sufficient for string
* @description A buffer allocated using 'malloc' may not have enough space for a string that is being copied into it. The operation can cause a buffer overrun. Make sure that the buffer contains enough room for the string (including the zero terminator).
* @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
* @id cpp/overflow-calculated
* @problem.severity warning
* @security-severity 9.8
Expand Down Expand Up @@ -40,6 +41,25 @@ predicate spaceProblem(FunctionCall append, string msg) {
)
}

predicate wideCharSizeofProblem(FunctionCall call, string msg) {
exists(
Variable buffer, SizeofExprOperator sizeofOp, ArrayType arrayType
|
// Function call is to wcsftime
call.getTarget().hasGlobalOrStdName("wcsftime") and
// Second argument (count parameter) is a sizeof operation
call.getArgument(1) = sizeofOp and
// The sizeof is applied to a buffer variable
sizeofOp.getExprOperand() = buffer.getAnAccess() and
// The buffer is an array of wchar_t
arrayType = buffer.getType() and
arrayType.getBaseType().hasName("wchar_t") and
msg =
"Using sizeof(" + buffer.getName() + ") passes byte count instead of wchar_t element count to wcsftime. " +
"Use sizeof(" + buffer.getName() + ")/sizeof(wchar_t) or array length instead."
)
}

from Expr problem, string msg
where spaceProblem(problem, msg)
where spaceProblem(problem, msg) or wideCharSizeofProblem(problem, msg)
select problem, msg
Loading