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

Name: Suspicious pointer scaling

Description: Implicit scaling of pointer arithmetic expressions can cause buffer overflow conditions.

ID: cpp/suspicious-pointer-scaling

Kind: problem

Severity: warning

Precision: high

Query: IncorrectPointerScaling.ql
/**
 * @name Suspicious pointer scaling
 * @description Implicit scaling of pointer arithmetic expressions
 *              can cause buffer overflow conditions.
 * @kind problem
 * @problem.severity warning
 * @precision high
 * @id cpp/suspicious-pointer-scaling
 * @tags security
 *       external/cwe/cwe-468
 */
import IncorrectPointerScalingCommon

from Expr dest, Type destType, Type sourceType, Type sourceBase,
     Type destBase, Location sourceLoc
where exists(pointerArithmeticParent(dest))
  and exprSourceType(dest, sourceType, sourceLoc)
  and sourceBase = baseType(sourceType)
  and destType = dest.getFullyConverted().getType()
  and destBase = baseType(destType)
  and destBase.getSize() != sourceBase.getSize()
  and not dest.isInMacroExpansion()

  // If the source type is a char* or void* then don't
  // produce a result, because it is likely to be a false
  // positive.
  and not (sourceBase instanceof CharType)
  and not (sourceBase instanceof VoidType)

  // Low-level pointer tricks often involve casting a struct pointer to a
  // char pointer, then accessing it at byte offsets. For example, this can
  // be necessary in order to resume an interrupted `write(2)`.
  and not (destBase instanceof CharType)
  
  // Similarly, gcc and compilers emulating it will allow void pointer
  // arithmetic as if void were a 1-byte type
  and not (destBase instanceof VoidType)

  // Don't produce an alert if the root expression computes
  // an offset, rather than a pointer. For example:
  // ```
  //     (p + 1) - q
  // ```
  and forall(Expr parent |
             parent = pointerArithmeticParent+(dest) |
             parent.getFullyConverted().getUnspecifiedType() instanceof PointerType)
select
  dest,
  "This pointer might have type $@ (size " + sourceBase.getSize() +
  "), but the pointer arithmetic here is done with type " +
  destType + " (size " + destBase.getSize() + ").",
  sourceLoc, sourceBase.toString()

Pointer arithmetic in C and C++ is automatically scaled according to the size of the data type. For example, if the type of p is T* and sizeof(T) == 4 then the expression p+1 adds 4 bytes to p. This can cause a buffer overflow condition if the programmer forgets that they are adding a multiple of sizeof(T), rather than a number of bytes.

This query finds pointer arithmetic expressions where it appears likely that the programmer has forgotten that the offset is automatically scaled.

Recommendation
  1. Whenever possible, use the array subscript operator rather than pointer arithmetic. For example, replace *(p+k) with p[k].
  2. Cast to the correct type before using pointer arithmetic. For example, if the type of p is int* but it really points to an array of type double[] then use the syntax (double*)p + k to get a pointer to the k'th element of the array.
Example

int example1(int i) {
  int intArray[10] = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };
  int *intPointer = intArray;
  // BAD: the offset is already automatically scaled by sizeof(int),
  // so this code will compute the wrong offset.
  return *(intPointer + (i * sizeof(int)));
}

int example2(int i) {
  int intArray[10] = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };
  int *intPointer = intArray;
  // GOOD: the offset is automatically scaled by sizeof(int).
  return *(intPointer + i);
}

References
  • Common Weakness Enumeration: CWE-468.