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

Name: Static array access may cause overflow

Description: Exceeding the size of a static array during write or access operations may result in a buffer overflow.

ID: cpp/static-buffer-overflow

Kind: problem

Severity: warning

Precision: medium

Query: OverflowStatic.ql
/**
 * @name Static array access may cause overflow
 * @description Exceeding the size of a static array during write or access operations
 *              may result in a buffer overflow.
 * @kind problem
 * @problem.severity warning
 * @precision medium
 * @id cpp/static-buffer-overflow
 * @tags reliability
 *       security
 *       external/cwe/cwe-119
 *       external/cwe/cwe-131
 */

import cpp
import semmle.code.cpp.commons.Buffer
import LoopBounds

private predicate staticBufferBase(VariableAccess access, Variable v) {
  v.getType().(ArrayType).getBaseType() instanceof CharType and
  access = v.getAnAccess() and
  not memberMayBeVarSize(_, v)
}

predicate staticBuffer(VariableAccess access, Variable v, int size) {
  staticBufferBase(access, v) and
  size = getBufferSize(access, _)
}

class BufferAccess extends ArrayExpr {
  BufferAccess() {
    exists(int size |
      staticBuffer(this.getArrayBase(), _, size) and
      size != 0
    ) and

    // exclude accesses in macro implementation of `strcmp`,
    // which are carefully controlled but can look dangerous.
    not exists(Macro m |
      m.getName() = "strcmp" and
      m.getAnInvocation().getAnExpandedElement() = this
    )
  }

  int bufferSize() { staticBuffer(this.getArrayBase(), _, result) }

  Variable buffer() { result.getAnAccess() = this.getArrayBase() }
}

predicate overflowOffsetInLoop(BufferAccess bufaccess, string msg) {
  exists(ClassicForLoop loop |
    loop.getStmt().getAChild*() = bufaccess.getEnclosingStmt() and
    loop.limit() >= bufaccess.bufferSize() and
    loop.counter().getAnAccess() = bufaccess.getArrayOffset() and
    msg = "Potential buffer-overflow: counter '" + loop.counter().toString() + "' <= " +
        loop.limit().toString() + " but '" + bufaccess.buffer().getName() + "' has " +
        bufaccess.bufferSize().toString() + " elements."
  )
}

predicate bufferAndSizeFunction(Function f, int buf, int size) {
  f.hasGlobalName("read") and buf = 1 and size = 2
  or
  f.hasGlobalName("fgets") and buf = 0 and size = 1
  or
  f.hasGlobalName("strncpy") and buf = 0 and size = 2
  or
  f.hasGlobalName("strncat") and buf = 0 and size = 2
  or
  f.hasGlobalName("memcpy") and buf = 0 and size = 2
  or
  f.hasGlobalName("memmove") and buf = 0 and size = 2
  or
  f.hasGlobalName("snprintf") and buf = 0 and size = 1
  or
  f.hasGlobalName("vsnprintf") and buf = 0 and size = 1
}

class CallWithBufferSize extends FunctionCall {
  CallWithBufferSize() { bufferAndSizeFunction(this.getTarget(), _, _) }

  Expr buffer() {
    exists(int i |
      bufferAndSizeFunction(this.getTarget(), i, _) and
      result = this.getArgument(i)
    )
  }

  Expr statedSizeExpr() {
    exists(int i |
      bufferAndSizeFunction(this.getTarget(), _, i) and
      result = this.getArgument(i)
    )
  }

  int statedSizeValue() {
    exists(Expr statedSizeSrc |
      DataFlow::localFlow(DataFlow::exprNode(statedSizeSrc), DataFlow::exprNode(statedSizeExpr())) and
      result = statedSizeSrc.getValue().toInt()
    )
  }
}

predicate wrongBufferSize(Expr error, string msg) {
  exists(CallWithBufferSize call, int bufsize, Variable buf, int statedSize |
    staticBuffer(call.buffer(), buf, bufsize) and
    statedSize = min(call.statedSizeValue()) and
    statedSize > bufsize and
    error = call.statedSizeExpr() and
    msg = "Potential buffer-overflow: '" + buf.getName() + "' has size " + bufsize.toString() +
        " not " + statedSize + "."
  )
}

predicate outOfBounds(BufferAccess bufaccess, string msg) {
  exists(int size, int access, string buf |
    buf = bufaccess.buffer().getName() and
    bufaccess.bufferSize() = size and
    bufaccess.getArrayOffset().getValue().toInt() = access and
    (
      access > size
      or
      access = size and not exists(AddressOfExpr addof | bufaccess = addof.getOperand())
    ) and
    msg = "Potential buffer-overflow: '" + buf + "' has size " + size.toString() + " but '" + buf +
        "[" + access.toString() + "]' is accessed here."
  )
}

from Element error, string msg
where
  overflowOffsetInLoop(error, msg) or
  wrongBufferSize(error, msg) or
  outOfBounds(error, msg)
select error, msg

When you use static arrays you must ensure that you do not exceed the size of the array during write and access operations. If an operation attempts to write to or access an element that is outside the range of the array then this results in a buffer overflow. Buffer overflows can lead to anything from a segmentation fault to a security vulnerability.

Recommendation

Check the offsets and sizes used in the highlighted operations to ensure that a buffer overflow will not occur.

Example

#define SIZE 30

int f(char * s) {
	char buf[20]; //buf not set to use SIZE macro

	strncpy(buf, s, SIZE); //wrong: copy may exceed size of buf

	for (int i = 0; i < SIZE; i++) { //wrong: upper limit that is higher than array size
		cout << array[i];
	}
}

References
  • I. Gerg. An Overview and Example of the Buffer-Overflow Exploit. IANewsletter vol 7 no 4. 2005.
  • M. Donaldson. Inside the Buffer Overflow Attack: Mechanism, Method & Prevention. SANS Institute InfoSec Reading Room. 2002.
  • Common Weakness Enumeration: CWE-119.
  • Common Weakness Enumeration: CWE-131.