CodeQL queries 1.24

Skip to end of metadata
Go to start of metadata

Name: Pointer overflow check

Description: Adding a value to a pointer to check if it overflows relies on undefined behavior and may lead to memory corruption.

ID: cpp/pointer-overflow-check

Kind: problem

Severity: error

Precision: high

Query: PointerOverflow.ql
/**
 * @name Pointer overflow check
 * @description Adding a value to a pointer to check if it overflows relies
 *              on undefined behavior and may lead to memory corruption.
 * @kind problem
 * @problem.severity error
 * @precision high
 * @id cpp/pointer-overflow-check
 * @tags reliability
 *       security
 */

import cpp
private import semmle.code.cpp.valuenumbering.GlobalValueNumbering
private import semmle.code.cpp.commons.Exclusions

from RelationalOperation ro, PointerAddExpr add, Expr expr1, Expr expr2
where
  ro.getAnOperand() = add and
  add.getAnOperand() = expr1 and
  ro.getAnOperand() = expr2 and
  globalValueNumber(expr1) = globalValueNumber(expr2) and
  // Exclude macros but not their arguments
  not isFromMacroDefinition(ro) and
  // There must be a compilation of this file without a flag that makes pointer
  // overflow well defined.
  exists(Compilation c | c.getAFileCompiled() = ro.getFile() |
    not c.getAnArgument() = "-fwrapv-pointer" and
    not c.getAnArgument() = "-fno-strict-overflow"
  )
select ro, "Range check relying on pointer overflow."

When checking for integer overflow, you may often write tests like p + i < p. This works fine if p and i are unsigned integers, since any overflow in the addition will cause the value to simply "wrap around." However, using this pattern when p is a pointer is problematic because pointer overflow has undefined behavior according to the C and C++ standards. If the addition overflows and has an undefined result, the comparison will likewise be undefined; it may produce an unintended result, or may be deleted entirely by an optimizing compiler.

Recommendation

To check whether an index i is less than the length of an array, simply compare these two numbers as unsigned integers: i < ARRAY_LENGTH. If the length of the array is defined as the difference between two pointers ptr and p_end, write i < p_end - ptr. If i is signed, cast it to unsigned in order to guard against negative i. For example, write (size_t)i < p_end - ptr.

Example

An invalid check for pointer overflow is most often seen as part of checking whether a number a is too large by checking first if adding the number to ptr goes past the end of an allocation and then checking if adding it to ptr creates a pointer so large that it overflows and wraps around.

bool not_in_range(T *ptr, T *ptr_end, size_t i) {
    return ptr + i >= ptr_end || ptr + i < ptr; // BAD
}

In both of these checks, the operations are performed in the wrong order. First, an expression that may cause undefined behavior is evaluated (ptr + i), and then the result is checked for being in range. But once undefined behavior has happened in the pointer addition, it cannot be recovered from: it's too late to perform the range check after a possible pointer overflow.

While it's not the subject of this query, the expression ptr + i < ptr_end is also an invalid range check. It's undefined behavor in C/C++ to create a pointer that points more than one past the end of an allocation.

The next example shows how to portably check whether an unsigned number is outside the range of an allocation between ptr and ptr_end.

bool not_in_range(T *ptr, T *ptr_end, size_t i) {
    return i >= ptr_end - ptr; // GOOD
}

References