codeql port of C28648. TODO needs tests

microsoft · Aug 16, 2024 · c282407 · c282407
1 parent 2034189
commit c282407
Show file tree

Hide file tree

Showing 4 changed files with 291 additions and 0 deletions.
diff --git a/src/drivers/general/queries/UnreliableFunction/UnreliableFunction.qhelp b/src/drivers/general/queries/UnreliableFunction/UnreliableFunction.qhelp
@@ -0,0 +1,47 @@
+<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
+<qhelp>
+	<overview>
+		<p>
+		PulseEvent is an unreliable function. A thread waiting on a synchronization object can be momentarily removed from the wait state by a kernel-mode APC, and then returned to the wait state after the APC is complete. If the call to PulseEvent occurs during the period when the thread was removed from the wait state, the thread will not be released and will "hang" forever. This is because PulseEvent releases only those threads that are waiting at the moment it is called.
+		</p>
+	</overview>
+	<recommendation>
+		<p>
+		If only one of the threads waiting on the event needs to be released AND the event is a manual-reset event, change it to an auto-reset event and call SetEvent instead of PulseEvent.
+
+		If only one of the threads waiting on the event needs to be released AND the event is an auto-reset event, call SetEvent instead of PulseEvent.
+
+		If all threads waiting on the event need to be released AND the event is a manual-reset event, redesign your code to use a different kind of synchronization object (such as a semaphore).
+
+		If all threads waiting on the event need to be released AND the event is an auto-reset event, call SetEvent instead of PulseEvent (your original call to PulseEvent was releasing only one thread anyway).
+		</p>
+	</recommendation>
+	<example>
+		<p>
+			TODO example
+		</p>
+		<sample language="c"> <![CDATA[
+		// Example code
+		}]]>
+		</sample>
+		<p>
+			TODO example 2
+		</p>
+		<sample language="c"> <![CDATA[
+				// Example code
+		}]]>
+		</sample>
+	</example>
+	<semmleNotes>
+		<p>
+			TODO notes
+		</p>
+	</semmleNotes>
+	<references>
+		<li>
+			<a href="example.com">
+				Example link
+			</a>
+		</li>
+	</references>
+</qhelp>
diff --git a/src/drivers/general/queries/UnreliableFunction/UnreliableFunction.ql b/src/drivers/general/queries/UnreliableFunction/UnreliableFunction.ql
@@ -0,0 +1,27 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+/**
+ * @id cpp/drivers/unreliable-function
+ * @kind problem
+ * @name Unreliable Function
+ * @description PulseEvent is an unreliable function
+ * @platform Desktop
+ * @feature.area Multiple
+ * @impact Insecure Coding Practice
+ * @repro.text
+ * @owner.email: [email protected]
+ * @opaqueid CQLD-C28648
+ * @problem.severity warning
+ * @precision high
+ * @tags correctness
+ * @scope domainspecific
+ * @query-version v1
+ */
+
+
+import cpp
+
+from FunctionCall f
+where
+  f.getTarget().getName().matches("PulseEvent") 
+select f, f.getTarget().toString() + " is an unreliable function and should not be used"
diff --git a/src/drivers/general/queries/UnreliableFunction/UnreliableFunction.sarif b/src/drivers/general/queries/UnreliableFunction/UnreliableFunction.sarif
@@ -0,0 +1,199 @@
+{
+  "$schema" : "https://json.schemastore.org/sarif-2.1.0.json",
+  "version" : "2.1.0",
+  "runs" : [ {
+    "tool" : {
+      "driver" : {
+        "name" : "CodeQL",
+        "organization" : "GitHub",
+        "semanticVersion" : "2.15.4",
+        "notifications" : [ {
+          "id" : "cpp/baseline/expected-extracted-files",
+          "name" : "cpp/baseline/expected-extracted-files",
+          "shortDescription" : {
+            "text" : "Expected extracted files"
+          },
+          "fullDescription" : {
+            "text" : "Files appearing in the source archive that are expected to be extracted."
+          },
+          "defaultConfiguration" : {
+            "enabled" : true
+          },
+          "properties" : {
+            "tags" : [ "expected-extracted-files", "telemetry" ]
+          }
+        } ],
+        "rules" : [ {
+          "id" : "cpp/drivers/TODO",
+          "name" : "cpp/drivers/TODO",
+          "shortDescription" : {
+            "text" : "TODO"
+          },
+          "fullDescription" : {
+            "text" : "TODO"
+          },
+          "defaultConfiguration" : {
+            "enabled" : true,
+            "level" : "warning"
+          },
+          "properties" : {
+            "tags" : [ "correctness" ],
+            "description" : "TODO",
+            "feature.area" : "Multiple",
+            "id" : "cpp/drivers/TODO",
+            "impact" : "Insecure Coding Practice",
+            "kind" : "problem",
+            "name" : "TODO",
+            "opaqueid" : "CQLD-TODO",
+            "owner.email:" : "[email protected]",
+            "platform" : "Desktop",
+            "precision" : "medium",
+            "problem.severity" : "warning",
+            "query-version" : "v1",
+            "repro.text" : "",
+            "scope" : "domainspecific"
+          }
+        } ]
+      },
+      "extensions" : [ {
+        "name" : "microsoft/windows-drivers",
+        "semanticVersion" : "1.0.13+4cf80ade609037becb8999823de45e08bd818a20",
+        "locations" : [ {
+          "uri" : "file:///C:/codeql-home/WDDST/src/",
+          "description" : {
+            "text" : "The QL pack root directory."
+          }
+        }, {
+          "uri" : "file:///C:/codeql-home/WDDST/src/qlpack.yml",
+          "description" : {
+            "text" : "The QL pack definition file."
+          }
+        } ]
+      } ]
+    },
+    "invocations" : [ {
+      "toolExecutionNotifications" : [ {
+        "locations" : [ {
+          "physicalLocation" : {
+            "artifactLocation" : {
+              "uri" : "driver/driver_snippet.c",
+              "uriBaseId" : "%SRCROOT%",
+              "index" : 1
+            }
+          }
+        } ],
+        "message" : {
+          "text" : ""
+        },
+        "level" : "none",
+        "descriptor" : {
+          "id" : "cpp/baseline/expected-extracted-files",
+          "index" : 0
+        },
+        "properties" : {
+          "formattedMessage" : {
+            "text" : ""
+          }
+        }
+      }, {
+        "locations" : [ {
+          "physicalLocation" : {
+            "artifactLocation" : {
+              "uri" : "driver/fail_driver1.c",
+              "uriBaseId" : "%SRCROOT%",
+              "index" : 0
+            }
+          }
+        } ],
+        "message" : {
+          "text" : ""
+        },
+        "level" : "none",
+        "descriptor" : {
+          "id" : "cpp/baseline/expected-extracted-files",
+          "index" : 0
+        },
+        "properties" : {
+          "formattedMessage" : {
+            "text" : ""
+          }
+        }
+      }, {
+        "locations" : [ {
+          "physicalLocation" : {
+            "artifactLocation" : {
+              "uri" : "driver/fail_driver1.h",
+              "uriBaseId" : "%SRCROOT%",
+              "index" : 2
+            }
+          }
+        } ],
+        "message" : {
+          "text" : ""
+        },
+        "level" : "none",
+        "descriptor" : {
+          "id" : "cpp/baseline/expected-extracted-files",
+          "index" : 0
+        },
+        "properties" : {
+          "formattedMessage" : {
+            "text" : ""
+          }
+        }
+      } ],
+      "executionSuccessful" : true
+    } ],
+    "artifacts" : [ {
+      "location" : {
+        "uri" : "driver/fail_driver1.c",
+        "uriBaseId" : "%SRCROOT%",
+        "index" : 0
+      }
+    }, {
+      "location" : {
+        "uri" : "driver/driver_snippet.c",
+        "uriBaseId" : "%SRCROOT%",
+        "index" : 1
+      }
+    }, {
+      "location" : {
+        "uri" : "driver/fail_driver1.h",
+        "uriBaseId" : "%SRCROOT%",
+        "index" : 2
+      }
+    } ],
+    "results" : [ {
+      "ruleId" : "cpp/drivers/TODO",
+      "ruleIndex" : 0,
+      "rule" : {
+        "id" : "cpp/drivers/TODO",
+        "index" : 0
+      },
+      "message" : {
+        "text" : "TODO"
+      },
+      "locations" : [ {
+        "physicalLocation" : {
+          "artifactLocation" : {
+            "uri" : "driver/fail_driver1.c",
+            "uriBaseId" : "%SRCROOT%",
+            "index" : 0
+          },
+          "region" : {
+            "startLine" : 56,
+            "endColumn" : 12
+          }
+        }
+      } ],
+      "partialFingerprints" : {
+        "primaryLocationLineHash" : "60c6386a5ee58eb6:1",
+        "primaryLocationStartColumnFingerprint" : "0"
+      }
+    } ],
+    "columnKind" : "utf16CodeUnits",
+    "properties" : {
+      "semmle.formatSpecifier" : "sarifv2.1.0"
+    }
+  } ]
+}
diff --git a/src/drivers/general/queries/UnreliableFunction/driver_snippet.c b/src/drivers/general/queries/UnreliableFunction/driver_snippet.c
@@ -0,0 +1,18 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+
+// Macros to enable or disable a code section that may or may not conflict with this test.
+#define SET_DISPATCH 1
+
+// Template function. Not used for this test.
+void top_level_call()
+{
+}
+
+// void test_bad()
+// {
+//     HANDLE hEvent = NULL;
+//     // hEvent would normally not be NULL
+
+//     PulseEvent(hEvent);
+// }