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

Name: Copy function using source size

Description: Calling a copy operation with a size derived from the source buffer instead of the destination buffer may result in a buffer overflow.

ID: cpp/overflow-destination

Kind: problem

Severity: warning

Precision: low

Query: OverflowDestination.ql
/**
 * @name Copy function using source size
 * @description Calling a copy operation with a size derived from the source
 *              buffer instead of the destination buffer may result in a buffer overflow.
 * @kind problem
 * @id cpp/overflow-destination
 * @problem.severity warning
 * @precision low
 * @tags reliability
 *       security
 *       external/cwe/cwe-119
 *       external/cwe/cwe-131
 */
import cpp
import semmle.code.cpp.security.TaintTracking

/**
 * Holds if `fc` is a call to a copy operation where the size argument contains
 * a reference to the source argument.  For example:
 * ```
 *   memcpy(dest, src, sizeof(src));
 * ```
 */
predicate sourceSized(FunctionCall fc, Expr src)
{
  exists(string name |
    (name = "strncpy" or name = "strncat" or name = "memcpy" or name = "memmove") and
    fc.getTarget().hasQualifiedName(name))
  and
  exists(Expr dest, Expr size, Variable v |
    fc.getArgument(0) = dest and fc.getArgument(1) = src and fc.getArgument(2) = size and
    src = v.getAnAccess() and size.getAChild+() = v.getAnAccess() and

    // exception: `dest` is also referenced in the size argument
    not exists(Variable other |
      dest = other.getAnAccess() and size.getAChild+() = other.getAnAccess())
    and

    // exception: `src` and `dest` are both arrays of the same type and size
    not exists(ArrayType srctype, ArrayType desttype |
      dest.getType().getUnderlyingType() = desttype and
      src.getType().getUnderlyingType() = srctype and
      desttype.getBaseType().getUnderlyingType() = srctype.getBaseType().getUnderlyingType() and
      desttype.getArraySize() = srctype.getArraySize()))
}

from FunctionCall fc, Expr vuln, Expr taintSource
where sourceSized(fc, vuln)
  and tainted(taintSource, vuln)
select fc, "To avoid overflow, this operation should be bounded by destination-buffer size, not source-buffer size."

The bounded copy functions memcpy, memmove, strncpy, strncat accept a size argument. You should call these functions with a size argument that is derived from the size of the destination buffer. Using a size argument that is derived from the source buffer may cause a buffer overflow. Buffer overflows can lead to anything from a segmentation fault to a security vulnerability.

Recommendation

Check the highlighted function calls carefully. Ensure that the size parameter is derived from the size of the destination buffer, and not the source buffer.

This check is an approximation, so some results may not be actual defects in the program. It is not possible in general to compute the exact value of the variable without running the program with all possible input data.

Example

The code below shows an example where strncpy is called incorrectly, without checking the size of the destination buffer. In the second example the call has been updated to include the size of the destination buffer.

int main(int argc, char* argv[]) {
	char param[20];
	char *arg1;

	arg1 = argv[1];

	//wrong: only uses the size of the source (argv[1]) when using strncpy
	strncpy(param, arg1, strlen(arg1));

	//correct: uses the size of the destination array as well
	strncpy(param, arg1, min(strlen(arg1), sizeof(param) -1));
}

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.