Semmle 1.20
Skip to end of metadata
Go to start of metadata

Name: Bad check for overflow of integer addition

Description: Checking for overflow of integer addition by comparing against one of the arguments of the addition does not work when the result of the addition is automatically promoted to a larger type.

ID: cpp/bad-addition-overflow-check

Kind: problem

Severity: error

Precision: very-high

Query: BadAdditionOverflowCheck.ql
/**
 * @name Bad check for overflow of integer addition
 * @description Checking for overflow of integer addition by comparing
 *              against one of the arguments of the addition does not work
 *              when the result of the addition is automatically promoted
 *              to a larger type.
 * @kind problem
 * @problem.severity error
 * @precision very-high
 * @id cpp/bad-addition-overflow-check
 * @tags reliability
 *       correctness
 *       security
 *       external/cwe/cwe-190
 *       external/cwe/cwe-192
 */

import cpp
import BadAdditionOverflowCheck

from RelationalOperation cmp, AddExpr a
where badAdditionOverflowCheck(cmp, a)
select cmp, "Bad overflow check."

Checking for overflow of integer addition needs to be done with care, because automatic type promotion can prevent the check from working correctly.

Recommendation

Use an explicit cast to make sure that the result of the addition is not implicitly converted to a larger type.

Example

bool checkOverflow(unsigned short x, unsigned short y) {
  return (x + y < x);  // BAD: x and y are automatically promoted to int.
}

On a typical architecture where short is 16 bits and int is 32 bits, the operands of the addition are automatically promoted to int, so it cannot overflow and the result of the comparison is always false.

The code below implements the check correctly, by using an explicit cast to make sure that the result of the addition is unsigned short.

bool checkOverflow(unsigned short x, unsigned short y) {
  return ((unsigned short)(x + y) < x);  // GOOD: explicit cast
}

References