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

Name: Use of string copy function in a condition

Description: The return value for strcpy, strncpy, or related string copy functions have no reserved return value to indicate an error. Using them in a condition is likely to be a logic error.

ID: cpp/string-copy-return-value-as-boolean

Kind: problem

Severity: error

Precision: high

Query: UsingStrcpyAsBoolean.ql
/**
 * @name Use of string copy function in a condition
 * @description The return value for strcpy, strncpy, or related string copy
 *              functions have no reserved return value to indicate an error.
 *              Using them in a condition is likely to be a logic error.
 * @kind problem
 * @problem.severity error
 * @precision high
 * @id cpp/string-copy-return-value-as-boolean
 * @tags external/microsoft/C6324
 *       correctness
 */

import cpp
import semmle.code.cpp.models.implementations.Strcpy
import semmle.code.cpp.dataflow.DataFlow

predicate isBoolean(Expr e1) {
  exists(Type t1 |
    t1 = e1.getType() and
    (t1.hasName("bool") or t1.hasName("BOOL") or t1.hasName("_Bool"))
  )
}

predicate isStringCopyCastedAsBoolean(FunctionCall func, Expr expr1, string msg) {
  DataFlow::localFlow(DataFlow::exprNode(func), DataFlow::exprNode(expr1)) and
  isBoolean(expr1.getConversion*()) and
  func.getTarget() instanceof StrcpyFunction and
  msg = "Return value of " + func.getTarget().getName() + " used as a Boolean."
}

predicate isStringCopyUsedInLogicalOperationOrCondition(FunctionCall func, Expr expr1, string msg) {
  func.getTarget() instanceof StrcpyFunction and
  (
    (
      // it is being used in an equality or logical operation
      exists(EqualityOperation eop |
        eop = expr1 and
        func = eop.getAnOperand()
      )
      or
      exists(UnaryLogicalOperation ule |
        expr1 = ule and
        func = ule.getOperand()
      )
      or
      exists(BinaryLogicalOperation ble |
        expr1 = ble and
        func = ble.getAnOperand()
      )
    ) and
    msg = "Return value of " + func.getTarget().getName() + " used in a logical operation."
    or
    // or the string copy function is used directly as the conditional expression
    (
      exists(ConditionalStmt condstmt |
        func = condstmt.getControllingExpr() and
        expr1 = func
      )
      or
      exists(ConditionalExpr ce |
        expr1 = ce and
        func = ce.getCondition()
      )
    ) and
    msg = "Return value of " + func.getTarget().getName() +
        " used directly in a conditional expression."
  )
}

from FunctionCall func, Expr expr1, string msg
where
  (
    isStringCopyCastedAsBoolean(func, expr1, msg) and
    not isStringCopyUsedInLogicalOperationOrCondition(func, _, _)
  )
  or
  isStringCopyUsedInLogicalOperationOrCondition(func, expr1, msg)
select expr1, msg

This query identifies calls to string copy functions used in conditions, either directly or as part of an equality operator or logical operator. The most common string copy functions always return their destination parameter and do not have a return value reserved to indicate an error. Therefore, such a function call always evaluates to true in a Boolean context.

The string copy functions that the rule takes into consideration are:

  • strcpy
  • wcscpy
  • _mbscpy
  • strncpy
  • _strncpy_l
  • wcsncpy
  • _wcsncpy_l
  • _mbsncpy
  • _mbsncpy_l

NOTE: It is highly recommended to consider using a more secure version of string manipulation functions such as as strcpy_s.

Recommendation

Check to ensure that the flagged expressions are not typos.

If a string comparison is intended, change the function to the appropriate string comparison function.

If a string copy is really intended, very likely a secure version of the string copy function such as strcpy_s was intended instead of the insecure version of the string copy function.

Example

if(strcpy(szbuf1, "Manager") == 0) // most likely strcmp was intended instead of strcpy
{
	// ...
}

References