Use range analysis to detect realloc() where size may be zero, vs, is exactly zero.

MichaelRFairhurst · MichaelRFairhurst · commit 9dc3c2a154d7 · 2024-10-18T18:15:06.000-07:00
diff --git a/c/misra/src/rules/RULE-1-5/SizeInReallocCallIsZero.ql b/c/misra/src/rules/RULE-1-5/SizeInReallocCallIsZero.ql
@@ -1,6 +1,6 @@
 /**
- * @id c/misra/call-to-realloc-with-size-zero
- * @name RULE-1-5: Disallowed size argument value equal to zero in call to realloc
+ * @id c/misra/size-in-realloc-call-is-zero
+ * @name RULE-1-5: Size argument value in realloc call is equal zero
  * @description Invoking realloc with a size argument set to zero is implementation-defined behavior
  *              and declared as an obsolete feature in C18.
  * @kind problem
@@ -15,11 +15,12 @@
 import cpp
 import codingstandards.c.misra
 import semmle.code.cpp.rangeanalysis.new.RangeAnalysis
+import codingstandards.cpp.Realloc
 
-from FunctionCall call, Expr arg
+from ReallocCall call
 where
-  not isExcluded(call, Language4Package::callToReallocWithSizeZeroQuery()) and
-  call.getTarget().hasGlobalOrStdName("realloc") and
-  arg = call.getArgument(1) and
-  upperBound(arg) = 0
-select arg, "Calling realloc with size zero results in implementation-defined behavior."
+  not isExcluded(call, Language4Package::sizeInReallocCallIsZeroQuery()) and
+  call.sizeIsExactlyZero()
+select call,
+  "Size argument '$@' may equal zero in realloc call, resulting in obsolescent and/or implementation-defined behavior.",
+  call.getSizeArgument(), call.getSizeArgument().toString()
diff --git a/c/misra/src/rules/RULE-1-5/SizeInReallocCallMayBeZero.ql b/c/misra/src/rules/RULE-1-5/SizeInReallocCallMayBeZero.ql
@@ -0,0 +1,26 @@
+/**
+ * @id c/misra/size-in-realloc-call-may-be-zero
+ * @name RULE-1-5: Size argument value in realloc call may equal zero
+ * @description Invoking realloc with a size argument set to zero is implementation-defined behavior
+ *              and declared as an obsolete feature in C18.
+ * @kind problem
+ * @precision medium
+ * @problem.severity error
+ * @tags external/misra/id/rule-1-5
+ *       correctness
+ *       external/misra/c/2012/amendment3
+ *       external/misra/obligation/required
+ */
+
+import cpp
+import codingstandards.c.misra
+import codingstandards.cpp.Realloc
+
+from ReallocCall call
+where
+  not isExcluded(call, Language4Package::sizeInReallocCallMayBeZeroQuery()) and
+  call.sizeMayBeZero() and
+  not call.sizeIsExactlyZero()
+select call,
+  "Size argument '$@' equals zero in realloc call, resulting in obsolescent and/or implementation-defined behavior.",
+  call.getSizeArgument(), call.getSizeArgument().toString()
diff --git a/c/misra/test/rules/RULE-1-5/CallToObsolescentFunctionGets.expected b/c/misra/test/rules/RULE-1-5/CallToObsolescentFunctionGets.expected
@@ -1 +1 @@
-| test.c:36:3:36:6 | call to gets | Call to obsolescent function 'gets'. |
+| test.c:37:3:37:6 | call to gets | Call to obsolescent function 'gets'. |
diff --git a/c/misra/test/rules/RULE-1-5/CallToReallocWithSizeZero.expected b/c/misra/test/rules/RULE-1-5/CallToReallocWithSizeZero.expected
diff --git a/c/misra/test/rules/RULE-1-5/CallToReallocWithSizeZero.qlref b/c/misra/test/rules/RULE-1-5/CallToReallocWithSizeZero.qlref
diff --git a/c/misra/test/rules/RULE-1-5/InvalidDefineOrUndefOfStdBoolMacro.expected b/c/misra/test/rules/RULE-1-5/InvalidDefineOrUndefOfStdBoolMacro.expected
@@ -1,6 +1,6 @@
-| test.c:21:1:21:14 | #define true 3 | Invalid define of boolean standard macro 'true'. |
-| test.c:22:1:22:15 | #define false 3 | Invalid define of boolean standard macro 'false'. |
-| test.c:23:1:23:18 | #define bool int * | Invalid define of boolean standard macro 'bool'. |
-| test.c:24:1:24:11 | #undef true | Invalid undefine of boolean standard macro 'true'. |
-| test.c:25:1:25:12 | #undef false | Invalid undefine of boolean standard macro 'false'. |
-| test.c:26:1:26:11 | #undef bool | Invalid undefine of boolean standard macro 'bool'. |
+| test.c:22:1:22:14 | #define true 3 | Invalid define of boolean standard macro 'true'. |
+| test.c:23:1:23:15 | #define false 3 | Invalid define of boolean standard macro 'false'. |
+| test.c:24:1:24:18 | #define bool int * | Invalid define of boolean standard macro 'bool'. |
+| test.c:25:1:25:11 | #undef true | Invalid undefine of boolean standard macro 'true'. |
+| test.c:26:1:26:12 | #undef false | Invalid undefine of boolean standard macro 'false'. |
+| test.c:27:1:27:11 | #undef bool | Invalid undefine of boolean standard macro 'bool'. |
diff --git a/c/misra/test/rules/RULE-1-5/SizeInReallocCallIsZero.expected b/c/misra/test/rules/RULE-1-5/SizeInReallocCallIsZero.expected
@@ -0,0 +1 @@
+| test.c:14:3:14:9 | call to realloc | Size argument '$@' may equal zero in realloc call, resulting in obsolescent and/or implementation-defined behavior. | test.c:14:14:14:14 | 0 | 0 |
diff --git a/c/misra/test/rules/RULE-1-5/SizeInReallocCallIsZero.qlref b/c/misra/test/rules/RULE-1-5/SizeInReallocCallIsZero.qlref
@@ -0,0 +1 @@
+rules/RULE-1-5/SizeInReallocCallIsZero.ql
diff --git a/c/misra/test/rules/RULE-1-5/SizeInReallocCallMayBeZero.expected b/c/misra/test/rules/RULE-1-5/SizeInReallocCallMayBeZero.expected
@@ -0,0 +1 @@
+| test.c:15:3:15:9 | call to realloc | Size argument '$@' equals zero in realloc call, resulting in obsolescent and/or implementation-defined behavior. | test.c:15:14:15:15 | p0 | p0 |
diff --git a/c/misra/test/rules/RULE-1-5/SizeInReallocCallMayBeZero.qlref b/c/misra/test/rules/RULE-1-5/SizeInReallocCallMayBeZero.qlref
@@ -0,0 +1 @@
+rules/RULE-1-5/SizeInReallocCallMayBeZero.ql
diff --git a/c/misra/test/rules/RULE-1-5/UngetcCallOnStreamPositionZero.expected b/c/misra/test/rules/RULE-1-5/UngetcCallOnStreamPositionZero.expected
@@ -1,8 +1,8 @@
 edges
-| test.c:38:16:38:20 | call to fopen indirection | test.c:40:15:40:18 | file indirection |
+| test.c:39:16:39:20 | call to fopen indirection | test.c:41:15:41:18 | file indirection |
 nodes
-| test.c:38:16:38:20 | call to fopen indirection | semmle.label | call to fopen indirection |
-| test.c:40:15:40:18 | file indirection | semmle.label | file indirection |
+| test.c:39:16:39:20 | call to fopen indirection | semmle.label | call to fopen indirection |
+| test.c:41:15:41:18 | file indirection | semmle.label | file indirection |
 subpaths
 #select
-| test.c:40:15:40:18 | file indirection | test.c:38:16:38:20 | call to fopen indirection | test.c:40:15:40:18 | file indirection | Obsolescent call to ungetc on file stream $@ at position zero. | test.c:38:16:38:20 | call to fopen indirection | call to fopen indirection |
+| test.c:41:15:41:18 | file indirection | test.c:39:16:39:20 | call to fopen indirection | test.c:41:15:41:18 | file indirection | Obsolescent call to ungetc on file stream $@ at position zero. | test.c:39:16:39:20 | call to fopen indirection | call to fopen indirection |
diff --git a/c/misra/test/rules/RULE-1-5/UseOfObsoleteMacroAtomicVarInit.expected b/c/misra/test/rules/RULE-1-5/UseOfObsoleteMacroAtomicVarInit.expected
@@ -1 +1 @@
-| test.c:28:18:28:36 | ATOMIC_VAR_INIT(value) | Usage of macro ATOMIC_VAR_INIT() is declared obscelescent in C18, and discouraged in earlier C versions. |
+| test.c:29:18:29:36 | ATOMIC_VAR_INIT(value) | Usage of macro ATOMIC_VAR_INIT() is declared obscelescent in C18, and discouraged in earlier C versions. |
diff --git a/c/misra/test/rules/RULE-1-5/test.c b/c/misra/test/rules/RULE-1-5/test.c
@@ -3,15 +3,16 @@
 #include "stdio.h"
 #include "stdlib.h"
 
-void f1(void) {
+void f1(int p0) {
   // malloc() is not obsolete, though it is banned by Rule 21.3
   int *t = malloc(10); // COMPLIANT
 
-  // Obsolete usage of realloc.
-  realloc(t, 0); // NON-COMPLIANT
-
   // Valid usage of realloc, but all use of realloc is banned by Rule 21.3
   realloc(t, 20); // NON-COMPLIANT
+
+  // Obsolete usage of realloc.
+  realloc(t, 0);  // NON-COMPLIANT
+  realloc(t, p0); // NON-COMPLIANT
 }
 
 extern const int g1; // COMPLIANT
diff --git a/cpp/common/src/codingstandards/cpp/Realloc.qll b/cpp/common/src/codingstandards/cpp/Realloc.qll
@@ -0,0 +1,18 @@
+import cpp
+import codingstandards.cpp.CodingStandards
+
+class ReallocCall extends FunctionCall {
+  ReallocCall() { getTarget().hasGlobalOrStdName("realloc") }
+
+  Expr getSizeArgument() { result = getArgument(1) }
+
+  predicate sizeIsExactlyZero() {
+    upperBound(getSizeArgument().getConversion()) = 0 and
+    lowerBound(getSizeArgument().getConversion()) = 0
+  }
+
+  predicate sizeMayBeZero() {
+    upperBound(getSizeArgument().getConversion()) >= 0 and
+    lowerBound(getSizeArgument().getConversion()) <= 0
+  }
+}
diff --git a/cpp/common/src/codingstandards/cpp/exclusions/c/Language4.qll b/cpp/common/src/codingstandards/cpp/exclusions/c/Language4.qll
@@ -11,7 +11,8 @@ newtype Language4Query =
   TInvalidDefineOrUndefOfStdBoolMacroQuery() or
   TCallToObsolescentFunctionGetsQuery() or
   TUngetcCallOnStreamPositionZeroQuery() or
-  TCallToReallocWithSizeZeroQuery()
+  TSizeInReallocCallMayBeZeroQuery() or
+  TSizeInReallocCallIsZeroQuery()
 
 predicate isLanguage4QueryMetadata(Query query, string queryId, string ruleId, string category) {
   query =
@@ -78,11 +79,20 @@ predicate isLanguage4QueryMetadata(Query query, string queryId, string ruleId, s
   category = "required"
   or
   query =
-    // `Query` instance for the `callToReallocWithSizeZero` query
-    Language4Package::callToReallocWithSizeZeroQuery() and
+    // `Query` instance for the `sizeInReallocCallMayBeZero` query
+    Language4Package::sizeInReallocCallMayBeZeroQuery() and
   queryId =
-    // `@id` for the `callToReallocWithSizeZero` query
-    "c/misra/call-to-realloc-with-size-zero" and
+    // `@id` for the `sizeInReallocCallMayBeZero` query
+    "c/misra/size-in-realloc-call-may-be-zero" and
+  ruleId = "RULE-1-5" and
+  category = "required"
+  or
+  query =
+    // `Query` instance for the `sizeInReallocCallIsZero` query
+    Language4Package::sizeInReallocCallIsZeroQuery() and
+  queryId =
+    // `@id` for the `sizeInReallocCallIsZero` query
+    "c/misra/size-in-realloc-call-is-zero" and
   ruleId = "RULE-1-5" and
   category = "required"
 }
@@ -137,10 +147,17 @@ module Language4Package {
       TQueryC(TLanguage4PackageQuery(TUngetcCallOnStreamPositionZeroQuery()))
   }
 
-  Query callToReallocWithSizeZeroQuery() {
+  Query sizeInReallocCallMayBeZeroQuery() {
+    //autogenerate `Query` type
+    result =
+      // `Query` type for `sizeInReallocCallMayBeZero` query
+      TQueryC(TLanguage4PackageQuery(TSizeInReallocCallMayBeZeroQuery()))
+  }
+
+  Query sizeInReallocCallIsZeroQuery() {
     //autogenerate `Query` type
     result =
-      // `Query` type for `callToReallocWithSizeZero` query
-      TQueryC(TLanguage4PackageQuery(TCallToReallocWithSizeZeroQuery()))
+      // `Query` type for `sizeInReallocCallIsZero` query
+      TQueryC(TLanguage4PackageQuery(TSizeInReallocCallIsZeroQuery()))
   }
 }
diff --git a/rule_packages/c/Language4.json b/rule_packages/c/Language4.json
@@ -102,10 +102,22 @@
         {
           "description": "Invoking realloc with a size argument set to zero is implementation-defined behavior and declared as an obsolete feature in C18.",
           "kind": "problem",
-          "name": "Disallowed size argument value equal to zero in call to realloc",
+          "name": "Size argument value in realloc call may equal zero",
+          "precision": "medium",
+          "severity": "error",
+          "short_name": "SizeInReallocCallMayBeZero",
+          "tags": [
+            "correctness",
+            "external/misra/c/2012/amendment3"
+          ]
+        },
+        {
+          "description": "Invoking realloc with a size argument set to zero is implementation-defined behavior and declared as an obsolete feature in C18.",
+          "kind": "problem",
+          "name": "Size argument value in realloc call is equal zero",
           "precision": "very-high",
           "severity": "error",
-          "short_name": "CallToReallocWithSizeZero",
+          "short_name": "SizeInReallocCallIsZero",
           "tags": [
             "correctness",
             "external/misra/c/2012/amendment3"

Original file line number	Diff line number	Diff line change
`@@ -1 +1 @@`
`1`		`-\| test.c:36:3:36:6 \| call to gets \| Call to obsolescent function 'gets'. \|`
	`1`	`+\| test.c:37:3:37:6 \| call to gets \| Call to obsolescent function 'gets'. \|`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+\| test.c:14:3:14:9 \| call to realloc \| Size argument '$@' may equal zero in realloc call, resulting in obsolescent and/or implementation-defined behavior. \| test.c:14:14:14:14 \| 0 \| 0 \|`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+rules/RULE-1-5/SizeInReallocCallIsZero.ql`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+\| test.c:15:3:15:9 \| call to realloc \| Size argument '$@' equals zero in realloc call, resulting in obsolescent and/or implementation-defined behavior. \| test.c:15:14:15:15 \| p0 \| p0 \|`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+rules/RULE-1-5/SizeInReallocCallMayBeZero.ql`