SideEffectsInArgumentsToUnsafeMacros.ql

/**
 * @id c/cert/side-effects-in-arguments-to-unsafe-macros
 * @name PRE31-C: Avoid side effects in arguments to unsafe macros
 * @description Macro arguments can be expanded multiple times which can cause side-effects to be
 *              evaluated multiple times leading to unexpected program behavior.
 * @kind problem
 * @precision low
 * @problem.severity error
 * @tags external/cert/id/pre31-c
 *       correctness
 *       external/cert/obligation/rule
 */

import cpp
import codingstandards.c.cert
import codingstandards.cpp.Macro
import codingstandards.cpp.SideEffect
import codingstandards.cpp.sideeffect.DefaultEffects
import codingstandards.cpp.sideeffect.Customizations
import semmle.code.cpp.valuenumbering.HashCons

/**
 * Add side-effecting functions to the default set of side-effects.
 */
class FunctionCallEffect extends GlobalSideEffect::Range {
  FunctionCallEffect() {
    exists(Function f |
      // Capture function calls as side-effects
      f = this.(FunctionCall).getTarget() and
      // Excluding __builtin_expect, which is not a side-effecting function
      not f.(BuiltInFunction).getName() = "__builtin_expect" and
      // Excluding common math functions
      not exists(string name |
        name =
          [
            "acos", "asin", "atan", "atan2", "ceil", "cos", "cosh", "exp", "fabs", "floor", "fmod",
            "frexp", "ldexp", "log", "log10", "modf", "pow", "sin", "sinh", "sqrt", "tan", "tanh",
            "cbrt", "erf", "erfc", "exp2", "expm1", "fdim", "fma", "fmax", "fmin", "hypot", "ilogb",
            "lgamma", "llrint", "llround", "log1p", "log2", "logb", "lrint", "lround", "nan",
            "nearbyint", "nextafter", "nexttoward", "remainder", "remquo", "rint", "round",
            "scalbln", "scalbn", "tgamma", "trunc"
          ] and
        f.hasGlobalOrStdName([name, name + "f", name + "l"])
      )
    )
  }
}

/**
 * Add crement operations to the default set of side-effects.
 */
class CrementEffect extends LocalSideEffect::Range {
  CrementEffect() { this instanceof CrementOperation }
}

/**
 * A macro that is considered potentially "unsafe" because one or more arguments are expanded
 * multiple times.
 */
class UnsafeMacro extends FunctionLikeMacro {
  int unsafeArgumentIndex;

  UnsafeMacro() {
    exists(this.getAParameterUse(unsafeArgumentIndex)) and
    // Only consider arguments that are expanded multiple times, and do not consider "stringified" arguments
    count(int indexInBody |
      indexInBody = this.getAParameterUse(unsafeArgumentIndex) and
      not this.getBody().charAt(indexInBody) = "#"
    ) > 1
  }

  int getAnUnsafeArgumentIndex() { result = unsafeArgumentIndex }
}

/**
 * An invocation of a potentially unsafe macro.
 */
class UnsafeMacroInvocation extends MacroInvocation {
  UnsafeMacroInvocation() {
    this.getMacro() instanceof UnsafeMacro and not exists(this.getParentInvocation())
  }

  /**
   * Gets a side-effect for a potentially unsafe argument to the macro.
   */
  SideEffect getSideEffectForUnsafeArg(int index) {
    index = this.getMacro().(UnsafeMacro).getAnUnsafeArgumentIndex() and
    exists(Expr e, string arg |
      e = this.getAnExpandedElement() and
      result = getASideEffect(e) and
      // Unfortunately, there's no semantic way to check whether a particular expression or
      // side-effect generated by a macro came from a particular macro argument. The only
      // information we get is the string of the expanded argument. We therefore do some basic
      // string matching to check whether it looks like this side-effect comes from the given
      // argument
      arg = this.getExpandedArgument(index) and
      (
        // If this is a crement effect, then check that the text of the macro argument includes -- or ++
        result instanceof CrementEffect and
        exists(arg.indexOf(result.(CrementOperation).getOperator()))
        or
        // If this is a functional call effect, then check that the text of the macro argument includes a call to that function
        result instanceof FunctionCallEffect and
        exists(arg.indexOf(result.(FunctionCall).getTarget().getName() + "("))
      )
    )
  }
}

from
  UnsafeMacroInvocation unsafeMacroInvocation, SideEffect sideEffect, int i, string sideEffectDesc
where
  not isExcluded(sideEffect, SideEffects4Package::sideEffectsInArgumentsToUnsafeMacrosQuery()) and
  sideEffect = unsafeMacroInvocation.getSideEffectForUnsafeArg(i) and
  (
    sideEffect instanceof CrementEffect and
    // Do we observe the same side-effect multiple times?
    count(SideEffect equivalentSideEffect |
      equivalentSideEffect = unsafeMacroInvocation.getSideEffectForUnsafeArg(i) and
      hashCons(equivalentSideEffect.(CrementOperation).getOperand()) =
        hashCons(sideEffect.(CrementOperation).getOperand())
    ) > 1 and
    sideEffectDesc = "the use of the " + sideEffect.(CrementOperation).getOperator() + " operator"
    or
    sideEffect instanceof FunctionCallEffect and
    // Do we observe the same side-effect multiple times?
    count(SideEffect equivalentSideEffect |
      equivalentSideEffect = unsafeMacroInvocation.getSideEffectForUnsafeArg(i) and
      equivalentSideEffect.(FunctionCall).getTarget() = sideEffect.(FunctionCall).getTarget()
    ) > 1 and
    sideEffectDesc =
      "a call to the function '" + sideEffect.(FunctionCall).getTarget().getName() + "'"
  )
select sideEffect,
  "Argument " + unsafeMacroInvocation.getUnexpandedArgument(i) + " to unsafe macro '" +
    unsafeMacroInvocation.getMacroName() + "' is expanded to '" +
    unsafeMacroInvocation.getExpandedArgument(i) + "' multiple times and includes " + sideEffectDesc
    + " as a side-effect."