init DecOverflowWhenComparing query

Author: GrosQuildu

cpp/src/security/DecOverflowWhenComparing/DecOverflowWhenComparing.c

@@ -0,0 +1,62 @@
+#include <stdio.h>
+#include <stdint.h>
+#include <stdlib.h>
+#include <string.h>
+
+// from https://github.com/apple-oss-distributions/Libinfo/blob/9fce29e5c5edc15d3ecea55116ca17d3f6350603/lookup.subproj/mdns_module.c#L1033C1-L1079C2
+char* _mdns_parse_domain_name(const uint8_t *data, uint32_t datalen)
+{
+	int i = 0, j = 0;
+	uint32_t len;
+	uint32_t domainlen = 0;
+	char *domain = NULL;
+
+	if ((data == NULL) || (datalen == 0)) return NULL;
+
+	/*
+	 * i: index into input data
+	 * j: index into output string
+	 */
+	while (datalen-- > 0)
+	{
+		printf("%d\n", len);
+		len = data[i++];
+		domainlen += (len + 1);
+		domain = reallocf(domain, domainlen);
+
+		if (domain == NULL) return NULL;
+
+		if (len == 0) break;	// DNS root (NUL)
+
+		if (j > 0)
+		{
+			domain[j++] = datalen ? '.' : '\0';
+		}
+
+		while ((len-- > 0) && (0 != datalen--))
+		{
+			if (data[i] == '.')
+			{
+				/* special case: escape the '.' with a '\' */
+				domain = reallocf(domain, ++domainlen);
+				if (domain == NULL) return NULL;
+
+				domain[j++] = '\\';
+			}
+
+			domain[j++] = data[i++];
+		}
+	}
+
+	domain[j] = '\0';
+
+	return domain;
+}
+
+int main() {
+    const uint16_t datalen = 128;
+    uint8_t data[datalen] = {};
+    memcpy(data, "\x04quildu\x03xyz\x00", 11);
+    _mdns_parse_domain_name(data, datalen);
+}
+

cpp/src/security/DecOverflowWhenComparing/DecOverflowWhenComparing.qhelp

@@ -0,0 +1,34 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>
+Finds unsigned integer overflow issues with the following heuristic:
+* variable is compared to 0 and decremented
+* variable is used after the comparison and decrementation
+
+In such cases it is likely that the decrementation was not expected.
+</p>
+
+<p>
+You can read about a real-world vulnerability here: https://github.com/trailofbits/exploits/tree/main/obts-2025-macos-lpe
+</p>
+</overview>
+
+<recommendation>
+<p>
+Move the decrementation outside of comparison and/or add explicit checks for overflows.
+</p>
+</recommendation>
+
+<example>
+<sample src="DecOverflowWhenComparing.c" />
+
+<p>
+The <code>datalen</code> variable may overflow to UINT_MAX given a specific input.
+</p>
+</example>
+
+</qhelp>
+

cpp/src/security/DecOverflowWhenComparing/DecOverflowWhenComparing.ql

@@ -0,0 +1,57 @@
+/**
+ * @name Decrementation overflow when comparing
+ * @id tob/cpp/dec-overflow-when-comparing
+ * @description This query finds unsigned integer overflows resulting from unchecked decrementation during comparison.
+ * @kind problem
+ * @tags security
+ * @problem.severity error
+ * @precision high
+ * @security-severity 5.0
+ * @group security
+ */
+
+import cpp
+import semmle.code.cpp.ir.IR
+
+from Variable var, VariableAccess varAcc, DecrementOperation dec,
+  VariableAccess varAccAfterOverflow, ComparisonOperation cmp
+where
+  // get unsigned variable that is decremented
+  varAcc = var.getAnAccess() and
+  varAccAfterOverflow = var.getAnAccess() and
+  varAcc != varAccAfterOverflow and
+  dec.getOperand() = varAcc.getExplicitlyConverted() and
+  varAcc.getUnderlyingType().(IntegralType).isUnsigned() and
+
+  // only decrementations inside comparisons
+  cmp.getAnOperand().getAChild*() = varAcc and
+  cmp.getAnOperand() instanceof Zero and
+
+  // only if the variable is used after the comparison
+  cmp.getASuccessor+() = varAccAfterOverflow and
+  cmp.getAnOperand().getAChild*() != varAccAfterOverflow and
+
+  // skip if the variable is overwritten
+  // TODO: handle loops correctly
+  // not exists(VariableAccess varAccLV | varAccLV.isUsedAsLValue() |
+  //   varAccLV = var.getAnAccess() and
+  //   varAccLV != varAcc and
+  //   varAccLV != varAccAfterOverflow and
+  //   cmp.getASuccessor+() = varAccLV and
+  //   varAccAfterOverflow.getAPredecessor+() = varAccLV
+  // ) and
+
+  // var-- > 0 (0 < var--) then accesses only in false branch
+  // var-- >= 0 then accesses in all branches
+  // var-- == 0 then accesses in all branches
+  // var-- != 0 then accesses in all branches
+  if (
+    (cmp instanceof GTExpr and cmp.getRightOperand() instanceof Zero)
+    or
+    (cmp instanceof LTExpr and cmp.getLeftOperand() instanceof Zero)
+  )
+  then
+    cmp.getAFalseSuccessor().getASuccessor*() = varAccAfterOverflow
+  else
+    any()
+select cmp, varAccAfterOverflow

cpp/test/query-tests/security/DecOverflowWhenComparing/DecOverflowWhenComparing.c

@@ -0,0 +1,66 @@
+#ifndef USE_HEADERS
+#include "../../../include/libc/string_stubs.h"
+
+#else
+#include <stdio.h>
+#include <stdint.h>
+#include <stdlib.h>
+#include <string.h>
+#endif
+
+// from https://github.com/apple-oss-distributions/Libinfo/blob/9fce29e5c5edc15d3ecea55116ca17d3f6350603/lookup.subproj/mdns_module.c#L1033C1-L1079C2
+char* _mdns_parse_domain_name(const uint8_t *data, uint32_t datalen)
+{
+	int i = 0, j = 0;
+	uint32_t len;
+	uint32_t domainlen = 0;
+	char *domain = NULL;
+
+	if ((data == NULL) || (datalen == 0)) return NULL;
+
+	/*
+	 * i: index into input data
+	 * j: index into output string
+	 */
+	while (datalen-- > 0)
+	{
+		len = data[i++];
+		domainlen += (len + 1);
+		domain = reallocf(domain, domainlen);
+
+		if (domain == NULL) return NULL;
+
+		if (len == 0) break;	// DNS root (NUL)
+
+		if (j > 0)
+		{
+			domain[j++] = datalen ? '.' : '\0';
+		}
+
+		while ((len-- > 0) && (0 != datalen--))
+		{
+			if (data[i] == '.')
+			{
+				/* special case: escape the '.' with a '\' */
+				domain = reallocf(domain, ++domainlen);
+				if (domain == NULL) return NULL;
+
+				domain[j++] = '\\';
+			}
+
+			domain[j++] = data[i++];
+		}
+	}
+
+	domain[j] = '\0';
+
+	return domain;
+}
+
+int main() {
+    const uint16_t datalen = 128;
+    uint8_t data[datalen] = {};
+    memcpy(data, "\x04quildu\x03xyz\x00", 11);
+    _mdns_parse_domain_name(data, datalen);
+}
+

cpp/test/query-tests/security/DecOverflowWhenComparing/DecOverflowWhenComparing.expected

@@ -0,0 +1 @@
+security/DecOverflowWhenComparing/DecOverflowWhenComparing.ql