SizeOfConstIntMacro qhelp, ql, examples and related qll.

Author: bdrodes

cpp/ql/src/Microsoft/Likely Bugs/SizeOfMisuse/SizeOfConstIntMacro.qhelp

@@ -0,0 +1,26 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+  <overview>
+    <p>If the argument for a <code>sizeof</code> call is a macro that expands to a constant integer type, it is a likely indication that the macro operation may be misused or that the argument was selected by mistake (i.e. typo).</p> 
+    <p>This query detects if the argument for <code>sizeof</code> is a macro that expands to a constant integer value.</p>
+    <p>NOTE: This rule will ignore multicharacter literal values that are exactly 4 bytes long as it matches the length of <code>int</code> and may be expected.</p>
+  </overview>
+
+<recommendation>
+  <p>Any findings from this rule may indicate that the <code>sizeof</code> is being used incorrectly.</p>
+  <p>Review the logic of the call.</p>
+</recommendation>
+
+<example>
+<p>The following example shows a case where <code>sizeof</code> a constant was used instead of the <code>sizeof</code> of a structure by mistake as the names are similar.</p>
+<sample src="SizeOfConstIntMacroBad.c" />
+
+<p>In this example, the fix is to replace the argument for <code>sizeof</code> with the structure name.</p>
+
+<sample src="SizeOfConstIntMacroGood.c" />
+
+</example>
+
+</qhelp>

cpp/ql/src/Microsoft/Likely Bugs/SizeOfMisuse/SizeOfConstIntMacro.ql

@@ -0,0 +1,54 @@
+/**
+ * @id cpp/sizeof/const-int-argument
+ * @name Passing a constant integer macro to sizeof
+ * @description The expression passed to sizeof is a macro that expands to an integer constant. A data type was likely intended instead.
+ * @kind problem
+ * @problem.severity error
+ * @precision high
+ * @tags security
+ */
+
+import cpp
+import SizeOfTypeUtils
+
+predicate isExprAConstInteger(Expr e, MacroInvocation mi) {
+  exists(Type type |
+    type = e.getExplicitlyConverted().getType() and
+    isTypeDangerousForSizeof(type) and
+    // Special case for wide-char literals when the compiler doesn't recognize wchar_t (i.e. L'\\', L'\0')
+    // Accounting for parenthesis "()" around the value
+    not exists(Macro m | m = mi.getMacro() |
+      m.getBody().toString().regexpMatch("^[\\s(]*L'.+'[\\s)]*$")
+    ) and
+    // Special case for token pasting operator
+    not exists(Macro m | m = mi.getMacro() | m.getBody().toString().regexpMatch("^.*\\s*##\\s*.*$")) and
+    // Special case for multichar literal integers that are exactly 4 character long (i.e. 'val1')
+    not exists(Macro m | m = mi.getMacro() |
+      e.getType().toString() = "int" and
+      m.getBody().toString().regexpMatch("^'.{4}'$")
+    ) and
+    e.isConstant()
+  )
+}
+
+int countMacros(Expr e) { result = count(MacroInvocation mi | mi.getExpr() = e | mi) }
+
+predicate isSizeOfExprOperandMacroInvocationAConstInteger(
+  SizeofExprOperator sizeofExpr, MacroInvocation mi
+) {
+  exists(Expr e |
+    e = mi.getExpr() and
+    e = sizeofExpr.getExprOperand() and
+    isExprAConstInteger(e, mi) and
+    // Special case for FPs that involve an inner macro that resolves to 0 such as _T('\0')
+    not exists(int macroCount | macroCount = countMacros(e) |
+      macroCount > 1 and e.(Literal).getValue().toInt() = 0
+    )
+  )
+}
+
+from SizeofExprOperator sizeofExpr, MacroInvocation mi
+where isSizeOfExprOperandMacroInvocationAConstInteger(sizeofExpr, mi)
+select sizeofExpr,
+  "$@: sizeof of integer macro $@ will always return the size of the underlying integer type.",
+  sizeofExpr, sizeofExpr.getEnclosingFunction().getName(), mi.getMacro(), mi.getMacro().getName()

cpp/ql/src/Microsoft/Likely Bugs/SizeOfMisuse/SizeOfConstIntMacroBad.c

@@ -0,0 +1,12 @@
+#define SOMESTRUCT_ERRNO_THAT_MATTERS 0x8000000d
+
+typedef struct {
+	int a;
+	bool b;
+} SOMESTRUCT_THAT_MATTERS;
+
+//bug, the code is using SOMESTRUCT_ERRNO_THAT_MATTERS by mistake instead of SOMESTRUCT_THAT_MATTERS
+if (somedata.length >= sizeof(SOMESTRUCT_ERRNO_THAT_MATTERS)) 
+{
+	/// Do something
+}

cpp/ql/src/Microsoft/Likely Bugs/SizeOfMisuse/SizeOfConstIntMacroGood.c

@@ -0,0 +1,11 @@
+#define SOMESTRUCT_ERRNO_THAT_MATTERS 0x8000000d
+
+typedef struct {
+	int a;
+	bool b;
+} SOMESTRUCT_THAT_MATTERS;
+
+if (somedata.length >= sizeof(SOMESTRUCT_THAT_MATTERS)) 
+{
+	/// Do something
+}

cpp/ql/src/Microsoft/Likely Bugs/SizeOfMisuse/SizeOfTypeUtils.qll

@@ -0,0 +1,45 @@
+import cpp
+
+/**
+ * Holds if `type` is a `Type` that typically should not be used for `sizeof` in macros or function return values.
+ */
+predicate isTypeDangerousForSizeof(Type type) {
+  (
+    type instanceof IntegralOrEnumType and
+    // ignore string literals
+    not type instanceof WideCharType and
+    not type instanceof CharType
+  )
+}
+
+/**
+ * Holds if `type` is a `Type` that typically should not be used for `sizeof` in macros or function return values.
+ * This predicate extends the types detected in exchange of precision.
+ * For higher precision, please use `isTypeDangerousForSizeof`
+ */
+predicate isTypeDangerousForSizeofLowPrecision(Type type) {
+  (
+    // UINT8/BYTE are typedefs to char, so we treat them separately.
+    // WCHAR is sometimes a typedef to UINT16, so we treat it separately too.
+    type.getName() = "UINT8"
+    or
+    type.getName() = "BYTE"
+    or
+    not type.getName() = "WCHAR" and
+    exists(Type ut |
+      ut = type.getUnderlyingType() and
+      ut instanceof IntegralOrEnumType and
+      not ut instanceof WideCharType and
+      not ut instanceof CharType
+    )
+  )
+}
+
+/**
+ * Holds if the `Function` return type is dangerous as input for `sizeof`.
+ */
+class FunctionWithTypeDangerousForSizeofLowPrecision extends Function {
+  FunctionWithTypeDangerousForSizeofLowPrecision() {
+    exists(Type type | type = this.getType() | isTypeDangerousForSizeofLowPrecision(type))
+  }
+}