CodeQL queries 1.23

Skip to end of metadata
Go to start of metadata

Name: Suspicious pointer scaling to char

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

ID: cpp/incorrect-pointer-scaling-char

Kind: problem

Severity: warning

Precision: low

Query: IncorrectPointerScalingChar.ql
/**
 * @name Suspicious pointer scaling to char
 * @description Implicit scaling of pointer arithmetic expressions
 *              can cause buffer overflow conditions.
 * @kind problem
 * @id cpp/incorrect-pointer-scaling-char
 * @problem.severity warning
 * @precision low
 * @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() and
  // If the source type is a char* or void* then don't
  // produce a result, because it is likely to be a false
  // positive.
  not sourceBase instanceof CharType and
  not sourceBase instanceof VoidType and
  // Don't produce an alert if the dest type is `char *` but the
  // expression contains a `sizeof`, which is probably correct.  For
  // example:
  // ```
  //   int x[3] = {1,2,3};
  //   char* p = (char*)x;
  //   return *(int*)(p + (2 * sizeof(int)))
  // ```
  not (
    destBase instanceof CharType and
    dest.getParent().(Expr).getAChild*() instanceof SizeofOperator
  ) and
  // Don't produce an alert if the root expression computes
  // an offset, rather than a pointer. For example:
  // ```
  //     (p + 1) - q
  // ```
  forall(Expr parent | parent = pointerArithmeticParent+(dest) |
    parent.getFullyConverted().getUnspecifiedType() instanceof PointerType
  ) and
  // Only produce alerts that are not produced by `IncorrectPointerScaling.ql`.
  destBase instanceof CharType
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()

Casting arbitrary pointers into char* and then accessing their contents should be done with care. The results may not be portable.

This query finds pointer arithmetic expressions where a pointer to char (or similar) is dereferenced even though the underlying value is of a type larger than char.

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 char* 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

char example1(int i) {
  int intArray[5] = { 1, 2, 3, 4, 5 };
  char *charPointer = (char *)intArray;
  // BAD: the pointer arithmetic uses type char*, so the offset
  // is not scaled by sizeof(int).
  return *(charPointer + i);
}

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.