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

Name: Array offset used before range check

Description: Accessing an array offset before checking the range means that the program may attempt to read beyond the end of a buffer

ID: cpp/offset-use-before-range-check

Kind: problem

Severity: warning

Precision: medium

Query: OffsetUseBeforeRangeCheck.ql
/**
 * @name Array offset used before range check
 * @description Accessing an array offset before checking the range means that
 *              the program may attempt to read beyond the end of a buffer
 * @kind problem
 * @id cpp/offset-use-before-range-check
 * @problem.severity warning
 * @precision medium
 * @tags reliability
 *       security
 *       external/cwe/cwe-120
 *       external/cwe/cwe-125
 */

import cpp

predicate beforeArrayAccess(Variable v, ArrayExpr access, Expr before) {
  exists(LogicalAndExpr andexpr |
    access.getArrayOffset() = v.getAnAccess() and
    andexpr.getRightOperand().getAChild*() = access and
    andexpr.getLeftOperand() = before
  )
}

predicate afterArrayAccess(Variable v, ArrayExpr access, Expr after) {
  exists(LogicalAndExpr andexpr |
    access.getArrayOffset() = v.getAnAccess() and
    andexpr.getLeftOperand().getAChild*() = access and
    andexpr.getRightOperand() = after
  )
}

from Variable v, ArrayExpr access, LTExpr rangecheck
where
  afterArrayAccess(v, access, rangecheck) and
  rangecheck.getLeftOperand() = v.getAnAccess() and
  not access.isInMacroExpansion() and
  not exists(LTExpr altcheck |
    beforeArrayAccess(v, access, altcheck) and
    altcheck.getLeftOperand() = v.getAnAccess()
  )
select access, "This use of offset '" + v.getName() + "' should follow the $@.", rangecheck,
  "range check"

The program contains an and-expression where the array access is defined before the range check. Consequently the array is accessed without any bounds checking. The range check does not protect the program from segmentation faults caused by attempts to read beyond the end of a buffer.

Recommendation

Update the and-expression so that the range check precedes the array offset. This will ensure that the bounds are checked before the array is accessed.

Example

The find function can read past the end of the buffer pointed to by str if start is longer than or equal to the length of the buffer (or longer than len, depending on the contents of the buffer).

int find(int start, char *str, char goal)
{
    int len = strlen(str);
    //Potential buffer overflow
    for (int i = start; str[i] != 0 && i < len; i++) { 
        if (str[i] == goal)
            return i; 
    }
    return -1;
}

int findRangeCheck(int start, char *str, char goal)
{
    int len = strlen(str);
    //Range check protects against buffer overflow
    for (int i = start; i < len && str[i] != 0 ; i++) {
        if (str[i] == goal)
            return i; 
    }
    return -1;
}



Update the and-expression so that the range check precedes the array offset (for example, the findRangeCheck function).

References