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

Name: Not enough memory allocated for array of pointer type

Description: Calling 'malloc', 'calloc' or 'realloc' without allocating enough memory to contain multiple instances of the type of the pointer may result in a buffer overflow

ID: cpp/suspicious-allocation-size

Kind: problem

Severity: warning

Precision: medium

Query: SizeCheck2.ql
/**
 * @name Not enough memory allocated for array of pointer type
 * @description Calling 'malloc', 'calloc' or 'realloc' without allocating enough memory to contain
 *              multiple instances of the type of the pointer may result in a buffer overflow
 * @kind problem
 * @problem.severity warning
 * @precision medium
 * @id cpp/suspicious-allocation-size
 * @tags reliability
 *       security
 *       external/cwe/cwe-131
 *       external/cwe/cwe-122
 */

import cpp

class Allocation extends FunctionCall {
  Allocation() {
    exists(string name |
      this.getTarget().hasGlobalName(name) and
      (name = "malloc" or name = "calloc" or name = "realloc")
    )
  }

  private string getName() { this.getTarget().hasGlobalName(result) }

  int getSize() {
    this.getName() = "malloc" and
    this.getArgument(0).getValue().toInt() = result
    or
    this.getName() = "realloc" and
    this.getArgument(1).getValue().toInt() = result
    or
    this.getName() = "calloc" and
    result = this.getArgument(0).getValue().toInt() * this.getArgument(1).getValue().toInt()
  }
}

predicate baseType(Allocation alloc, Type base) {
  exists(PointerType pointer |
    pointer.getBaseType() = base and
    (
      exists(AssignExpr assign |
        assign.getRValue() = alloc and assign.getLValue().getType() = pointer
      )
      or
      exists(Variable v | v.getInitializer().getExpr() = alloc and v.getType() = pointer)
    )
  )
}

from Allocation alloc, Type base, int basesize, int allocated
where
  baseType(alloc, base) and
  allocated = alloc.getSize() and
  // If the codebase has more than one type with the same name, check if any matches
  not exists(int size | base.getSize() = size |
    size = 0 or
    (allocated / size) * size = allocated
  ) and
  basesize = min(base.getSize())
select alloc,
  "Allocated memory (" + allocated.toString() + " bytes) is not a multiple of the size of '" +
    base.getName() + "' (" + basesize.toString() + " bytes)."

When you allocate an array from memory using malloc, calloc or realloc, you should ensure that you allocate enough memory to contain a multiple of the size of the required pointer type. Calls that are assigned to a non-void pointer variable, but do not allocate enough memory will cause a buffer overflow when a field accessed on the pointer points to memory that is beyond the allocated array. Buffer overflows can lead to anything from a segmentation fault to a security vulnerability.

Recommendation

The highlighted call allocates memory that is not a multiple of the size of the pointer type, which can cause a memory overrun. Use the sizeof operator to ensure that the function call allocates enough memory for that type.

Example

#define RECORD_SIZE 30  //incorrect or outdated size for record
typedef struct {
	char name[30];
	int status;
} Record;

void f() {
	Record* p = malloc(RECORD_SIZE * 4); //wrong: not a multiple of the size of Record
	p[3].status = 1; //will most likely segfault
	...
}

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-131.
  • Common Weakness Enumeration: CWE-122.