LGTM: Query for dereferencing a pointer argument without NULL-comparison

.lgtm/cpp-queries/missing-argument-null-check.c

@@ -0,0 +1,29 @@
+#include <assert.h>
+
+typedef struct {
+    char *string;
+} StructWithString;
+
+void bad_deref(StructWithString *data)
+{
+    char *string = data->string;
+}
+
+void good_with_assert(StructWithString *data)
+{
+    assert(data != NULL);
+    char *string = data->string;
+}
+
+void good_with_no_deref(StructWithString *data)
+{
+    good_with_assert(data);
+}
+
+int main(void)
+{
+    StructWithString data = { 0 };
+    good_with_no_deref(&data);
+    bad_deref(&data);
+    return 0;
+}

.lgtm/cpp-queries/missing-argument-null-check.qhelp

@@ -0,0 +1,49 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+
+<p>
+Functions which dereference a pointer should test for <code>NULL</code>.
+This should be done as an explicit comparison in an <code>assert</code> or <code>if</code> statement.
+If the function assumes that an argument is non-null, add an assert at the beginning of the function body.
+</p>
+
+<p>
+There are some limitations in the current implementation.
+It only looks for explicit comparisons, <code>if (ptr == NULL)</code>, but this is the correct way to write it, according to our style guidelines.
+Only arrow syntax is detected, <code>ptr->field</code>, so it should be expanded to also detect asterisk syntax, <code>*ptr</code>.
+Also, it doesn't check that the comparison is before the dereference, it should be improved to also alert in cases where the check is after the dereference.
+There shouldn't be false positives, but it should be expanded to find more problematic cases.
+</p>
+
+</overview>
+<recommendation>
+
+<p>
+Add an assert to the beginning of the function if it assumes the argument is not null: <code>assert(ptr != NULL)</code>.
+Add an an explicit comparison somewhere in the function if it is okay for the argument to be NULL.
+(Usually this should be an <code>if</code> around the dereference).
+Note that in both cases, the comparison <b>must</b> be explicit (using <code>== NULL</code> or <code>!= NULL</code>).
+</p>
+
+</recommendation>
+<example>
+
+<p>
+This example has 2 correct (good) functions, and one incorrect (bad) function:
+</p>
+
+<sample src="missing-argument-null-check.c" />
+
+</example>
+<references>
+
+<li>
+CFEngine Contribution guidelines: <a href="https://github.com/cfengine/core/blob/master/CONTRIBUTING.md">CONTRIBUTING.md</a>
+</li>
+
+</references>
+</qhelp>

.lgtm/cpp-queries/missing-argument-null-check.ql

@@ -0,0 +1,30 @@
+/**
+ * @name Pointer argument is dereferenced without checking for NULL
+ * @description Functions which dereference a pointer should test for NULL.
+ *              This should be done as an explicit comparison in an assert or if statement.
+ * @kind problem
+ * @problem.severity recommendation
+ * @id cpp/missing-argument-null-check
+ * @tags readability
+ *       correctness
+ *       safety
+ * @precision very-high
+ */
+
+import cpp
+
+from Function func, PointerFieldAccess acc, Parameter p, PointerType pt
+where acc.getEnclosingFunction() = func
+  and p.getFunction() = func
+  and p.getType() = pt
+  and acc.getQualifier().toString() = p.getName()
+  and not
+  (
+    exists(EqualityOperation comparison |
+           comparison.getEnclosingFunction() = func
+           and comparison.getLeftOperand().toString() = p.getName()
+           and comparison.getRightOperand().findRootCause().toString() = "NULL")
+  )
+select acc, "Parameter " + p.getName() +
+            " in " + func.getName() +
+            "() is dereferenced without a null-check"