Adding IncorrectUsageOfRtlCompareMemory.qhelp, ql and example files.

Author: bdrodes

cpp/ql/src/Microsoft/Likely Bugs/Drivers/IncorrectUsageOfRtlCompareMemory.qhelp

@@ -0,0 +1,29 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+  <overview>
+    <p><code>RtlCompareMemory</code> routine compares two blocks of memory and returns the number of bytes that match, not a boolean value indicating a full comparison like <code>RtlEqualMemory</code> does.</p> 
+    <p>This query detects the return value of <code>RtlCompareMemory</code> being handled as a boolean.</p>
+  </overview>
+
+<recommendation>
+  <p>Any findings from this rule may indicate that the return value of a call to <code>RtlCompareMemory</code> is being incorrectly interpreted as a boolean.</p>
+  <p>Review the logic of the call, and if necessary, replace the function call with <code>RtlEqualMemory</code>.</p>
+</recommendation>
+
+<example>
+<p>The following example is a typical one where an identity comparison is intended, but the wrong API is being used.</p>
+<sample src="IncorrectUsageOfRtlCompareMemoryBad.c" />
+
+<p>In this example, the fix is to replace the call to <code>RtlCompareMemory</code> with <code>RtlEqualMemory</code>.</p>
+
+<sample src="IncorrectUsageOfRtlCompareMemoryGood.c" />
+
+</example>
+<references>
+    <li>Books online <a href="https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/wdm/nf-wdm-rtlcomparememory">RtlCompareMemory function (wdm.h)</a></li>
+    <li>Books online <a href="https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/wdm/nf-wdm-rtlequalmemory">RtlEqualMemory macro (wdm.h)</a></li>
+</references>
+
+</qhelp>

cpp/ql/src/Microsoft/Likely Bugs/Drivers/IncorrectUsageOfRtlCompareMemory.ql

@@ -0,0 +1,69 @@
+/**
+ * @id cpp/drivers/incorrect-usage-of-rtlcomparememory
+ * @name Incorrect usage of RtlCompareMemory
+ * @description `RtlCompareMemory` routine compares two blocks of memory and returns the number of bytes that match, not a boolean value indicating a full comparison like `RtlEqualMemory` does.
+ *     This query detects the return value of `RtlCompareMemory` being handled as a boolean.
+ * @security.severity Important
+ * @kind problem
+ * @problem.severity error
+ * @precision high
+ * @tags security
+ *       kernel
+ */
+
+import cpp
+
+predicate isLiteralABooleanMacro(Literal l) {
+  exists(MacroInvocation mi | mi.getExpr() = l |
+    mi.getMacroName() in ["true", "false", "TRUE", "FALSE"]
+  )
+}
+
+from FunctionCall fc, Function f, Expr e, string msg
+where
+  f.getQualifiedName() = "RtlCompareMemory" and
+  f.getACallToThisFunction() = fc and
+  (
+    exists(UnaryLogicalOperation ulo | e = ulo |
+      ulo.getAnOperand() = fc and
+      msg = "as an operand in an unary logical operation"
+    )
+    or
+    exists(BinaryLogicalOperation blo | e = blo |
+      blo.getAnOperand() = fc and
+      msg = "as an operand in a binary logical operation"
+    )
+    or
+    exists(Conversion conv | e = conv |
+      (
+        conv.getType().hasName("bool") or
+        conv.getType().hasName("BOOLEAN") or
+        conv.getType().hasName("_Bool")
+      ) and
+      conv.getUnconverted() = fc and
+      msg = "as a boolean"
+    )
+    or
+    exists(IfStmt s | e = s.getControllingExpr() |
+      s.getControllingExpr() = fc and
+      msg = "as the controlling expression in an If statement"
+    )
+    or
+    exists(EqualityOperation bao, Expr e2 | e = bao |
+      bao.hasOperands(fc, e2) and
+      isLiteralABooleanMacro(e2) and
+      msg =
+        "as an operand in an equality operation where the other operand is a boolean value (high precision result)"
+    )
+    or
+    exists(EqualityOperation bao, Expr e2 | e = bao |
+      bao.hasOperands(fc, e2) and
+      (e2.(Literal).getValue().toInt() = 1 or e2.(Literal).getValue().toInt() = 0) and
+      not isLiteralABooleanMacro(e2) and
+      msg =
+        "as an operand in an equality operation where the other operand is likely a boolean value (lower precision result, needs to be reviewed)"
+    )
+  )
+select e,
+  "This $@ is being handled $@ instead of the number of matching bytes. Please review the usage of this function and consider replacing it with `RtlEqualMemory`.",
+  fc, "call to `RtlCompareMemory`", e, msg

cpp/ql/src/Microsoft/Likely Bugs/Drivers/IncorrectUsageOfRtlCompareMemoryBad.c

@@ -0,0 +1,5 @@
+//bug, the code assumes RtlCompareMemory is comparing for identical values & return false if not identical
+if (!RtlCompareMemory(pBuffer, ptr, 16)) 
+{
+	return FALSE;
+}

cpp/ql/src/Microsoft/Likely Bugs/Drivers/IncorrectUsageOfRtlCompareMemoryGood.c

@@ -0,0 +1,5 @@
+//fixed
+if (!RtlEqualMemory(pBuffer, ptr, 16))
+{
+	return FALSE;
+}