Fix strncat/wcscat param definition and add rules

Author: Nikita Kraiouchkine

c/cert/src/rules/ARR38-C/LibraryFunctionArgumentOutOfBounds.ql

@@ -1,7 +1,8 @@
 /**
  * @id c/cert/library-function-argument-out-of-bounds
  * @name ARR38-C: Guarantee that library functions do not form invalid pointers
- * @description 
+ * @description Passing out-of-bounds pointers or erroneous size arguments to standard library
+ *              functions can result in out-of-bounds accesses and other undefined behavior.
  * @kind problem
  * @precision high
  * @problem.severity error

c/cert/test/rules/ARR38-C/LibraryFunctionArgumentOutOfBounds.expected

@@ -15,13 +15,9 @@
 | test.c:153:5:153:10 | call to strcat | The $@ passed to strcat might not be null-terminated. | test.c:153:12:153:15 | buf1 | argument | test.c:153:12:153:15 | buf1 |  |
 | test.c:158:5:158:10 | call to strcat | The size of the $@ passed to strcat is 6 bytes, but the size of the $@ is only 5 bytes. | test.c:158:24:158:30 | 12345 | read buffer | test.c:158:12:158:19 | call to get_ca_5 | write buffer |
 | test.c:160:5:160:10 | call to strcat | The size of the $@ passed to strcat is 5 bytes, but the size of the $@ is only 4 bytes. | test.c:160:28:160:33 | 1234 | read buffer | test.c:160:12:160:25 | ... + ... | write buffer |
-| test.c:183:5:183:11 | call to wcsncat | The size of the $@ passed to wcsncat is 5 bytes, but the $@ is 20 bytes. | test.c:183:13:183:20 | call to get_ca_5 | write buffer | test.c:183:35:183:35 | 5 | size argument |
 | test.c:183:5:183:11 | call to wcsncat | The size of the $@ passed to wcsncat is 24 bytes, but the size of the $@ is only 5 bytes. | test.c:183:25:183:32 | 12345 | read buffer | test.c:183:13:183:20 | call to get_ca_5 | write buffer |
-| test.c:184:5:184:11 | call to wcsncat | The size of the $@ passed to wcsncat is 5 bytes, but the $@ is 16 bytes. | test.c:184:13:184:20 | call to get_ca_5 | write buffer | test.c:184:34:184:34 | 4 | size argument |
 | test.c:184:5:184:11 | call to wcsncat | The size of the $@ passed to wcsncat is 20 bytes, but the size of the $@ is only 5 bytes. | test.c:184:25:184:31 | 1234 | read buffer | test.c:184:13:184:20 | call to get_ca_5 | write buffer |
-| test.c:185:5:185:11 | call to wcsncat | The size of the $@ passed to wcsncat is 1 bytes, but the $@ is 16 bytes. | test.c:185:13:185:26 | ... + ... | write buffer | test.c:185:38:185:38 | 4 | size argument |
 | test.c:185:5:185:11 | call to wcsncat | The size of the $@ passed to wcsncat is 20 bytes, but the size of the $@ is only 1 bytes. | test.c:185:29:185:35 | 1234 | read buffer | test.c:185:13:185:26 | ... + ... | write buffer |
-| test.c:186:5:186:11 | call to wcsncat | The size of the $@ passed to wcsncat is 5 bytes, but the $@ is 8 bytes. | test.c:186:13:186:20 | call to get_ca_5 | write buffer | test.c:186:32:186:32 | 2 | size argument |
 | test.c:186:5:186:11 | call to wcsncat | The size of the $@ passed to wcsncat is 12 bytes, but the size of the $@ is only 5 bytes. | test.c:186:25:186:29 | 12 | read buffer | test.c:186:13:186:20 | call to get_ca_5 | write buffer |
 | test.c:191:5:191:10 | call to strcmp | The $@ passed to strcmp might not be null-terminated. | test.c:191:22:191:28 | ca5_bad | argument | test.c:191:22:191:28 | ca5_bad |  |
 | test.c:193:5:193:10 | call to strcmp | The $@ passed to strcmp might not be null-terminated. | test.c:193:12:193:18 | ca5_bad | argument | test.c:193:12:193:18 | ca5_bad |  |

c/common/src/codingstandards/c/OutOfBounds.qll

@@ -18,7 +18,7 @@ import semmle.code.cpp.security.BufferWrite
 
 module OOB {
   bindingset[name, result]
-  private string getNameOrInternalName(string name) {
+  string getNameOrInternalName(string name) {
     result = name or
     result.regexpMatch("__.*_+" + name + "_.*")
   }
@@ -162,8 +162,8 @@ module OOB {
       name = ["strncat", "wcsncat"] and
       dst = 0 and
       src = 1 and
-      src_sz = -1 and
-      dst_sz = 2
+      src_sz = 2 and
+      dst_sz = -1
       or
       name = ["snprintf", "vsnprintf", "swprintf", "vswprintf"] and
       dst = 0 and
@@ -362,6 +362,11 @@ module OOB {
    */
   class StrncatLibraryFunction extends StringConcatenationFunctionLibraryFunction {
     StrncatLibraryFunction() { this.getName() = getNameOrInternalName(["strncat", "wcsncat"]) }
+
+    override predicate getALengthParameterIndex(int i) {
+      // `strncat` and `wcsncat` exclude the size of a null terminator
+      i = 2
+    }
   }
 
   /**

c/misra/src/rules/RULE-21-17/StringFunctionPointerArgumentOutOfBounds.ql

@@ -0,0 +1,27 @@
+/**
+ * @id c/misra/string-function-pointer-argument-out-of-bounds
+ * @name RULE-21-17: Use of the string handling functions from <string.h> shall not result in accesses beyond the bounds
+ * @description Use of string manipulation functions from <string.h> with improper buffer sizes can
+ *              result in out-of-bounds buffer accesses.
+ * @kind problem
+ * @precision high
+ * @problem.severity error
+ * @tags external/misra/id/rule-21-17
+ *       correctness
+ *       security
+ *       external/misra/obligation/mandatory
+ */
+
+import cpp
+import codingstandards.c.misra
+import codingstandards.c.OutOfBounds
+
+class RULE_21_17_Subset_FC = OOB::SimpleStringLibraryFunctionCall;
+
+from
+  RULE_21_17_Subset_FC fc, string message, Expr bufferArg, string bufferArgStr,
+  Expr sizeOrOtherBufferArg, string otherStr
+where
+  not isExcluded(fc, OutOfBoundsPackage::stringFunctionPointerArgumentOutOfBoundsQuery()) and
+  OOB::problems(fc, message, bufferArg, bufferArgStr, sizeOrOtherBufferArg, otherStr)
+select fc, message, bufferArg, bufferArgStr, sizeOrOtherBufferArg, otherStr

c/misra/src/rules/RULE-21-18/StringLibrarySizeArgumentOutOfBounds.ql

@@ -0,0 +1,35 @@
+/**
+ * @id c/misra/string-library-size-argument-out-of-bounds
+ * @name RULE-21-18: The size_t argument passed to any function in <string.h> shall have an appropriate value
+ * @description Passing a size_t argument that is non-positive or greater than the size of the
+ *              smallest buffer argument to any function in <string.h> may result in out-of-bounds
+ *              buffer accesses.
+ * @kind problem
+ * @precision high
+ * @problem.severity error
+ * @tags external/misra/id/rule-21-18
+ *       correctness
+ *       security
+ *       external/misra/obligation/mandatory
+ */
+
+import cpp
+import codingstandards.c.misra
+import codingstandards.c.OutOfBounds
+
+class RULE_21_18_Subset_FC extends OOB::BufferAccessLibraryFunctionCall {
+  RULE_21_18_Subset_FC() {
+    this.getTarget().getName() =
+      OOB::getNameOrInternalName([
+          "mem" + ["chr", "cmp", "cpy", "move", "set"], "str" + ["ncat", "ncmp", "ncpy", "xfrm"]
+        ])
+  }
+}
+
+from
+  RULE_21_18_Subset_FC fc, string message, Expr bufferArg, string bufferArgStr,
+  Expr sizeOrOtherBufferArg, string otherStr
+where
+  not isExcluded(fc, OutOfBoundsPackage::stringLibrarySizeArgumentOutOfBoundsQuery()) and
+  OOB::problems(fc, message, bufferArg, bufferArgStr, sizeOrOtherBufferArg, otherStr)
+select fc, message, bufferArg, bufferArgStr, sizeOrOtherBufferArg, otherStr

c/misra/test/rules/RULE-21-17/StringFunctionPointerArgumentOutOfBounds.expected

@@ -0,0 +1 @@
+No expected results have yet been specified

c/misra/test/rules/RULE-21-17/StringFunctionPointerArgumentOutOfBounds.qlref

@@ -0,0 +1 @@
+rules/RULE-21-17/StringFunctionPointerArgumentOutOfBounds.ql

c/misra/test/rules/RULE-21-18/StringLibrarySizeArgumentOutOfBounds.expected

@@ -0,0 +1 @@
+No expected results have yet been specified

c/misra/test/rules/RULE-21-18/StringLibrarySizeArgumentOutOfBounds.qlref

@@ -0,0 +1 @@
+rules/RULE-21-18/StringLibrarySizeArgumentOutOfBounds.ql

rule_packages/c/OutOfBounds.json

@@ -26,7 +26,7 @@
       },
       "queries": [
         {
-          "description": "",
+          "description": "Passing out-of-bounds pointers or erroneous size arguments to standard library functions can result in out-of-bounds accesses and other undefined behavior.",
           "kind": "problem",
           "name": "Guarantee that library functions do not form invalid pointers",
           "precision": "high",