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

Name: Time-of-check time-of-use filesystem race condition

Description: Separately checking the state of a file before operating on it may allow an attacker to modify the file between the two operations.

ID: cpp/toctou-race-condition

Kind: problem

Severity: warning

Precision: medium

Query: TOCTOUFilesystemRace.ql
/**
 * @name Time-of-check time-of-use filesystem race condition
 * @description Separately checking the state of a file before operating
 *              on it may allow an attacker to modify the file between
 *              the two operations.
 * @kind problem
 * @problem.severity warning
 * @precision medium
 * @id cpp/toctou-race-condition
 * @tags security
 *       external/cwe/cwe-367
 */
import cpp
import semmle.code.cpp.controlflow.Guards

/**
 * An operation on a filename.
 *
 * Note: we're not interested in operations on file descriptors, as they
 * are better behaved.
 */
FunctionCall filenameOperation(Expr path) {
  exists(string name | name = result.getTarget().getName() |
    (
      (
        name = "remove" or
        name = "unlink" or
        name = "rmdir" or
        name = "rename" or
        name = "chmod" or
        name = "chown" or
        name = "fopen" or
        name = "open" or
        name = "freopen" or
        name = "_open" or
        name = "_wopen" or
        name = "_wfopen"
      )
      and
      result.getArgument(0) = path
    )
    or
    (
      (
        name = "fopen_s" or
        name = "wfopen_s"
      )
      and
      result.getArgument(1) = path
    )
  )
}

/**
 * A use of `access` (or similar) on a filename.
 */
FunctionCall accessCheck(Expr path) {
  exists(string name | name = result.getTarget().getName() |
    name = "access" or
    name = "_access" or
    name = "_waccess" or
    name = "_access_s" or
    name = "_waccess_s"
  )
  and
  path = result.getArgument(0)
}

/**
 * A use of `stat` (or similar) on a filename.
 */
FunctionCall stat(Expr path, Expr buf) {
  exists(string name | name = result.getTarget().getName() |
    name = "stat" or
    name = "lstat" or
    name = "fstat" or
    name.matches("\\_stat%") or
    name.matches("\\_wstat%")
  )
  and
  path = result.getArgument(0)
  and
  buf = result.getArgument(1)
}

/**
 * Holds if `use` points to `source`, either by being the same or by
 * one step of variable indirection.
 */
predicate referenceTo(Expr source, Expr use) {
  source = use
  or
  exists(SsaDefinition def, LocalScopeVariable v |
         def.getAnUltimateDefiningValue(v) = source and def.getAUse(v) = use)
}

from FunctionCall fc, Expr check, Expr checkUse, Expr opUse
where // checkUse looks like a check on a filename
      (
        // either:
        // an access check
        check = accessCheck(checkUse)
        or
        // a stat
        check = stat(checkUse, _)
        or
        // another filename operation (null pointers can indicate errors)
        check = filenameOperation(checkUse)
        or
        // access to a member variable on the stat buf
        // (morally, this should be a use-use pair, but it seems unlikely
        // that this variable will get reused in practice)
        exists(Variable buf |
               exists(stat(checkUse, buf.getAnAccess())) |
               check.(VariableAccess).getQualifier() = buf.getAnAccess())
      )
  and // checkUse and opUse refer to the same SSA variable
      exists(SsaDefinition def, LocalScopeVariable v |
             def.getAUse(v) = checkUse and def.getAUse(v) = opUse)
  and // opUse looks like an operation on a filename
      fc = filenameOperation(opUse)
  and // the return value of check is used (possibly with one step of
      // variable indirection) in a guard which controls fc
      exists(GuardCondition guard |
             referenceTo(check, guard.getAChild*()) |
             guard.controls(fc.(ControlFlowNode).getBasicBlock(), _)
      )
select fc, "The $@ being operated upon was previously $@, but the underlying file may have been changed since then.", opUse, "filename", check, "checked"

Often it is necessary to check the state of a file before using it. These checks usually take a file name to be checked, and if the check returns positively, then the file is opened or otherwise operated upon.

However, in the time between the check and the operation, the underlying file referenced by the file name could be changed by an attacker, causing unexpected behavior.

Recommendation

Wherever possible, use functions that operate on file descriptors rather than file names (for example, fchmod rather than chmod).

For access checks, you can temporarily change the UID and GID to that of the user whose permissions are being checked, and then perform the operation. This has the effect of "atomically" combining a permissions check with the operation.

If file-system locking tools are available on your platform, then locking the file before the check can prevent an unexpected update. However, note that on some platforms (for example, Unix) file-system locks are typically advisory, and so can be ignored by an attacker.

Example

The following example shows a case where a file is opened and then, if the opening was successful, its permissions are changed with chmod. However, an attacker might change the target of the file name between the initial opening and the permissions change, potentially changing the permissions of a different file.

char *file_name;
FILE *f_ptr;
 
/* Initialize file_name */
 
f_ptr = fopen(file_name, "w");
if (f_ptr == NULL)  {
  /* Handle error */
}
 
/* ... */
 
if (chmod(file_name, S_IRUSR) == -1) {
  /* Handle error */
}

This can be avoided by using fchmod with the file descriptor that was received from opening the file. This ensures that the permissions change is applied to the very same file that was opened.

char *file_name;
int fd;
 
/* Initialize file_name */
 
fd = open(
  file_name,
  O_WRONLY | O_CREAT | O_EXCL,
  S_IRWXU
);
if (fd == -1) {
  /* Handle error */
}
 
/* ... */
 
if (fchmod(fd, S_IRUSR) == -1) {
  /* Handle error */
}

References