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

Name: Suspicious 'sizeof' use

Description: Taking 'sizeof' of an array parameter is often mistakenly thought to yield the size of the underlying array, but it always yields the machine pointer size.

ID: cpp/suspicious-sizeof

Kind: problem

Severity: warning

Precision: medium

Query: SuspiciousSizeof.ql
/**
 * @name Suspicious 'sizeof' use
 * @description Taking 'sizeof' of an array parameter is often mistakenly thought
 *              to yield the size of the underlying array, but it always yields
 *              the machine pointer size.
 * @kind problem
 * @problem.severity warning
 * @precision medium
 * @id cpp/suspicious-sizeof
 * @tags reliability
 *       correctness
 *       security
 *       external/cwe/cwe-467
 */
import cpp

class CandidateParameter extends Parameter {
  CandidateParameter() {
      // an array parameter
      getType().getUnspecifiedType() instanceof ArrayType
      or
      (
        // a pointer parameter
        getType().getUnspecifiedType() instanceof PointerType and
        
        // whose address is never taken (rules out common
        // false positive patterns)
        not exists(AddressOfExpr aoe | aoe.getAddressable() = this)
      )
  }
}

from SizeofExprOperator seo, VariableAccess va
where seo.getExprOperand() = va and
      va.getTarget() instanceof CandidateParameter and
      not va.isAffectedByMacro() and
      not va.isCompilerGenerated()
select seo, "This evaluates to the size of the pointer type, which may not be what you want."

This rule finds expressions that take the size of a function parameter of array type. In C, function parameters of array type are treated as if they had the corresponding pointer type, so their size is always the size of the pointer type (typically either four or eight). In particular, one cannot determine the size of a memory buffer passed as a parameter in this way. Using the sizeof operator on pointer types will produce unexpected results if the developer intended to get the size of an array instead of the pointer.

Recommendation

Modify the function to take an extra argument indicating the buffer size.

Example

void f(char s[]) {
	int size = sizeof(s); //wrong: s is now a char*, not an array. 
	                      //sizeof(s) will evaluate to sizeof(char *)
}

References