JBaskota102211 (Community Member) asked a question.
Veracode is flagging the line in bold the following code for potential multiplication operation overflow. I think I have done the necessary checks. Please help.
bool DoCalculatePoints (const short numLines, const aPoint pnts[]) {
if (numLines > 0)
{
// Check for potential overflow in the calculation of total points.
if (static_cast<size_t> (numLines) <= (std::numeric_limits<size_t>::max () / 2))
{
const size_t maxPoints = static_cast<size_t> (numLines) * 2;
.png){kind=logo}
Short answer: your guard is logically correct, but Veracode's CWE-190 (Integer Overflow) detector may not recognize it as a sanitizer on this particular multiplication. You have two realistic paths forward: (1) restructure the check so the static analyzer can clearly tie the bound to the multiplication, or (2) mark it as a mitigated finding with a justification.
Why Veracode is still flagging it:
1) The taint source is numLines (a short), not a size_t. The scanner tracks the tainted signed value through the cast and into the multiplication. Your bounds check compares the cast value to max/2, but Veracode's data-flow model often doesn't treat "compared against a constant derived from numeric_limits" as a sanitizer for the original tainted input.
2) numLines is a short, so on any modern platform SHRT_MAX is 32,767 and static_cast<size_t>(numLines) * 2 cannot overflow a 32- or 64-bit size_t. The check <= max/2 is essentially dead code the analyzer can't correlate, so it doesn't see a "real" sanitizer between source and sink.
3) The guard is on the operand, but the sink annotation is on the multiplication expression. Some Veracode rules want the guard expressed as a direct comparison on the result's domain using the source variable type.
Recommended code change - validate using the source variable and a named constant, then cast:
bool DoCalculatePoints(const short numLines, const aPoint pnts[]) {
if (numLines <= 0) return false;
constexpr size_t kMultiplier = 2;
if (static_cast<size_t>(numLines) > std::numeric_limits<size_t>::max() / kMultiplier) {
return false; // would overflow
}
const size_t maxPoints = static_cast<size_t>(numLines) * kMultiplier;
// ...
}
Even better, use a checked-arithmetic primitive that Veracode recognizes as a sanitizer:
size_t maxPoints = 0;
if (__builtin_mul_overflow(static_cast<size_t>(numLines), 2u, &maxPoints)) {
return false;
}
Veracode's CWE-190 rules explicitly credit these APIs as mitigations, so the finding should drop off on the next scan.
If you don't want to change the code: because numLines is a short, the multiplication genuinely cannot overflow size_t on any supported platform. This is a reasonable case for marking the finding as Mitigated by Design (or Potential False Positive) in the Triage Flaws view, with a comment such as: "Operand is short (max 32,767); result fits in size_t on all supported architectures; pre-multiplication bound check confirms this."
TL;DR: rewrite the guard to compare the source variable against numeric_limits<size_t>::max() / multiplier directly, or use __builtin_mul_overflow / SafeInt, and Veracode will stop flagging it. Otherwise triage it as mitigated - the check you wrote is mathematically sound.