Merge branch 'main' into pointers3

Author: lcartey

.vscode/tasks.json

@@ -210,6 +210,7 @@
                 "Declarations4",
                 "Declarations5",
                 "Declarations6",
+                "Declarations7",
                 "Exceptions1",
                 "Exceptions2",
                 "Expressions",
@@ -222,6 +223,8 @@
                 "Includes",
                 "Initialization",
                 "IntegerConversion",
+                "InvalidMemory1",
+                "InvalidMemory2",
                 "Invariants",
                 "Iterators",
                 "Lambdas",
@@ -230,6 +233,8 @@
                 "Literals",
                 "Loops",
                 "Macros",
+                "Memory1",
+                "Memory2",
                 "Misc",
                 "MoveForward",
                 "Naming",

README.md

@@ -9,7 +9,7 @@ _Carnegie Mellon and CERT are registered trademarks of Carnegie Mellon Universit
 This repository contains CodeQL queries and libraries which support various Coding Standards for the [C++14](https://www.iso.org/standard/64029.html) programming language.
 
 The following coding standards are supported:
-- [AUTOSAR - Guidelines for the use of C++14 language in critical and safety-related systems Release 20-11](https://www.autosar.org/fileadmin/user_upload/standards/adaptive/20-11/AUTOSAR_RS_CPP14Guidelines.pdf)
+- [AUTOSAR - Guidelines for the use of C++14 language in critical and safety-related systems Release 20-11](https://www.autosar.org/fileadmin/standards/adaptive/20-11/AUTOSAR_RS_CPP14Guidelines.pdf)
 - [MISRA C++:2008](https://www.misra.org.uk) (support limited to the rules specified in AUTOSAR 20-11).
 - [SEI CERT C++ Coding Standard: Rules for Developing Safe, Reliable, and Secure Systems (2016 Edition)](https://resources.sei.cmu.edu/library/asset-view.cfm?assetID=494932)
 

c/cert/src/rules/DCL39-C/InformationLeakageAcrossTrustBoundariesC.md

@@ -0,0 +1,22 @@
+/**
+ * @id c/cert/information-leakage-across-trust-boundaries-c
+ * @name DCL39-C: Avoid information leakage when passing a structure across a trust boundary
+ * @description Passing a structure with uninitialized fields or padding bytes can cause information
+ *              to be unintentionally leaked.
+ * @kind problem
+ * @precision medium
+ * @problem.severity error
+ * @tags external/cert/id/dcl39-c
+ *       security
+ *       external/cert/obligation/rule
+ */
+
+import cpp
+import codingstandards.c.cert
+import codingstandards.cpp.rules.informationleakageacrossboundaries.InformationLeakageAcrossBoundaries
+
+class InformationLeakageAcrossTrustBoundariesCQuery extends InformationLeakageAcrossBoundariesSharedQuery {
+  InformationLeakageAcrossTrustBoundariesCQuery() {
+    this = Declarations7Package::informationLeakageAcrossTrustBoundariesCQuery()
+  }
+}

c/cert/src/rules/DCL39-C/InformationLeakageAcrossTrustBoundariesC.ql

@@ -0,0 +1,23 @@
+/**
+ * @id c/cert/do-not-read-uninitialized-memory
+ * @name EXP33-C: Do not read uninitialized memory
+ * @description Using the value of an object with automatic storage duration while it is
+ *              indeterminate is undefined behavior.
+ * @kind problem
+ * @precision medium
+ * @problem.severity error
+ * @tags external/cert/id/exp33-c
+ *       correctness
+ *       security
+ *       external/cert/obligation/rule
+ */
+
+import cpp
+import codingstandards.c.cert
+import codingstandards.cpp.rules.readofuninitializedmemory.ReadOfUninitializedMemory
+
+class DoNotReadUninitializedMemoryQuery extends ReadOfUninitializedMemorySharedQuery {
+  DoNotReadUninitializedMemoryQuery() {
+    this = InvalidMemory1Package::doNotReadUninitializedMemoryQuery()
+  }
+}

c/cert/src/rules/EXP33-C/DoNotReadUninitializedMemory.md

@@ -0,0 +1,21 @@
+/**
+ * @id c/cert/do-not-dereference-null-pointers
+ * @name EXP34-C: Do not dereference null pointers
+ * @description Dereferencing a null pointer leads to undefined behavior.
+ * @kind problem
+ * @precision medium
+ * @problem.severity error
+ * @tags external/cert/id/exp34-c
+ *       correctness
+ *       external/cert/obligation/rule
+ */
+
+import cpp
+import codingstandards.c.cert
+import codingstandards.cpp.rules.dereferenceofnullpointer.DereferenceOfNullPointer
+
+class DoNotDereferenceNullPointersQuery extends DereferenceOfNullPointerSharedQuery {
+  DoNotDereferenceNullPointersQuery() {
+    this = InvalidMemory1Package::doNotDereferenceNullPointersQuery()
+  }
+}

c/cert/src/rules/EXP33-C/DoNotReadUninitializedMemory.ql

@@ -0,0 +1,66 @@
+/**
+ * @id c/cert/do-not-access-freed-memory
+ * @name MEM30-C: Do not access freed memory
+ * @description Accessing memory that has been deallocated is undefined behavior.
+ * @kind problem
+ * @precision high
+ * @problem.severity error
+ * @tags external/cert/id/mem30-c
+ *       correctness
+ *       security
+ *       external/cert/obligation/rule
+ */
+
+import cpp
+import codingstandards.c.cert
+import codingstandards.cpp.Allocations
+import semmle.code.cpp.controlflow.StackVariableReachability
+
+/** `e` is an expression that frees the memory pointed to by `v`. */
+predicate isFreeExpr(Expr e, StackVariable v) {
+  exists(VariableAccess va | va.getTarget() = v and freeExprOrIndirect(e, va, _))
+}
+
+/** `e` is an expression that accesses `v` but is not the lvalue of an assignment. */
+predicate isAccessExpr(Expr e, StackVariable v) {
+  v.getAnAccess() = e and
+  not exists(Assignment a | a.getLValue() = e)
+  or
+  isDerefByCallExpr(_, _, e, v)
+}
+
+/**
+ * `va` is passed by value as (part of) the `i`th argument in
+ * call `c`. The target function is either a library function
+ * or a source code function that dereferences the relevant
+ * parameter.
+ */
+predicate isDerefByCallExpr(Call c, int i, VariableAccess va, StackVariable v) {
+  v.getAnAccess() = va and
+  va = c.getAnArgumentSubExpr(i) and
+  not c.passesByReference(i, va) and
+  (c.getTarget().hasEntryPoint() implies isAccessExpr(_, c.getTarget().getParameter(i)))
+}
+
+class UseAfterFreeReachability extends StackVariableReachability {
+  UseAfterFreeReachability() { this = "UseAfterFree" }
+
+  override predicate isSource(ControlFlowNode node, StackVariable v) { isFreeExpr(node, v) }
+
+  override predicate isSink(ControlFlowNode node, StackVariable v) { isAccessExpr(node, v) }
+
+  override predicate isBarrier(ControlFlowNode node, StackVariable v) {
+    definitionBarrier(v, node) or
+    isFreeExpr(node, v)
+  }
+}
+
+// This query is a modified version of the `UseAfterFree.ql`
+// (cpp/use-after-free) query from the CodeQL standard library.
+from UseAfterFreeReachability r, StackVariable v, Expr free, Expr e
+where
+  not isExcluded(e, InvalidMemory1Package::doNotAccessFreedMemoryQuery()) and
+  r.reaches(free, v, e)
+select e,
+  "Pointer '" + v.getName().toString() + "' accessed but may have been previously freed $@.", free,
+  "here"

c/cert/src/rules/EXP34-C/DoNotDereferenceNullPointers.md

@@ -0,0 +1 @@
+c/common/test/rules/informationleakageacrossboundaries/InformationLeakageAcrossBoundaries.ql

c/cert/src/rules/EXP34-C/DoNotDereferenceNullPointers.ql

@@ -0,0 +1 @@
+c/common/test/rules/readofuninitializedmemory/ReadOfUninitializedMemory.ql

c/cert/src/rules/MEM30-C/DoNotAccessFreedMemory.md

@@ -0,0 +1 @@
+c/common/test/rules/dereferenceofnullpointer/DereferenceOfNullPointer.ql

c/cert/src/rules/MEM30-C/DoNotAccessFreedMemory.ql

@@ -0,0 +1,5 @@
+| test.c:11:47:11:47 | p | Pointer 'p' accessed but may have been previously freed $@. | test.c:12:5:12:8 | call to free | here |
+| test.c:25:10:25:12 | buf | Pointer 'buf' accessed but may have been previously freed $@. | test.c:24:3:24:6 | call to free | here |
+| test.c:32:15:32:17 | buf | Pointer 'buf' accessed but may have been previously freed $@. | test.c:31:3:31:6 | call to free | here |
+| test.c:33:9:33:11 | buf | Pointer 'buf' accessed but may have been previously freed $@. | test.c:31:3:31:6 | call to free | here |
+| test.c:34:16:34:18 | buf | Pointer 'buf' accessed but may have been previously freed $@. | test.c:31:3:31:6 | call to free | here |

c/cert/test/rules/DCL39-C/InformationLeakageAcrossTrustBoundariesC.testref

@@ -0,0 +1 @@
+rules/MEM30-C/DoNotAccessFreedMemory.ql

c/cert/test/rules/EXP33-C/DoNotReadUninitializedMemory.testref

@@ -0,0 +1,38 @@
+#include <stdlib.h>
+#include <string.h>
+
+struct node {
+  struct node *next;
+};
+
+void test_freed_loop_var(struct node *list1, struct node *list2) {
+  struct node *tmp;
+
+  for (struct node *p = list1; p != NULL; p = p->next) { // NON_COMPLIANT
+    free(p);
+  }
+
+  for (struct node *p = list2; p != NULL; p = tmp) { // COMPLIANT
+    tmp = p->next;
+    free(p);
+  }
+}
+
+void test_freed_arg(char *input) {
+  char *buf = (char *)malloc(strlen(input) + 1);
+  strcpy(buf, input); // COMPLIANT
+  free(buf);
+  strcpy(buf, input); // NON_COMPLIANT
+}
+
+void test_freed_access_no_deref(char *input) {
+  char *buf = (char *)malloc(strlen(input) + 1);
+  strcpy(buf, input); // COMPLIANT
+  free(buf);
+  char *tmp = buf;  // NON_COMPLIANT
+  tmp = buf + 1;    // NON_COMPLIANT
+  char *tmp2 = buf; // NON_COMPLIANT
+  buf = NULL;       // COMPLIANT
+  (char *)buf;      // COMPLIANT
+  tmp2 = buf + 1;   // COMPLIANT
+}

c/cert/test/rules/EXP34-C/DoNotDereferenceNullPointers.testref

@@ -0,0 +1,2 @@
+| test.c:11:4:11:5 | l1 | Null may be dereferenced here because a null value was assigned $@. | test.c:4:21:4:21 | 0 | here |
+| test.c:18:6:18:7 | l1 | Null may be dereferenced here because a null value was assigned $@. | test.c:4:21:4:21 | 0 | here |

c/cert/test/rules/MEM30-C/DoNotAccessFreedMemory.expected

@@ -0,0 +1,2 @@
+// GENERATED FILE - DO NOT MODIFY
+import codingstandards.cpp.rules.dereferenceofnullpointer.DereferenceOfNullPointer

c/cert/test/rules/MEM30-C/DoNotAccessFreedMemory.qlref

@@ -0,0 +1,31 @@
+#include <stdlib.h>
+
+void test_null(int p1) {
+  int *l1 = (void *)0;
+
+  if (p1 > 10) {
+    // l1 is only conditionally initialized
+    l1 = malloc(10 * sizeof(int));
+  }
+
+  *l1; // NON_COMPLIANT - dereferenced and still null
+
+  if (l1) {
+    *l1; // COMPLIANT - null check before dereference
+  }
+
+  if (!l1) {
+    *l1; // NON_COMPLIANT - dereferenced and definitely null
+  } else {
+    *l1; // COMPLIANT - null check before dereference
+  }
+
+  free(l1); // COMPLIANT - free of `NULL` is not undefined behavior
+}
+
+void test_default_value_init() {
+  int *l1; // indeterminate and thus invalid but non-null state
+
+  *l1; // COMPLIANT - considered an uninitialized pointer,
+       // not a null pointer
+}

c/cert/test/rules/MEM30-C/test.c

@@ -0,0 +1,18 @@
+| arrays.c:11:20:11:21 | wa | 'wa' may leak information from {elements of a[...] (arrays.c:7)}. Path: wa (arrays.c:11) --> & ... (arrays.c:12) |
+| arrays.c:33:22:33:23 | wa | 'wa' may leak information from {elements of elements of a[...][...] (arrays.c:29)}. Path: wa (arrays.c:33) --> & ... (arrays.c:34) |
+| arrays.c:57:22:57:23 | wa | 'wa' may leak information from {WithPointer (arrays.c:52)}. Path: wa (arrays.c:57) --> & ... (arrays.c:59) |
+| interprocedural.c:37:9:37:9 | p | 'p' may leak information from {y (interprocedural.c:8)}. Path: p (interprocedural.c:37) --> past assign_x (interprocedural.c:32) --> & ... (interprocedural.c:39) |
+| interprocedural.c:104:9:104:9 | p | 'p' may leak information from {x (interprocedural.c:7), y (interprocedural.c:8)}. Path: p (interprocedural.c:104) --> overwrite_after_leak(...) (interprocedural.c:96) --> p (interprocedural.c:97) |
+| multilayer.c:16:10:16:10 | s | 's' may leak information from {b (multilayer.c:12)}. Path: s (multilayer.c:16) --> & ... (multilayer.c:18) |
+| multilayer.c:29:10:29:10 | s | 's' may leak information from {b (multilayer.c:12), x (multilayer.c:7)}. Path: s (multilayer.c:29) --> & ... (multilayer.c:30) |
+| multilayer.c:34:8:34:8 | s | 's' may leak information from {struct <unnamed> (multilayer.c:6)}. Path: s (multilayer.c:34) --> & ... (multilayer.c:35) |
+| test.c:12:12:12:12 | s | 's' may leak information from {y (test.c:8)}. Path: s (test.c:12) --> & ... (test.c:14) |
+| test.c:18:12:18:12 | s | 's' may leak information from {x (test.c:7)}. Path: s (test.c:18) --> & ... (test.c:20) |
+| test.c:24:12:24:12 | s | 's' may leak information from {x (test.c:7), y (test.c:8)}. Path: s (test.c:24) --> & ... (test.c:25) |
+| test.c:36:12:36:12 | s | 's' may leak information from {y (test.c:8)}. Path: s (test.c:36) --> & ... (test.c:38) |
+| test.c:43:12:43:12 | s | 's' may leak information from {x (test.c:7)}. Path: s (test.c:43) --> & ... (test.c:47) |
+| test.c:58:12:58:12 | s | 's' may leak information from {x (test.c:7), y (test.c:8)}. Path: s (test.c:58) --> & ... (test.c:59) |
+| test.c:64:12:64:12 | s | 's' may leak information from {y (test.c:8)}. Path: s (test.c:64) --> & ... (test.c:66) |
+| test.c:112:16:112:16 | s | 's' may leak information from {buf (test.c:92)}. Path: s (test.c:112) --> & ... (test.c:115) |
+| test.c:128:12:128:12 | s | 's' may leak information from {x (test.c:7), y (test.c:8)}. Path: s (test.c:128) --> & ... (test.c:132) |
+| test.c:157:22:157:22 | s | 's' may leak information from {2 to 2 bytes of padding in has_padding (test.c:151)}. Path: s (test.c:157) --> & ... (test.c:160) |

c/common/test/rules/dereferenceofnullpointer/DereferenceOfNullPointer.expected

@@ -0,0 +1,2 @@
+// GENERATED FILE - DO NOT MODIFY
+import codingstandards.cpp.rules.informationleakageacrossboundaries.InformationLeakageAcrossBoundaries

c/common/test/rules/dereferenceofnullpointer/DereferenceOfNullPointer.ql

@@ -0,0 +1,62 @@
+#include <stddef.h>
+#include <string.h>
+
+unsigned long copy_to_user(void *to, const void *from, unsigned long n);
+
+struct WithArray {
+  int a[2];
+};
+
+void forget_array() {
+  struct WithArray wa;
+  copy_to_user(0, &wa, sizeof wa); // NON_COMPLIANT
+}
+
+void write_partial_array() {
+  struct WithArray wa;
+  wa.a[0] = 1;
+  copy_to_user(0, &wa, sizeof wa); // NON_COMPLIANT[FALSE NEGATIVE]
+}
+
+void write_full_array() {
+  struct WithArray wa;
+  wa.a[0] = 1;
+  wa.a[1] = 1;
+  copy_to_user(0, &wa, sizeof wa); // COMPLIANT
+}
+
+struct WithArray2D {
+  int a[2][1];
+};
+
+void forget_array2d() {
+  struct WithArray2D wa;
+  copy_to_user(0, &wa, sizeof wa); // NON_COMPLIANT
+}
+
+void write_partial_array2d() {
+  struct WithArray2D wa;
+  wa.a[0][0] = 1;
+  copy_to_user(0, &wa, sizeof wa); // NON_COMPLIANT[FALSE NEGATIVE]
+}
+
+void write_full_array2d() {
+  struct WithArray2D wa;
+  wa.a[0][0] = 1;
+  wa.a[1][0] = 1;
+  copy_to_user(0, &wa, sizeof wa); // COMPLIANT
+}
+
+// A pointer field allows mostly the same syntactic operations as an array
+// field, but the semantics are completely different.
+struct WithPointer {
+  int *a;
+};
+
+void pointer_array_expression() {
+  struct WithPointer wa;
+  wa.a[0] = 1;
+  copy_to_user(0, &wa, sizeof wa); // NON_COMPLIANT
+}
+
+// TODO: test a struct in an array