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

Name: Potential improper null termination

Description: Using a string that may not be null terminated as an argument to a string function can result in buffer overflow or buffer over-read.

ID: cpp/improper-null-termination

Kind: problem

Severity: warning

Query: ImproperNullTermination.ql
/**
 * @name Potential improper null termination
 * @description Using a string that may not be null terminated as an argument
 *              to a string function can result in buffer overflow or buffer over-read.
 * @kind problem
 * @id cpp/improper-null-termination
 * @problem.severity warning
 * @tags security
 *       external/cwe/cwe-170
 *       external/cwe/cwe-665
 */

import cpp
import semmle.code.cpp.controlflow.LocalScopeVariableReachability
import semmle.code.cpp.commons.NullTermination

/**
 * A declaration of a local variable that leaves the variable uninitialized.
 */
DeclStmt declWithNoInit(LocalVariable v) {
  result.getADeclaration() = v and
  not exists(v.getInitializer())
}

class ImproperNullTerminationReachability extends LocalScopeVariableReachabilityWithReassignment {
  ImproperNullTerminationReachability() { this = "ImproperNullTerminationReachability" }

  override predicate isSourceActual(ControlFlowNode node, LocalScopeVariable v) {
    node = declWithNoInit(v)
    or
    exists(Call c, VariableAccess va |
      c = node and
      c.getTarget().hasName("readlink") and
      c.getArgument(1) = va and
      va.getTarget() = v
    )
  }

  override predicate isSinkActual(ControlFlowNode node, LocalScopeVariable v) {
    node.(VariableAccess).getTarget() = v and
    variableMustBeNullTerminated(node)
  }

  override predicate isBarrier(ControlFlowNode node, LocalScopeVariable v) {
    exprDefinition(v, node, _) or
    mayAddNullTerminator(node, v.getAnAccess()) or
    isSinkActual(node, v) // only report first use
  }
}

from ImproperNullTerminationReachability r, LocalVariable v, VariableAccess va
where r.reaches(_, v, va)
select va, "Variable $@ may not be null terminated.", v, v.getName()

Built-in C string functions such as strcat require that their input string arguments are null terminated. If the input string arguments are not null terminated, these functions will read/write beyond the length of the buffer containing the string, resulting in either buffer over-read or buffer overflow, respectively.

Recommendation

Review the code and consider whether the variable that holds the string should have an initializer or whether some path through the program fails to null terminate the string.

Example

The destination variable dest used in the call to strcat does not (necessarily) contain a null terminator. Consequently, the call to strcat may result in a buffer overflow.

char source[100];
memset(source, 'A', 100-1);
source[100-1] = '\0'; // null terminate source

char dest[200];
memset(dest, 'B', 100-1);

strcat(dest, source);

In the revised example, dest is properly null terminated before the the call to strcat.

char source[100];
memset(source, 'A', 100-1);
source[100-1] = '\0'; // null terminate source

char dest[200];
memset(dest, 'B', 100-1);
dest[100-1] = '\0'; // null terminate destination

strcat(dest, source);

References
  • B. Chess and J. West, Secure Programming with Static Analysis, 6.2 Maintaining the Null Terminator. Addison-Wesley. 2007.
  • Linux Programmer's Manual: STRCAT(3).
  • Common Weakness Enumeration: CWE-170.
  • Common Weakness Enumeration: CWE-665.