Name: Potentially unsafe call to strncat

Description:

Calling 'strncat' with the size of the destination buffer as the third argument may result in a buffer overflow.

ID: cpp/unsafe-strncat

Kind: problem

Severity: warning

Precision: medium

/**
 * @name Potentially unsafe call to strncat
 * @description Calling 'strncat' with the size of the destination buffer
 *              as the third argument may result in a buffer overflow.
 * @kind problem
 * @problem.severity warning
 * @precision medium
 * @id cpp/unsafe-strncat
 * @tags reliability
 *       correctness
 *       security
 *       external/cwe/cwe-676
 *       external/cwe/cwe-119
 *       external/cwe/cwe-251
 */
import cpp
import Buffer

from FunctionCall fc, VariableAccess va1, VariableAccess va2
where fc.getTarget().(Function).hasName("strncat") and
      va1 = fc.getArgument(0) and
      va2 = fc.getArgument(2).(BufferSizeExpr).getArg() and
      va1.getTarget() = va2.getTarget()
select fc, "Potentially unsafe call to strncat."

The standard library function strncat appends a source string to a target string. The third argument defines the maximum number of characters to append and should be less than or equal to the remaining space in the destination buffer. Calls of the form strncat(dest, src, strlen(dest)) or strncat(dest, src, sizeof(dest)) set the third argument to the entire size of the destination buffer. Executing a call of this type may cause a buffer overflow unless the buffer is known to be empty. Buffer overflows can lead to anything from a segmentation fault to a security vulnerability.

Recommendation

Check the highlighted function calls carefully to ensure that no buffer overflow is possible. For a more robust solution, consider updating the function call to include the remaining space in the destination buffer.

Example
     1strncat(dest, src, strlen(dest)); //wrong: should use remaining size of dest
     2
     3strncat(dest, src, sizeof(dest)); //wrong: should use remaining size of dest. 
     4                                  //Also fails if dest is a pointer and not an array.
strncat(dest, src, strlen(dest)); //wrong: should use remaining size of dest

strncat(dest, src, sizeof(dest)); //wrong: should use remaining size of dest. 
                                  //Also fails if dest is a pointer and not an array.
References