CodeQL queries 1.23

Skip to end of metadata
Go to start of metadata

You are viewing an old version of this page. View the current version.

Compare with Current View Page History

« Previous Version 2 Current »

Name: User-controlled data may not be null terminated

Description: String operations on user-controlled strings can result in buffer overflow or buffer over-read.

ID: cpp/user-controlled-null-termination-tainted

Kind: problem

Severity: warning

Query: ImproperNullTerminationTainted.ql
/**
 * @name User-controlled data may not be null terminated
 * @description String operations on user-controlled strings can result in
 *              buffer overflow or buffer over-read.
 * @kind problem
 * @id cpp/user-controlled-null-termination-tainted
 * @problem.severity warning
 * @tags security
 *       external/cwe/cwe-170
 */

import cpp
import semmle.code.cpp.commons.NullTermination
import semmle.code.cpp.security.TaintTracking

/** A user-controlled expression that may not be null terminated. */
class TaintSource extends VariableAccess {
  TaintSource() {
    exists(SecurityOptions x, string cause |
      this.getTarget() instanceof SemanticStackVariable and
      x.isUserInput(this, cause) |
      cause = "read" or
      cause = "fread" or
      cause = "recv" or
      cause = "recvfrom" or
      cause = "recvmsg"
    )
  }

  /**
   * Holds if `sink` is a tainted variable access that must be null
   * terminated.
   */
  private predicate isSink(VariableAccess sink) {
    tainted(this, sink) and
    variableMustBeNullTerminated(sink)
  }

  /**
   * Holds if this source can reach `va`, possibly using intermediate
   * reassignments.
   */
  private predicate sourceReaches(VariableAccess va) {
    definitionUsePair(_, this, va)
    or
    exists(VariableAccess mid, Expr def |
      sourceReaches(mid) and
      exprDefinition(_, def, mid) and
      definitionUsePair(_, def, va)
    )
  }

  /**
   * Holds if the sink `sink` is reachable both from this source and
   * from `va`, possibly using intermediate reassignments.
   */
  private predicate reachesSink(VariableAccess va, VariableAccess sink) {
    isSink(sink) and
    va = sink
    or
    exists(VariableAccess mid, Expr def |
      reachesSink(mid, sink) and
      exprDefinition(_, def, va) and
      definitionUsePair(_, def, mid)
    )
  }

  /**
   * Holds if `sink` is a tainted variable access that must be null
   * terminated, and no access which null terminates its contents can
   * either reach the sink or be reached from the source. (Ideally,
   * we should instead look for such accesses only on the path from
   * this source to `sink` found via `tainted(source, sink)`.)
   * */
  predicate reaches(VariableAccess sink) {
    isSink(sink) and
    not exists(VariableAccess va |
      va != this and
      va != sink and
      mayAddNullTerminator(_, va) |
      sourceReaches(va)
      or
      reachesSink(va, sink)
    )
  }
}

from TaintSource source, VariableAccess sink
where source.reaches(sink)
select
  sink,
  "$@ flows to here and may not be null terminated.",
  source, "User-provided value"

Built-in C string functions such as strcpy 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

Always guard against user-controlled strings that may not be null terminated. Otherwise, an attacker may be able to supply input without null termination.

Example

In this example, a string is read from a file using the read function. Because the value may be user-controlled, the string may not be null terminated, in which case the call to strcpy will perform a buffer overwrite on cpy, potentially resulting in a program crash.

char buf[BUF_SIZE];
read(fd, buf, BUF_SIZE);
char cpy[BUF_SIZE];
strcpy(cpy, buf);

In the revised example, the number of bytes read is recorded in the variable count. In addition to checking that the read succeeds (the condition count >= 0), a null terminator is inserted before the call to strcpy.

char buf[BUF_SIZE];
int count = read(fd, buf, BUF_SIZE);
if (count >= 0) {
  buf[count < BUF_SIZE ? count : BUF_SIZE - 1] = '\0';
  char cpy[BUF_SIZE];
  strcpy(cpy, buf);
}

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

  • No labels