From 934fd33d928183d4bf32c2d255820867ee727c4a Mon Sep 17 00:00:00 2001
From: GrosQuildu <e2.8a.95@gmail.com>
Date: Tue, 2 Jul 2024 12:56:18 +0200
Subject: [PATCH 01/10] initial work for InconsistentReturnValueHandling

---
 .../InconsistentReturnValueHandling.qhelp     |  27 ++
 .../InconsistentReturnValueHandling.ql        | 167 +++++++++
 .../InconsistentReturnValueHandling.cpp       | 323 ++++++++++++++++++
 .../InconsistentReturnValueHandling.expected  |   1 +
 .../InconsistentReturnValueHandling.qlref     |   2 +
 5 files changed, 520 insertions(+)
 create mode 100644 cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.qhelp
 create mode 100644 cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.ql
 create mode 100644 cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.cpp
 create mode 100644 cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.expected
 create mode 100644 cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.qlref

diff --git a/cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.qhelp b/cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.qhelp
new file mode 100644
index 0000000..fc60a6c
--- /dev/null
+++ b/cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.qhelp
@@ -0,0 +1,27 @@
+<!DOCTYPE qhelp PUBLIC
+"-//Semmle//qhelp//EN"
+"qhelp.dtd">
+<qhelp>
+<overview>
+<p>
+    Description of the vulnerability.
+</p>
+</overview>
+<recommendation>
+<p>
+    Do XYZ.
+</p>
+</recommendation>
+<example>
+<p>
+    The following example shows ...:
+</p>
+<sample src="SomeFileInCWD.cpp"/>
+</example>
+<references>
+<li>
+    XYZ:
+    <a href="https://trailofbits.com">XYZ</a>
+</li>
+</references>
+</qhelp>
diff --git a/cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.ql b/cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.ql
new file mode 100644
index 0000000..48324d8
--- /dev/null
+++ b/cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.ql
@@ -0,0 +1,167 @@
+/**
+ * @name Inconsistent handling of return values from a specific function
+ * @description If a function returns a value that is used in `if` statements,
+ *  and in some statements the value is compared differently than in some other statements, this implies a possible bugs.
+ * @kind problem
+ * @problem.severity warning
+ * @precision medium
+ * @id tob/cpp/inconsistent-retval-handling
+ * @tags security
+ */
+
+ /**
+ * Possible inconsistencies
+ * - small % of values is compared to a different type (int, bool, nullptr, retval of a function, ...)
+ */
+
+
+import cpp
+import semmle.code.cpp.dataflow.new.DataFlow
+import semmle.code.cpp.controlflow.IRGuards
+
+newtype TCmpClass =
+  Tint()
+  or
+  Tnull()
+  or
+  Tptr()
+  or
+  Tsizeof()
+  or
+  Tcall()
+  or
+  Targ()
+
+class CmpClass extends TCmpClass {
+  string toString() {
+    this = Tint() and result = "int"
+    or
+    this = Tnull() and result = "null"
+    or
+    this = Tptr() and result = "ptr"
+    or
+    this = Tsizeof() and result = "sizeof"
+    or
+    this = Tcall() and result = "call"
+    or
+    this = Targ() and result = "arg"
+  }
+}
+
+Expr getOtherOperand(ComparisonOperation cmp, Expr fcRetVal) {
+    if cmp.getRightOperand() = fcRetVal then
+        result = cmp.getLeftOperand()
+    else
+        result = cmp.getRightOperand()
+}
+
+// TODO: why codeql does not have IntegralLiteral?
+predicate numericLiteral(Expr expr) {
+    expr instanceof Literal
+    and not (
+        expr instanceof BlockExpr
+        or
+        expr instanceof FormatLiteral	
+        or
+        expr instanceof NULL
+        or
+        expr instanceof TextLiteral
+        or
+        expr instanceof LabelLiteral
+        or
+        expr.getType() instanceof NullPointerType
+    )
+}
+
+int countRetValType(Function f, TCmpClass comparedValCategory, FunctionCall fc) {
+    result = count(Expr fcRetVal, IfStmt ifs |
+        fc.getTarget() = f
+        and DataFlow::localFlow(
+            DataFlow::exprNode(fc),
+            DataFlow::exprNode(fcRetVal)
+        )
+        and fcRetVal = ifs.getCondition().getAChild*()
+        and (
+            (
+                // if (retVal != comparedVal)
+                exists(ComparisonOperation cmp, Expr comparedVal |
+                    ifs.getCondition() = cmp
+                    and comparedVal = getOtherOperand(cmp, fcRetVal)
+                    and comparedValCategory = operandCategory(comparedVal)
+                )
+            )
+            or
+            (
+                // if(func(retVal) != anything)
+                exists(FunctionCall anyFc |
+                    ifs.getCondition().getAChild*() = anyFc
+                    and anyFc.getAnArgument() = fcRetVal
+                    and comparedValCategory = Targ()
+                )
+            )
+        )
+        |
+        comparedValCategory
+    )
+}
+
+TCmpClass operandCategory(Expr comparedVal) {
+    (numericLiteral(comparedVal) and result = Tint())
+    or
+    ((comparedVal.getType() instanceof NullPointerType or comparedVal instanceof NULL) and result = Tnull())
+    or
+    (comparedVal.getUnderlyingType() instanceof DerivedType and result = Tptr())
+}
+
+from Function f, int categoryCount,
+    TCmpClass mostCommonCategory, CmpClass mostCommonCategoryClass, int categoryMax
+    // TCmpClass buggyCategory, CmpClass buggyCategoryClass, FunctionCall buggyFc
+where
+    // we are interested only in defined and used functions
+    exists(FunctionCall fc | fc.getTarget() = f)
+    and f.hasDefinition()
+
+    // the function's retVal must be used in some IF statements
+    and categoryCount = countRetValType(f, _, _)
+    and categoryCount > 2
+
+    // now we find how the function's retVal is used most commonly
+    and mostCommonCategory = max(| categoryMax = countRetValType(f, mostCommonCategory, _) | mostCommonCategory order by categoryMax)
+    and mostCommonCategoryClass = mostCommonCategory
+
+    // if threshold for "most common" use case is 80%, then remaining 20% function calls are handled incorrectly
+    // and ((float)(categoryMax * 100) / categoryCount) >= 80
+
+    // // and finally we are looking for calls that use retVal in an uncommon way
+    // and countRetValType(f, buggyCategory, buggyFc) != 0
+    // and buggyCategory != mostCommonCategory
+    // and buggyCategoryClass = buggyCategory 
+    
+// select buggyFc, "Function $@ is usually compared with " + mostCommonCategoryClass + "(" + categoryMax +
+//     " times), but this call compares with " + buggyCategoryClass, f, f.getName()
+
+select f, mostCommonCategoryClass, categoryMax
+
+
+
+// class RetValCheck {
+
+// }
+
+// predicate typicalRetValComparison(Function f) {
+
+// }
+
+// from Function f, FunctionCall fc, ControlFlowNode test, GuardCondition guard
+// where 
+//   fc.getTarget() = f
+//   and guard.controls(fc.getBasicBlock(), _)
+//   and test = guard.getTest()
+//   and fc = rank[1 .. 5](FunctionCall a, string t, int c |
+//     a.getTarget() = f
+//     and any(RelationalOperation ro | ro = test.getElement() |
+//       ro.getGreaterOperand() = a or ro.getLesserOperand() = a
+//     ).getFullyConverted().getType().getName() = t
+//     order by c = count(fc)
+//     ).first()
+// select f, fc, "Inconsistent return value handling found in function calls to \"" + f.getQualifiedName() + "\". The comparison type used in \"" + fc.toString() + "\" is not the most common comparison type."
\ No newline at end of file
diff --git a/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.cpp b/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.cpp
new file mode 100644
index 0000000..5c58a15
--- /dev/null
+++ b/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.cpp
@@ -0,0 +1,323 @@
+#include <stdio.h>
+
+struct something {
+    int x;
+    char w;
+};
+
+struct athing {
+    int x;
+    char w;
+    int y;
+};
+
+bool cmp_func(int a)
+{
+    return a >= 10 ? true : false;
+}
+bool cmp_func2(int a)
+{
+    return a >= 102 ? true : false;
+}
+
+int main() {
+    return 0;
+}
+
+// BAD 1
+int target_func_1(int a)
+{
+    return a + 1;
+}
+
+void test_1_1() {
+    // BAD: comparing with sizeof instead of hard-coded int
+    if (target_func_1(2) != sizeof(something)) {
+        puts("something");
+    }
+}
+
+void test_1_2() {
+    if (target_func_1(2) != 1) {
+        puts("something2");
+    }
+    if (target_func_1(2) > 0) {
+        puts("something2");
+    }
+    int r = target_func_1(3);
+    if (r > 0) {
+        puts("something2");
+    }
+}
+
+
+
+// BAD 2
+int target_func_2(int a)
+{
+    return a - 2;
+}
+
+void test_2_1() {
+    if (target_func_2(2) != sizeof(something)) {
+        puts("something");
+    }
+}
+
+void test_2_2() {
+    if (target_func_2(-123) != sizeof(something)) {
+        puts("something");
+    }
+    int r = target_func_2(123);
+    if (r > sizeof(something)) {
+        return;
+    }
+
+    r = target_func_2(3);
+    // BAD: comparing with hard-coded int instead of sizeof
+    if (r > 0) {
+        puts("something2");
+    }
+}
+
+// BAD 3
+int target_func_3(int a)
+{
+    return a*a;
+}
+
+void test_3_1() {
+    if (target_func_3(2) != sizeof(something)) {
+        puts("something");
+    }
+}
+
+void test_3_2() {
+    if (target_func_3(-123) != sizeof(something)) {
+        puts("something");
+    }
+    int r = target_func_3(123);
+    if (r > sizeof(something)) {
+        return;
+    }
+
+    r = target_func_3(-3);
+    // BAD: comparing with a different sizeof
+    if (r > sizeof(athing)) {
+        puts("something2");
+    }
+}
+
+// BAD 4
+class SomeClass
+{
+private:
+    bool r;
+    int r2;
+public:
+    SomeClass();
+    void DoSomething(int);
+    void DoSomethingElse();
+    int target_func_4(int);
+};
+
+void test_4_1() {
+    SomeClass sc = SomeClass();
+    if (sc.target_func_4(2) != 1) {
+        puts("something");
+    }
+}
+
+void test_4_2() {
+    SomeClass sc = SomeClass();
+    if (sc.target_func_4(-123) != -1) {
+        puts("something");
+    }
+    int r = sc.target_func_4(123);
+    if (r > 0) {
+        return;
+    }
+
+    r = sc.target_func_4(-3);
+    if (r > 123) {
+        puts("something2");
+    }
+}
+
+SomeClass::SomeClass() {
+    r = true;
+    r2 = 33;
+}
+
+void SomeClass::DoSomething(int a) {
+    if (target_func_4(-123) != -1) {
+        puts("something");
+    }
+    int r = target_func_4(123);
+    if (r > 0) {
+        return;
+    }
+
+    r = target_func_4(-3);
+    if (r > 123) {
+        puts("something2");
+    }
+}
+
+void SomeClass::DoSomethingElse() {
+    if (target_func_4(-123) != -1) {
+        puts("something");
+    }
+    int r = target_func_4(123);
+    // BAD: bool instead of int comparison
+    if (r != true) {
+        return;
+    }
+}
+
+int SomeClass::target_func_4(int a) {
+    return a + 2;
+}
+
+// BAD 5
+static int w = 2;
+int* target_func_5(int a)
+{
+    return &w;
+}
+
+void test_5_1() {
+    if (target_func_5(2) != nullptr) {
+        puts("something");
+    }
+}
+
+void test_5_2() {
+    if (target_func_5(-123) != nullptr) {
+        puts("something");
+    }
+    int *r = target_func_5(123);
+    if (r == NULL) {
+        return;
+    }
+
+    r = target_func_5(-3);
+    int *some_ptr = &w;
+    // BAD: comparing with a pointer instead of NULL
+    if (r > some_ptr) {
+        puts("something2");
+    }
+}
+
+// BAD 6
+bool target_func_6(int a)
+{
+    return a > 10 ? true : false;
+}
+
+void test_6_1() {
+    if (target_func_6(2) != cmp_func(2)) {
+        puts("something");
+    }
+}
+
+void test_6_2() {
+    if (target_func_6(-123) != cmp_func2(-123)) {
+        puts("something");
+    }
+    bool r = target_func_6(123);
+    if (r == cmp_func(3333)) {
+        return;
+    }
+
+    r = target_func_6(-3);
+    // BAD: comparing with sizeof instead of some other function
+    if (r != (bool)sizeof(something)) {
+        puts("something2");
+    }
+}
+
+// BAD 7
+bool target_func_7(int a)
+{
+    return a > 10 ? true : false;
+}
+bool some_func_cmp(bool x) {
+    return !x;
+}
+
+void test_6_1() {
+    if (some_func_cmp(target_func_7(2))) {
+        puts("something");
+    }
+}
+
+void test_6_2() {
+    if (some_func_cmp(target_func_7(12)) != cmp_func2(-123)) {
+        puts("something");
+    }
+    bool r = target_func_7(123);
+    if (some_func_cmp(r) < 7) {
+        return;
+    }
+
+    r = target_func_7(-3);
+    // BAD: comparing with something instead of using as argument to a function
+    if (r != cmp_func2(-123)) {
+        puts("something2");
+    }
+}
+
+// GOOD 1: always comparing with NULL
+int* target_func_g1(int a)
+{
+    return &w;
+}
+
+void test_g_1_1() {
+    if (target_func_g1(2) != nullptr) {
+        puts("something");
+    }
+}
+
+void test_g_1_2() {
+    if (target_func_g1(-123) != nullptr) {
+        puts("something");
+    }
+    int *r = target_func_g1(123);
+    if (r == NULL) {
+        return;
+    }
+
+    r = target_func_g1(-3);
+    if (r == nullptr) {
+        puts("something2");
+    }
+}
+
+// GOOD 2: always comparing with some function
+bool target_func_g2(int a)
+{
+    return a > 10 ? true : false;
+}
+
+void test_g_2_1() {
+    if (target_func_g2(2) != cmp_func(2)) {
+        puts("something");
+    }
+}
+
+void test_g_2_2() {
+    if (target_func_g2(-123) != cmp_func(-123)) {
+        puts("something");
+    }
+    bool r = target_func_g2(123);
+    if (r == cmp_func(3333)) {
+        return;
+    }
+
+    r = target_func_g2(-3);
+    if (r > cmp_func2(-3)) {
+        puts("something2");
+    }
+}
\ No newline at end of file
diff --git a/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.expected b/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.expected
new file mode 100644
index 0000000..0ae1442
--- /dev/null
+++ b/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.expected
@@ -0,0 +1 @@
+| new query passed |
diff --git a/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.qlref b/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.qlref
new file mode 100644
index 0000000..112388d
--- /dev/null
+++ b/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.qlref
@@ -0,0 +1,2 @@
+InconsistentReturnValueHandling/InconsistentReturnValueHandling.ql
+

From de7298c98be14c1fbdc9a6dfecc481829e2e47ff Mon Sep 17 00:00:00 2001
From: GrosQuildu <e2.8a.95@gmail.com>
Date: Mon, 8 Jul 2024 15:25:09 +0200
Subject: [PATCH 02/10] InconsistentReturnValueHandling v1 working

---
 .../InconsistentReturnValueHandling.ql        | 260 +++++++++++-------
 .../InconsistentReturnValueHandling.cpp       |  66 ++++-
 .../InconsistentReturnValueHandling.expected  |   9 +-
 .../InconsistentReturnValueHandling.qlref     |   2 +-
 4 files changed, 222 insertions(+), 115 deletions(-)

diff --git a/cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.ql b/cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.ql
index 48324d8..55aea8c 100644
--- a/cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.ql
+++ b/cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.ql
@@ -1,7 +1,10 @@
 /**
  * @name Inconsistent handling of return values from a specific function
- * @description If a function returns a value that is used in `if` statements,
- *  and in some statements the value is compared differently than in some other statements, this implies a possible bugs.
+ * @description If a function's return value is used in `if` statements,
+ *              and in a few statements the value is compared somehow differently than it is usually,
+ *              then this rare comparisons may indicate bugs.
+ *              The query categorizes uses of return values into a few categories
+ *              (cmp with int, bool, nullptr, sizeof, another function, ...)
  * @kind problem
  * @problem.severity warning
  * @precision medium
@@ -9,12 +12,6 @@
  * @tags security
  */
 
- /**
- * Possible inconsistencies
- * - small % of values is compared to a different type (int, bool, nullptr, retval of a function, ...)
- */
-
-
 import cpp
 import semmle.code.cpp.dataflow.new.DataFlow
 import semmle.code.cpp.controlflow.IRGuards
@@ -22,146 +19,201 @@ import semmle.code.cpp.controlflow.IRGuards
 newtype TCmpClass =
   Tint()
   or
+  Tbool()
+  or
   Tnull()
   or
   Tptr()
   or
-  Tsizeof()
+  Tsizeof(Type s)
   or
   Tcall()
   or
   Targ()
+  or
+  Tarithm()
 
 class CmpClass extends TCmpClass {
   string toString() {
-    this = Tint() and result = "int"
+    this = Tint() and result = " numeric literal"
     or
-    this = Tnull() and result = "null"
+    this = Tbool() and result = " bool"
     or
-    this = Tptr() and result = "ptr"
+    this = Tnull() and result = " null"
     or
-    this = Tsizeof() and result = "sizeof"
+    this = Tptr() and result = " pointer"
     or
-    this = Tcall() and result = "call"
+    exists(Type s | this = Tsizeof(s) and result = " sizeof(" + s + ")")
     or
-    this = Targ() and result = "arg"
+    this = Tcall() and result = " some function's return value"
+    or
+    this = Tarithm() and result = " arithmetic expression"
+    or
+    this = Targ() and result = "in call to some function"
   }
 }
 
-Expr getOtherOperand(ComparisonOperation cmp, Expr fcRetVal) {
-    if cmp.getRightOperand() = fcRetVal then
-        result = cmp.getLeftOperand()
-    else
-        result = cmp.getRightOperand()
-}
-
 // TODO: why codeql does not have IntegralLiteral?
 predicate numericLiteral(Expr expr) {
-    expr instanceof Literal
-    and not (
-        expr instanceof BlockExpr
-        or
-        expr instanceof FormatLiteral	
-        or
-        expr instanceof NULL
-        or
-        expr instanceof TextLiteral
-        or
-        expr instanceof LabelLiteral
+    (
+        expr instanceof Literal
+        and not (
+            expr instanceof BlockExpr
+            or
+            expr instanceof FormatLiteral	
+            or
+            expr instanceof NULL
+            or
+            expr instanceof TextLiteral
+            or
+            expr instanceof LabelLiteral
+            or
+            expr.getType() instanceof NullPointerType
+        )
+    )
+    or
+    (
+        expr instanceof UnaryArithmeticOperation
+        and
+        numericLiteral(expr.getAChild())
+    )
+}
+
+predicate binaryComputation(Expr e) {
+    e instanceof BinaryArithmeticOperation
+    or
+    e instanceof BinaryBitwiseOperation
+    or
+    e instanceof BinaryLogicalOperation
+}
+
+/**
+ * Categorize expressions using literals instead of types, because
+ * we want to differentiate between functions calls and hard-coded stuff 
+ */
+TCmpClass operandCategory(Expr comparedVal) {
+    (numericLiteral(comparedVal) and not comparedVal.getType() instanceof BoolType and result = Tint())
+    or
+    (numericLiteral(comparedVal) and comparedVal.getType() instanceof BoolType and result = Tbool())
+    or
+    ((comparedVal.getType() instanceof NullPointerType or comparedVal instanceof NULL) and result = Tnull())
+    or
+    (comparedVal.getUnderlyingType() instanceof DerivedType and result = Tptr())
+    or
+    (comparedVal instanceof FunctionCall and result = Tcall())
+    or
+    (comparedVal instanceof SizeofOperator and
+        (
+        result = Tsizeof(comparedVal.(SizeofExprOperator).getExprOperand().getType())
         or
-        expr.getType() instanceof NullPointerType
+        result = Tsizeof(comparedVal.(SizeofTypeOperator).getTypeOperand())
+        )
     )
+    or
+    (binaryComputation(comparedVal) and result = Tarithm())
+}
+
+module RetValFlowConfig implements DataFlow::ConfigSig {
+    predicate isSource(DataFlow::Node source) {
+        source = source
+    }
+
+    predicate isSink(DataFlow::Node sink) {
+        sink = sink
+    }
 }
+module RetValFlow = DataFlow::Global<RetValFlowConfig>;
 
-int countRetValType(Function f, TCmpClass comparedValCategory, FunctionCall fc) {
-    result = count(Expr fcRetVal, IfStmt ifs |
+Expr getOtherOperand(ComparisonOperation cmp, Expr fcRetVal) {
+    if cmp.getRightOperand().getAChild*() = fcRetVal then
+        result = cmp.getLeftOperand()
+    else
+        result = cmp.getRightOperand()
+}
+
+predicate categorize(Function f, FunctionCall fc, TCmpClass comparedValCategory, IfStmt ifs) {
+    exists(Expr fcRetVal |
         fc.getTarget() = f
-        and DataFlow::localFlow(
+        and RetValFlow::flow(
             DataFlow::exprNode(fc),
             DataFlow::exprNode(fcRetVal)
         )
-        and fcRetVal = ifs.getCondition().getAChild*()
-        and (
-            (
-                // if (retVal != comparedVal)
-                exists(ComparisonOperation cmp, Expr comparedVal |
-                    ifs.getCondition() = cmp
-                    and comparedVal = getOtherOperand(cmp, fcRetVal)
-                    and comparedValCategory = operandCategory(comparedVal)
-                )
+        and ifs.getCondition().getAChild*() = fcRetVal
+        and
+        if
+            // if(func(retVal) == anything)
+            exists(FunctionCall anyFc |
+                ifs.getCondition().getAChild*() = anyFc
+                and anyFc.getAnArgument() = fcRetVal
             )
-            or
-            (
-                // if(func(retVal) != anything)
-                exists(FunctionCall anyFc |
-                    ifs.getCondition().getAChild*() = anyFc
-                    and anyFc.getAnArgument() = fcRetVal
-                    and comparedValCategory = Targ()
-                )
+        then
+            comparedValCategory = Targ()
+        else
+            // if (retVal != comparedVal)
+            exists(ComparisonOperation cmp, Expr comparedVal |
+                ifs.getCondition().getAChild*() = cmp
+                and
+                    // if (2*retVal+1 != comparedVal)
+                    // if (retVal > 2*anything()+sizeof(struct))
+                    if (
+                        cmp.getAnOperand().getAChild*() = fcRetVal
+                        and binaryComputation(cmp.getAnOperand())
+                    )
+                    then
+                        comparedValCategory = Tarithm()
+                    else
+                        // if (retVal != comparedVal)
+                        comparedVal = getOtherOperand(cmp, fcRetVal)
+                        and comparedValCategory = operandCategory(comparedVal)
             )
-        )
-        |
-        comparedValCategory
     )
 }
 
-TCmpClass operandCategory(Expr comparedVal) {
-    (numericLiteral(comparedVal) and result = Tint())
-    or
-    ((comparedVal.getType() instanceof NullPointerType or comparedVal instanceof NULL) and result = Tnull())
-    or
-    (comparedVal.getUnderlyingType() instanceof DerivedType and result = Tptr())
+int countAllRetValTypes(Function f) {
+    result = count(IfStmt ifs |
+        categorize(f, _, _, ifs) |
+        ifs
+    )
 }
 
-from Function f, int categoryCount,
-    TCmpClass mostCommonCategory, CmpClass mostCommonCategoryClass, int categoryMax
-    // TCmpClass buggyCategory, CmpClass buggyCategoryClass, FunctionCall buggyFc
+int mostCommonRetValType(Function f, TCmpClass mostCommonCategory) {
+    result = max(int numberOfRetValTypeInstances |
+        categorize(f, _, mostCommonCategory, _)
+        and numberOfRetValTypeInstances = count(IfStmt ifs | categorize(f, _, mostCommonCategory, ifs) | ifs)
+    )
+}
+
+// uncomment for testing:
+// from Function f, FunctionCall fc, TCmpClass comparedValCategory, CmpClass x, IfStmt ifs
+// where
+//     categorize(f, fc, comparedValCategory, ifs)
+//     and x = comparedValCategory
+// select f, fc, x, ifs
+
+
+from Function f, int retValsTotalAmount,
+    TCmpClass mostCommonCategory, CmpClass mostCommonCategoryClass, int categoryMax,
+    TCmpClass buggyCategory, CmpClass buggyCategoryClass, FunctionCall buggyFc
 where
-    // we are interested only in defined and used functions
+    // we are interested only in used functions
     exists(FunctionCall fc | fc.getTarget() = f)
-    and f.hasDefinition()
+    // and f.hasDefinition()
 
     // the function's retVal must be used in some IF statements
-    and categoryCount = countRetValType(f, _, _)
-    and categoryCount > 2
+    and retValsTotalAmount = countAllRetValTypes(f)
+    and retValsTotalAmount >= 4
 
     // now we find how the function's retVal is used most commonly
-    and mostCommonCategory = max(| categoryMax = countRetValType(f, mostCommonCategory, _) | mostCommonCategory order by categoryMax)
+    and categoryMax = mostCommonRetValType(f, mostCommonCategory)
     and mostCommonCategoryClass = mostCommonCategory
 
-    // if threshold for "most common" use case is 80%, then remaining 20% function calls are handled incorrectly
-    // and ((float)(categoryMax * 100) / categoryCount) >= 80
+    // if threshold for "most common" use case is ~75%, then remaining 25% function calls are handled somehow incorrectly
+    and ((float)(categoryMax * 100) / retValsTotalAmount) >= 74
 
     // // and finally we are looking for calls that use retVal in an uncommon way
-    // and countRetValType(f, buggyCategory, buggyFc) != 0
-    // and buggyCategory != mostCommonCategory
-    // and buggyCategoryClass = buggyCategory 
+    and categorize(f, buggyFc, buggyCategory, _)
+    and buggyCategory != mostCommonCategory
+    and buggyCategoryClass = buggyCategory 
     
-// select buggyFc, "Function $@ is usually compared with " + mostCommonCategoryClass + "(" + categoryMax +
-//     " times), but this call compares with " + buggyCategoryClass, f, f.getName()
-
-select f, mostCommonCategoryClass, categoryMax
-
-
-
-// class RetValCheck {
-
-// }
-
-// predicate typicalRetValComparison(Function f) {
-
-// }
-
-// from Function f, FunctionCall fc, ControlFlowNode test, GuardCondition guard
-// where 
-//   fc.getTarget() = f
-//   and guard.controls(fc.getBasicBlock(), _)
-//   and test = guard.getTest()
-//   and fc = rank[1 .. 5](FunctionCall a, string t, int c |
-//     a.getTarget() = f
-//     and any(RelationalOperation ro | ro = test.getElement() |
-//       ro.getGreaterOperand() = a or ro.getLesserOperand() = a
-//     ).getFullyConverted().getType().getName() = t
-//     order by c = count(fc)
-//     ).first()
-// select f, fc, "Inconsistent return value handling found in function calls to \"" + f.getQualifiedName() + "\". The comparison type used in \"" + fc.toString() + "\" is not the most common comparison type."
\ No newline at end of file
+select buggyFc, "Function $@ return value is usually compared with" + mostCommonCategoryClass + " (" + categoryMax +
+    " times), but this call compares with" + buggyCategoryClass, f, f.getName()
diff --git a/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.cpp b/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.cpp
index 5c58a15..123f3f0 100644
--- a/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.cpp
+++ b/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.cpp
@@ -1,4 +1,4 @@
-#include <stdio.h>
+#include "../../../include/libc/string_stubs.h"
 
 struct something {
     int x;
@@ -59,17 +59,17 @@ int target_func_2(int a)
 }
 
 void test_2_1() {
-    if (target_func_2(2) != sizeof(something)) {
+    if (target_func_2(2) != sizeof(struct something)) {
         puts("something");
     }
 }
 
 void test_2_2() {
-    if (target_func_2(-123) != sizeof(something)) {
+    if (target_func_2(-123) != sizeof(struct something)) {
         puts("something");
     }
     int r = target_func_2(123);
-    if (r > sizeof(something)) {
+    if (r > sizeof(struct something)) {
         return;
     }
 
@@ -87,23 +87,40 @@ int target_func_3(int a)
 }
 
 void test_3_1() {
-    if (target_func_3(2) != sizeof(something)) {
+    if (target_func_3(2) != sizeof(struct something)) {
         puts("something");
     }
 }
 
 void test_3_2() {
+    auto athingInstance = athing{};
+    auto somethingInstance = something{};
+
     if (target_func_3(-123) != sizeof(something)) {
         puts("something");
     }
     int r = target_func_3(123);
-    if (r > sizeof(something)) {
+    if (r > sizeof(struct something)) {
+        return;
+    }
+    if (r > sizeof(somethingInstance)) {
+        return;
+    }
+    if (r >= sizeof(somethingInstance)) {
+        return;
+    }
+    if (r != sizeof(somethingInstance)) {
         return;
     }
 
     r = target_func_3(-3);
     // BAD: comparing with a different sizeof
-    if (r > sizeof(athing)) {
+    if (r > sizeof(struct athing)) {
+        puts("something2");
+    }
+
+    // BAD: comparing with a different sizeof
+    if (r > sizeof(athingInstance)) {
         puts("something2");
     }
 }
@@ -246,13 +263,13 @@ bool some_func_cmp(bool x) {
     return !x;
 }
 
-void test_6_1() {
+void test_7_1() {
     if (some_func_cmp(target_func_7(2))) {
         puts("something");
     }
 }
 
-void test_6_2() {
+void test_7_2() {
     if (some_func_cmp(target_func_7(12)) != cmp_func2(-123)) {
         puts("something");
     }
@@ -268,6 +285,37 @@ void test_6_2() {
     }
 }
 
+// BAD 8
+bool target_func_8(int a)
+{
+    return a > 10 ? true : false;
+}
+int some_func_arithmetic(int x) {
+    return x*3;
+}
+
+void test_8_1() {
+    if (target_func_8(2)*4 == 2*5+6) {
+        puts("something");
+    }
+}
+
+void test_8_2() {
+    if (target_func_8(12) != 4*some_func_arithmetic(-3)) {
+        puts("something");
+    }
+    bool r = target_func_8(123);
+    if (r*2 < sizeof(something)) {
+        return;
+    }
+
+    r = target_func_8(-3);
+    // BAD: comparing with constant instead of dynamically computed value
+    if (r != 3) {
+        puts("something8");
+    }
+}
+
 // GOOD 1: always comparing with NULL
 int* target_func_g1(int a)
 {
diff --git a/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.expected b/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.expected
index 0ae1442..288667c 100644
--- a/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.expected
+++ b/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.expected
@@ -1 +1,8 @@
-| new query passed |
+| InconsistentReturnValueHandling.cpp:35:9:35:21 | call to target_func_1 | Function $@ return value is usually compared with numeric literal (3 times), but this call compares with sizeof(something) | InconsistentReturnValueHandling.cpp:28:5:28:17 | target_func_1 | target_func_1 |
+| InconsistentReturnValueHandling.cpp:76:9:76:21 | call to target_func_2 | Function $@ return value is usually compared with sizeof(something) (3 times), but this call compares with numeric literal | InconsistentReturnValueHandling.cpp:56:5:56:17 | target_func_2 | target_func_2 |
+| InconsistentReturnValueHandling.cpp:116:9:116:21 | call to target_func_3 | Function $@ return value is usually compared with sizeof(something) (6 times), but this call compares with sizeof(athing) | InconsistentReturnValueHandling.cpp:84:5:84:17 | target_func_3 | target_func_3 |
+| InconsistentReturnValueHandling.cpp:188:13:188:25 | call to target_func_4 | Function $@ return value is usually compared with numeric literal (8 times), but this call compares with bool | InconsistentReturnValueHandling.cpp:195:5:195:28 | target_func_4 | target_func_4 |
+| InconsistentReturnValueHandling.cpp:221:9:221:21 | call to target_func_5 | Function $@ return value is usually compared with null (3 times), but this call compares with pointer | InconsistentReturnValueHandling.cpp:201:6:201:18 | target_func_5 | target_func_5 |
+| InconsistentReturnValueHandling.cpp:250:9:250:21 | call to target_func_6 | Function $@ return value is usually compared with some function's return value (3 times), but this call compares with sizeof(something) | InconsistentReturnValueHandling.cpp:230:6:230:18 | target_func_6 | target_func_6 |
+| InconsistentReturnValueHandling.cpp:281:9:281:21 | call to target_func_7 | Function $@ return value is usually compared within call to some function (3 times), but this call compares with some function's return value | InconsistentReturnValueHandling.cpp:258:6:258:18 | target_func_7 | target_func_7 |
+| InconsistentReturnValueHandling.cpp:312:9:312:21 | call to target_func_8 | Function $@ return value is usually compared with arithmetic expression (3 times), but this call compares with numeric literal | InconsistentReturnValueHandling.cpp:289:6:289:18 | target_func_8 | target_func_8 |
diff --git a/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.qlref b/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.qlref
index 112388d..47ca537 100644
--- a/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.qlref
+++ b/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.qlref
@@ -1,2 +1,2 @@
-InconsistentReturnValueHandling/InconsistentReturnValueHandling.ql
+security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.ql
 

From d701d86202385bdf001ca518297dc926290aef7a Mon Sep 17 00:00:00 2001
From: GrosQuildu <e2.8a.95@gmail.com>
Date: Mon, 8 Jul 2024 15:25:35 +0200
Subject: [PATCH 03/10] fix string stubs

---
 cpp/test/include/libc/string_stubs.h | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/cpp/test/include/libc/string_stubs.h b/cpp/test/include/libc/string_stubs.h
index 0f7889d..16e9058 100644
--- a/cpp/test/include/libc/string_stubs.h
+++ b/cpp/test/include/libc/string_stubs.h
@@ -1,20 +1,39 @@
 #ifndef USE_HEADERS
 
+#ifndef HEADER_STRINGSTUB_STUB_H
+#define HEADER_STRINGSTUB_STUB_H
+
+#ifdef  __cplusplus
+extern "C" {
+#endif
+
 #ifndef NULL
     #define NULL 0
 #endif
 
-typedef int wchar_t;
+#define wchar_t int
 extern char* strcpy_s(char* dst, int max_amount, char* src);
 extern int _mbsncat(char* dst, char* src, int count);
 extern int _mbsncmp(char* dst, char* src, int count);
 extern int _memicmp(char* a, char* b, long count);
 extern int _mbsnbcmp(char* a, char* b, int count);
-extern void *printf(const char * format, ...);
-extern int strlen(const char *s);
+extern int printf(const char * format, ...);
+extern unsigned long strlen(const char *s);
 extern char* strcpy(char * dst, const char * src);
 extern int wprintf(const wchar_t * format, ...);
 extern wchar_t* wcscpy(wchar_t * s1, const wchar_t * s2);
+extern void perror(const char *s);
+extern int puts(const char *s);
+
+extern void openlog(const char*, int, int);
+extern void syslog(int, const char*, ...);
+extern void closelog(void);
+
+#ifdef  __cplusplus
+}
+#endif
+
+#endif
 
 #else
 
@@ -23,6 +42,7 @@ extern wchar_t* wcscpy(wchar_t * s1, const wchar_t * s2);
 #include <stdio.h>
 #include <stdlib.h>
 #include <wchar.h>
+#include <syslog.h>
 
 #define strcpy_s strcpy
 #define _mbsncat strncat

From f00ccd042935f5480b7c2bbbb2e757be633bd16e Mon Sep 17 00:00:00 2001
From: GrosQuildu <e2.8a.95@gmail.com>
Date: Mon, 8 Jul 2024 18:40:12 +0200
Subject: [PATCH 04/10] InconsistentReturnValueHandling v2 working

---
 .../InconsistentReturnValueHandling.ql        | 126 +++++++++++-------
 .../InconsistentReturnValueHandling.cpp       | 101 +++++++++++++-
 .../InconsistentReturnValueHandling.expected  |  18 +--
 3 files changed, 187 insertions(+), 58 deletions(-)

diff --git a/cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.ql b/cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.ql
index 55aea8c..7766a57 100644
--- a/cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.ql
+++ b/cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.ql
@@ -16,6 +16,9 @@ import cpp
 import semmle.code.cpp.dataflow.new.DataFlow
 import semmle.code.cpp.controlflow.IRGuards
 
+/**
+ * Categories for uses of functions return values
+ */
 newtype TCmpClass =
   Tint()
   or
@@ -53,24 +56,28 @@ class CmpClass extends TCmpClass {
   }
 }
 
-// TODO: why codeql does not have IntegralLiteral?
+/**
+ * Literals like 3, 123, 43, 2
+ */
+pragma[inline]
 predicate numericLiteral(Expr expr) {
-    (
-        expr instanceof Literal
-        and not (
-            expr instanceof BlockExpr
-            or
-            expr instanceof FormatLiteral	
-            or
-            expr instanceof NULL
-            or
-            expr instanceof TextLiteral
-            or
-            expr instanceof LabelLiteral
-            or
-            expr.getType() instanceof NullPointerType
-        )
-    )
+    expr instanceof Literal
+    and not expr instanceof BlockExpr
+    and not expr instanceof FormatLiteral   
+    and not expr instanceof NULL
+    and not expr instanceof TextLiteral
+    and not expr instanceof LabelLiteral
+    and not expr.getType() instanceof NullPointerType
+    and not expr.getType() instanceof BoolType
+}
+
+/**
+ * Literals like 3, 123, -43, +2
+ * TODO: why codeql for cpp does not have IntegralLiteral?
+ */
+pragma[inline]
+predicate numericArithmLiteral(Expr expr) {
+    numericLiteral(expr)
     or
     (
         expr instanceof UnaryArithmeticOperation
@@ -79,6 +86,7 @@ predicate numericLiteral(Expr expr) {
     )
 }
 
+pragma[inline]
 predicate binaryComputation(Expr e) {
     e instanceof BinaryArithmeticOperation
     or
@@ -87,14 +95,23 @@ predicate binaryComputation(Expr e) {
     e instanceof BinaryLogicalOperation
 }
 
+pragma[inline]
+Expr getOtherOperand(ComparisonOperation cmp, Expr fcRetVal) {
+    (cmp.getRightOperand().getAChild*() = fcRetVal and result = cmp.getLeftOperand())
+    or
+    (cmp.getLeftOperand().getAChild*() = fcRetVal and result = cmp.getRightOperand())
+}
+
 /**
- * Categorize expressions using literals instead of types, because
- * we want to differentiate between functions calls and hard-coded stuff 
+ * Categorize expressions using mostly literals instead of types, because
+ * we want to differentiate between functions calls and hard-coded stuff.
+ * We use ugly if-then-else here in hope to speed up computation a bit
  */
+pragma[inline]
 TCmpClass operandCategory(Expr comparedVal) {
-    (numericLiteral(comparedVal) and not comparedVal.getType() instanceof BoolType and result = Tint())
+    (numericArithmLiteral(comparedVal) and result = Tint())
     or
-    (numericLiteral(comparedVal) and comparedVal.getType() instanceof BoolType and result = Tbool())
+    (comparedVal instanceof Literal and comparedVal.getType() instanceof BoolType and result = Tbool())
     or
     ((comparedVal.getType() instanceof NullPointerType or comparedVal instanceof NULL) and result = Tnull())
     or
@@ -113,46 +130,52 @@ TCmpClass operandCategory(Expr comparedVal) {
     (binaryComputation(comparedVal) and result = Tarithm())
 }
 
-module RetValFlowConfig implements DataFlow::ConfigSig {
-    predicate isSource(DataFlow::Node source) {
-        source = source
-    }
+// module RetValFlowConfig implements DataFlow::ConfigSig {
+//     predicate isSource(DataFlow::Node source) {
+//         source.asExpr() = any(FunctionCall f)
+//     }
 
-    predicate isSink(DataFlow::Node sink) {
-        sink = sink
-    }
-}
-module RetValFlow = DataFlow::Global<RetValFlowConfig>;
-
-Expr getOtherOperand(ComparisonOperation cmp, Expr fcRetVal) {
-    if cmp.getRightOperand().getAChild*() = fcRetVal then
-        result = cmp.getLeftOperand()
-    else
-        result = cmp.getRightOperand()
-}
+//     predicate isSink(DataFlow::Node sink) {
+//         exists(IfStmt ifs | ifs.getCondition().getAChild*() = sink.asExpr())
+//     }
+// }
+// module RetValFlow = DataFlow::Global<RetValFlowConfig>;
 
 predicate categorize(Function f, FunctionCall fc, TCmpClass comparedValCategory, IfStmt ifs) {
     exists(Expr fcRetVal |
         fc.getTarget() = f
-        and RetValFlow::flow(
+        and ifs.getCondition().getAChild*() = fcRetVal
+
+        // we could use global DF with a barrier, but that would return a lot of false positives
+        and DataFlow::localFlow(
             DataFlow::exprNode(fc),
             DataFlow::exprNode(fcRetVal)
         )
-        and ifs.getCondition().getAChild*() = fcRetVal
+        
+        // exclude far-reaching flows, when the ret val is not checked but is actually used
+        // in other words, find only the first use in an IF statement 
+        and not exists(IfStmt ifsPrev |
+            ifsPrev != ifs
+            and DataFlow::localFlow(
+                DataFlow::exprNode(fc),
+                DataFlow::exprNode(ifsPrev.getCondition().getAChild*())
+            )
+        )
+
         and
         if
-            // if(func(retVal) == anything)
+            // if(func(retVal))
             exists(FunctionCall anyFc |
                 ifs.getCondition().getAChild*() = anyFc
                 and anyFc.getAnArgument() = fcRetVal
             )
         then
             comparedValCategory = Targ()
-        else
+        else (
             // if (retVal != comparedVal)
-            exists(ComparisonOperation cmp, Expr comparedVal |
+            exists(ComparisonOperation cmp |
                 ifs.getCondition().getAChild*() = cmp
-                and
+                and (
                     // if (2*retVal+1 != comparedVal)
                     // if (retVal > 2*anything()+sizeof(struct))
                     if (
@@ -161,11 +184,13 @@ predicate categorize(Function f, FunctionCall fc, TCmpClass comparedValCategory,
                     )
                     then
                         comparedValCategory = Tarithm()
-                    else
+                    else (
                         // if (retVal != comparedVal)
-                        comparedVal = getOtherOperand(cmp, fcRetVal)
-                        and comparedValCategory = operandCategory(comparedVal)
+                        comparedValCategory = operandCategory(getOtherOperand(cmp, fcRetVal))
+                    )
+                )
             )
+        )
     )
 }
 
@@ -188,12 +213,14 @@ int mostCommonRetValType(Function f, TCmpClass mostCommonCategory) {
 // where
 //     categorize(f, fc, comparedValCategory, ifs)
 //     and x = comparedValCategory
+//     // and f.getName() = "sshbuf_fromb"
 // select f, fc, x, ifs
 
 
 from Function f, int retValsTotalAmount,
     TCmpClass mostCommonCategory, CmpClass mostCommonCategoryClass, int categoryMax,
-    TCmpClass buggyCategory, CmpClass buggyCategoryClass, FunctionCall buggyFc
+    TCmpClass buggyCategory, CmpClass buggyCategoryClass, FunctionCall buggyFc,
+    IfStmt ifs
 where
     // we are interested only in used functions
     exists(FunctionCall fc | fc.getTarget() = f)
@@ -211,9 +238,10 @@ where
     and ((float)(categoryMax * 100) / retValsTotalAmount) >= 74
 
     // // and finally we are looking for calls that use retVal in an uncommon way
-    and categorize(f, buggyFc, buggyCategory, _)
+    and categorize(f, buggyFc, buggyCategory, ifs)
     and buggyCategory != mostCommonCategory
     and buggyCategoryClass = buggyCategory 
     
 select buggyFc, "Function $@ return value is usually compared with" + mostCommonCategoryClass + " (" + categoryMax +
-    " times), but this call compares with" + buggyCategoryClass, f, f.getName()
+    " of " + retValsTotalAmount + " times), but this call compares with" + buggyCategoryClass + " $@",
+    f, f.getName(), ifs, "here"
diff --git a/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.cpp b/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.cpp
index 123f3f0..38fe2c4 100644
--- a/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.cpp
+++ b/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.cpp
@@ -11,6 +11,8 @@ struct athing {
     int y;
 };
 
+static int w = 2;
+
 bool cmp_func(int a)
 {
     return a >= 10 ? true : false;
@@ -103,12 +105,15 @@ void test_3_2() {
     if (r > sizeof(struct something)) {
         return;
     }
+    r = target_func_3(1223);
     if (r > sizeof(somethingInstance)) {
         return;
     }
+    r = target_func_3(1323);
     if (r >= sizeof(somethingInstance)) {
         return;
     }
+    r = target_func_3(1523);
     if (r != sizeof(somethingInstance)) {
         return;
     }
@@ -119,6 +124,7 @@ void test_3_2() {
         puts("something2");
     }
 
+    r = target_func_3(-3);
     // BAD: comparing with a different sizeof
     if (r > sizeof(athingInstance)) {
         puts("something2");
@@ -197,7 +203,6 @@ int SomeClass::target_func_4(int a) {
 }
 
 // BAD 5
-static int w = 2;
 int* target_func_5(int a)
 {
     return &w;
@@ -316,6 +321,34 @@ void test_8_2() {
     }
 }
 
+// BAD 9
+int* target_func_9(int a)
+{
+    return a > 10 ? &w : NULL;
+}
+
+void test_9_1() {
+    if (target_func_9(2) == NULL) {
+        puts("something");
+    }
+}
+
+void test_9_2() {
+    int *tt;
+    if ((tt = target_func_9(12)) != NULL) {
+        puts("something");
+    }
+    tt = target_func_9(123);
+    if (tt == NULL) {
+        return;
+    }
+
+    // BAD: comparing with int instead of NULL
+    if ((int)(tt = target_func_9(-3)) != 0) {
+        puts("something8");
+    }
+}
+
 // GOOD 1: always comparing with NULL
 int* target_func_g1(int a)
 {
@@ -368,4 +401,70 @@ void test_g_2_2() {
     if (r > cmp_func2(-3)) {
         puts("something2");
     }
+}
+
+// GOOD 3: always comparing with int, check only first use
+bool target_func_g3(int a)
+{
+    return a > 10 ? true : false;
+}
+
+void test_g_3_1() {
+    if (target_func_g3(2) != 3) {
+        puts("something");
+    }
+}
+
+void test_g_3_2() {
+    if (target_func_g3(-123) != 4) {
+        puts("something");
+    }
+    bool r = target_func_g3(123);
+    if (r == 777) {
+        return;
+    }
+
+    r = target_func_g3(-3);
+    if (r > -123) {
+        puts("something2");
+    }
+    // We don't want to find this use, because it is not a ret val check
+    // but an actuall use
+    if (cmp_func(r)) {
+        return;
+    }
+}
+
+// GOOD 4: always comparing with int, handle multi-condition IFs
+bool target_func_g4(int a)
+{
+    return a > 10 ? true : false;
+}
+
+void test_g_4_1() {
+    if (target_func_g4(2) != 3) {
+        puts("something");
+    }
+}
+
+void test_g_4_2() {
+    int *www = &w;
+
+    if (target_func_g4(-123) != 4) {
+        puts("something");
+    }
+    bool r = target_func_g4(123);
+    if (r == 777) {
+        return;
+    }
+
+    r = target_func_g4(-3);
+    if (r > -123) {
+        puts("something2");
+    }
+
+    bool rr = target_func_g4(-3);
+    if (www == NULL || rr > -123) {
+        puts("something2");
+    }
 }
\ No newline at end of file
diff --git a/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.expected b/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.expected
index 288667c..decd14a 100644
--- a/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.expected
+++ b/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.expected
@@ -1,8 +1,10 @@
-| InconsistentReturnValueHandling.cpp:35:9:35:21 | call to target_func_1 | Function $@ return value is usually compared with numeric literal (3 times), but this call compares with sizeof(something) | InconsistentReturnValueHandling.cpp:28:5:28:17 | target_func_1 | target_func_1 |
-| InconsistentReturnValueHandling.cpp:76:9:76:21 | call to target_func_2 | Function $@ return value is usually compared with sizeof(something) (3 times), but this call compares with numeric literal | InconsistentReturnValueHandling.cpp:56:5:56:17 | target_func_2 | target_func_2 |
-| InconsistentReturnValueHandling.cpp:116:9:116:21 | call to target_func_3 | Function $@ return value is usually compared with sizeof(something) (6 times), but this call compares with sizeof(athing) | InconsistentReturnValueHandling.cpp:84:5:84:17 | target_func_3 | target_func_3 |
-| InconsistentReturnValueHandling.cpp:188:13:188:25 | call to target_func_4 | Function $@ return value is usually compared with numeric literal (8 times), but this call compares with bool | InconsistentReturnValueHandling.cpp:195:5:195:28 | target_func_4 | target_func_4 |
-| InconsistentReturnValueHandling.cpp:221:9:221:21 | call to target_func_5 | Function $@ return value is usually compared with null (3 times), but this call compares with pointer | InconsistentReturnValueHandling.cpp:201:6:201:18 | target_func_5 | target_func_5 |
-| InconsistentReturnValueHandling.cpp:250:9:250:21 | call to target_func_6 | Function $@ return value is usually compared with some function's return value (3 times), but this call compares with sizeof(something) | InconsistentReturnValueHandling.cpp:230:6:230:18 | target_func_6 | target_func_6 |
-| InconsistentReturnValueHandling.cpp:281:9:281:21 | call to target_func_7 | Function $@ return value is usually compared within call to some function (3 times), but this call compares with some function's return value | InconsistentReturnValueHandling.cpp:258:6:258:18 | target_func_7 | target_func_7 |
-| InconsistentReturnValueHandling.cpp:312:9:312:21 | call to target_func_8 | Function $@ return value is usually compared with arithmetic expression (3 times), but this call compares with numeric literal | InconsistentReturnValueHandling.cpp:289:6:289:18 | target_func_8 | target_func_8 |
+| InconsistentReturnValueHandling.cpp:37:9:37:21 | call to target_func_1 | Function $@ return value is usually compared with numeric literal (3 of 4 times), but this call compares with sizeof(something) | InconsistentReturnValueHandling.cpp:30:5:30:17 | target_func_1 | target_func_1 |
+| InconsistentReturnValueHandling.cpp:78:9:78:21 | call to target_func_2 | Function $@ return value is usually compared with sizeof(something) (3 of 4 times), but this call compares with numeric literal | InconsistentReturnValueHandling.cpp:58:5:58:17 | target_func_2 | target_func_2 |
+| InconsistentReturnValueHandling.cpp:121:9:121:21 | call to target_func_3 | Function $@ return value is usually compared with sizeof(something) (6 of 8 times), but this call compares with sizeof(athing) | InconsistentReturnValueHandling.cpp:86:5:86:17 | target_func_3 | target_func_3 |
+| InconsistentReturnValueHandling.cpp:127:9:127:21 | call to target_func_3 | Function $@ return value is usually compared with sizeof(something) (6 of 8 times), but this call compares with sizeof(athing) | InconsistentReturnValueHandling.cpp:86:5:86:17 | target_func_3 | target_func_3 |
+| InconsistentReturnValueHandling.cpp:194:13:194:25 | call to target_func_4 | Function $@ return value is usually compared with numeric literal (8 of 9 times), but this call compares with bool | InconsistentReturnValueHandling.cpp:201:5:201:28 | target_func_4 | target_func_4 |
+| InconsistentReturnValueHandling.cpp:226:9:226:21 | call to target_func_5 | Function $@ return value is usually compared with null (3 of 4 times), but this call compares with pointer | InconsistentReturnValueHandling.cpp:206:6:206:18 | target_func_5 | target_func_5 |
+| InconsistentReturnValueHandling.cpp:255:9:255:21 | call to target_func_6 | Function $@ return value is usually compared with some function's return value (3 of 4 times), but this call compares with sizeof(something) | InconsistentReturnValueHandling.cpp:235:6:235:18 | target_func_6 | target_func_6 |
+| InconsistentReturnValueHandling.cpp:286:9:286:21 | call to target_func_7 | Function $@ return value is usually compared within call to some function (3 of 4 times), but this call compares with some function's return value | InconsistentReturnValueHandling.cpp:263:6:263:18 | target_func_7 | target_func_7 |
+| InconsistentReturnValueHandling.cpp:317:9:317:21 | call to target_func_8 | Function $@ return value is usually compared with arithmetic expression (3 of 4 times), but this call compares with numeric literal | InconsistentReturnValueHandling.cpp:294:6:294:18 | target_func_8 | target_func_8 |
+| InconsistentReturnValueHandling.cpp:347:20:347:32 | call to target_func_9 | Function $@ return value is usually compared with null (3 of 4 times), but this call compares with numeric literal | InconsistentReturnValueHandling.cpp:325:6:325:18 | target_func_9 | target_func_9 |

From cc57e58e6c651dd338dc65e18d8178674df79c9a Mon Sep 17 00:00:00 2001
From: GrosQuildu <e2.8a.95@gmail.com>
Date: Mon, 8 Jul 2024 19:08:43 +0200
Subject: [PATCH 05/10] InconsistentReturnValueHandling - reduced FPs

---
 .../InconsistentReturnValueHandling.ql        | 46 +++++++++++++------
 .../InconsistentReturnValueHandling.expected  | 20 ++++----
 2 files changed, 43 insertions(+), 23 deletions(-)

diff --git a/cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.ql b/cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.ql
index 7766a57..c9120fa 100644
--- a/cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.ql
+++ b/cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.ql
@@ -17,7 +17,7 @@ import semmle.code.cpp.dataflow.new.DataFlow
 import semmle.code.cpp.controlflow.IRGuards
 
 /**
- * Categories for uses of functions return values
+ * Categories for uses of functions' return values
  */
 newtype TCmpClass =
   Tint()
@@ -105,7 +105,6 @@ Expr getOtherOperand(ComparisonOperation cmp, Expr fcRetVal) {
 /**
  * Categorize expressions using mostly literals instead of types, because
  * we want to differentiate between functions calls and hard-coded stuff.
- * We use ugly if-then-else here in hope to speed up computation a bit
  */
 pragma[inline]
 TCmpClass operandCategory(Expr comparedVal) {
@@ -117,7 +116,7 @@ TCmpClass operandCategory(Expr comparedVal) {
     or
     (comparedVal.getUnderlyingType() instanceof DerivedType and result = Tptr())
     or
-    (comparedVal instanceof FunctionCall and result = Tcall())
+    (comparedVal instanceof Call and result = Tcall())
     or
     (comparedVal instanceof SizeofOperator and
         (
@@ -132,7 +131,7 @@ TCmpClass operandCategory(Expr comparedVal) {
 
 // module RetValFlowConfig implements DataFlow::ConfigSig {
 //     predicate isSource(DataFlow::Node source) {
-//         source.asExpr() = any(FunctionCall f)
+//         source.asExpr() = any(Call f)
 //     }
 
 //     predicate isSink(DataFlow::Node sink) {
@@ -141,7 +140,11 @@ TCmpClass operandCategory(Expr comparedVal) {
 // }
 // module RetValFlow = DataFlow::Global<RetValFlowConfig>;
 
-predicate categorize(Function f, FunctionCall fc, TCmpClass comparedValCategory, IfStmt ifs) {
+/**
+ * Given function's return value, find its first use in an IF statement
+ * and assign proper TCmpClass category
+ */
+predicate categorize(Function f, Call fc, TCmpClass comparedValCategory, IfStmt ifs) {
     exists(Expr fcRetVal |
         fc.getTarget() = f
         and ifs.getCondition().getAChild*() = fcRetVal
@@ -165,9 +168,9 @@ predicate categorize(Function f, FunctionCall fc, TCmpClass comparedValCategory,
         and
         if
             // if(func(retVal))
-            exists(FunctionCall anyFc |
+            exists(Call anyFc |
                 ifs.getCondition().getAChild*() = anyFc
-                and anyFc.getAnArgument() = fcRetVal
+                and anyFc.getAnArgument().getAChild*() = fcRetVal
             )
         then
             comparedValCategory = Targ()
@@ -175,6 +178,11 @@ predicate categorize(Function f, FunctionCall fc, TCmpClass comparedValCategory,
             // if (retVal != comparedVal)
             exists(ComparisonOperation cmp |
                 ifs.getCondition().getAChild*() = cmp
+                // skip if we are passing the return value to some function
+                and not exists(Call tmpCall |
+                    cmp.getAChild*() = tmpCall
+                    and tmpCall.getAnArgument().getAChild*() = fcRetVal
+                )
                 and (
                     // if (2*retVal+1 != comparedVal)
                     // if (retVal > 2*anything()+sizeof(struct))
@@ -194,6 +202,10 @@ predicate categorize(Function f, FunctionCall fc, TCmpClass comparedValCategory,
     )
 }
 
+/**
+ * Count all eligible IF statements that
+ * checks return values of the given function
+ */
 int countAllRetValTypes(Function f) {
     result = count(IfStmt ifs |
         categorize(f, _, _, ifs) |
@@ -201,6 +213,10 @@ int countAllRetValTypes(Function f) {
     )
 }
 
+/**
+ * Determine what is the most commont TCmpClass category
+ * for the given function (by counting eligible IF statements)
+ */
 int mostCommonRetValType(Function f, TCmpClass mostCommonCategory) {
     result = max(int numberOfRetValTypeInstances |
         categorize(f, _, mostCommonCategory, _)
@@ -209,7 +225,7 @@ int mostCommonRetValType(Function f, TCmpClass mostCommonCategory) {
 }
 
 // uncomment for testing:
-// from Function f, FunctionCall fc, TCmpClass comparedValCategory, CmpClass x, IfStmt ifs
+// from Function f, Call fc, TCmpClass comparedValCategory, CmpClass x, IfStmt ifs
 // where
 //     categorize(f, fc, comparedValCategory, ifs)
 //     and x = comparedValCategory
@@ -219,12 +235,12 @@ int mostCommonRetValType(Function f, TCmpClass mostCommonCategory) {
 
 from Function f, int retValsTotalAmount,
     TCmpClass mostCommonCategory, CmpClass mostCommonCategoryClass, int categoryMax,
-    TCmpClass buggyCategory, CmpClass buggyCategoryClass, FunctionCall buggyFc,
+    TCmpClass buggyCategory, CmpClass buggyCategoryClass, Call buggyFc,
     IfStmt ifs
 where
-    // we are interested only in used functions
-    exists(FunctionCall fc | fc.getTarget() = f)
-    // and f.hasDefinition()
+    // we are interested only in defined (e.g., not libc) and used functions
+    exists(Call fc | fc.getTarget() = f)
+    and f.hasDefinition()
 
     // the function's retVal must be used in some IF statements
     and retValsTotalAmount = countAllRetValTypes(f)
@@ -240,7 +256,11 @@ where
     // // and finally we are looking for calls that use retVal in an uncommon way
     and categorize(f, buggyFc, buggyCategory, ifs)
     and buggyCategory != mostCommonCategory
-    and buggyCategoryClass = buggyCategory 
+    and buggyCategoryClass = buggyCategory
+
+    // return value could be used multiple times in a single IF statement
+    // don't show such findings
+    and not categoryMax = retValsTotalAmount
     
 select buggyFc, "Function $@ return value is usually compared with" + mostCommonCategoryClass + " (" + categoryMax +
     " of " + retValsTotalAmount + " times), but this call compares with" + buggyCategoryClass + " $@",
diff --git a/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.expected b/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.expected
index decd14a..6d06444 100644
--- a/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.expected
+++ b/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.expected
@@ -1,10 +1,10 @@
-| InconsistentReturnValueHandling.cpp:37:9:37:21 | call to target_func_1 | Function $@ return value is usually compared with numeric literal (3 of 4 times), but this call compares with sizeof(something) | InconsistentReturnValueHandling.cpp:30:5:30:17 | target_func_1 | target_func_1 |
-| InconsistentReturnValueHandling.cpp:78:9:78:21 | call to target_func_2 | Function $@ return value is usually compared with sizeof(something) (3 of 4 times), but this call compares with numeric literal | InconsistentReturnValueHandling.cpp:58:5:58:17 | target_func_2 | target_func_2 |
-| InconsistentReturnValueHandling.cpp:121:9:121:21 | call to target_func_3 | Function $@ return value is usually compared with sizeof(something) (6 of 8 times), but this call compares with sizeof(athing) | InconsistentReturnValueHandling.cpp:86:5:86:17 | target_func_3 | target_func_3 |
-| InconsistentReturnValueHandling.cpp:127:9:127:21 | call to target_func_3 | Function $@ return value is usually compared with sizeof(something) (6 of 8 times), but this call compares with sizeof(athing) | InconsistentReturnValueHandling.cpp:86:5:86:17 | target_func_3 | target_func_3 |
-| InconsistentReturnValueHandling.cpp:194:13:194:25 | call to target_func_4 | Function $@ return value is usually compared with numeric literal (8 of 9 times), but this call compares with bool | InconsistentReturnValueHandling.cpp:201:5:201:28 | target_func_4 | target_func_4 |
-| InconsistentReturnValueHandling.cpp:226:9:226:21 | call to target_func_5 | Function $@ return value is usually compared with null (3 of 4 times), but this call compares with pointer | InconsistentReturnValueHandling.cpp:206:6:206:18 | target_func_5 | target_func_5 |
-| InconsistentReturnValueHandling.cpp:255:9:255:21 | call to target_func_6 | Function $@ return value is usually compared with some function's return value (3 of 4 times), but this call compares with sizeof(something) | InconsistentReturnValueHandling.cpp:235:6:235:18 | target_func_6 | target_func_6 |
-| InconsistentReturnValueHandling.cpp:286:9:286:21 | call to target_func_7 | Function $@ return value is usually compared within call to some function (3 of 4 times), but this call compares with some function's return value | InconsistentReturnValueHandling.cpp:263:6:263:18 | target_func_7 | target_func_7 |
-| InconsistentReturnValueHandling.cpp:317:9:317:21 | call to target_func_8 | Function $@ return value is usually compared with arithmetic expression (3 of 4 times), but this call compares with numeric literal | InconsistentReturnValueHandling.cpp:294:6:294:18 | target_func_8 | target_func_8 |
-| InconsistentReturnValueHandling.cpp:347:20:347:32 | call to target_func_9 | Function $@ return value is usually compared with null (3 of 4 times), but this call compares with numeric literal | InconsistentReturnValueHandling.cpp:325:6:325:18 | target_func_9 | target_func_9 |
+| InconsistentReturnValueHandling.cpp:37:9:37:21 | call to target_func_1 | Function $@ return value is usually compared with numeric literal (3 of 4 times), but this call compares with sizeof(something) $@ | InconsistentReturnValueHandling.cpp:30:5:30:17 | target_func_1 | target_func_1 | InconsistentReturnValueHandling.cpp:37:5:39:5 | if (...) ...  | here |
+| InconsistentReturnValueHandling.cpp:78:9:78:21 | call to target_func_2 | Function $@ return value is usually compared with sizeof(something) (3 of 4 times), but this call compares with numeric literal $@ | InconsistentReturnValueHandling.cpp:58:5:58:17 | target_func_2 | target_func_2 | InconsistentReturnValueHandling.cpp:80:5:82:5 | if (...) ...  | here |
+| InconsistentReturnValueHandling.cpp:121:9:121:21 | call to target_func_3 | Function $@ return value is usually compared with sizeof(something) (6 of 8 times), but this call compares with sizeof(athing) $@ | InconsistentReturnValueHandling.cpp:86:5:86:17 | target_func_3 | target_func_3 | InconsistentReturnValueHandling.cpp:123:5:125:5 | if (...) ...  | here |
+| InconsistentReturnValueHandling.cpp:127:9:127:21 | call to target_func_3 | Function $@ return value is usually compared with sizeof(something) (6 of 8 times), but this call compares with sizeof(athing) $@ | InconsistentReturnValueHandling.cpp:86:5:86:17 | target_func_3 | target_func_3 | InconsistentReturnValueHandling.cpp:129:5:131:5 | if (...) ...  | here |
+| InconsistentReturnValueHandling.cpp:194:13:194:25 | call to target_func_4 | Function $@ return value is usually compared with numeric literal (8 of 9 times), but this call compares with bool $@ | InconsistentReturnValueHandling.cpp:201:5:201:28 | target_func_4 | target_func_4 | InconsistentReturnValueHandling.cpp:196:5:198:5 | if (...) ...  | here |
+| InconsistentReturnValueHandling.cpp:226:9:226:21 | call to target_func_5 | Function $@ return value is usually compared with null (3 of 4 times), but this call compares with pointer $@ | InconsistentReturnValueHandling.cpp:206:6:206:18 | target_func_5 | target_func_5 | InconsistentReturnValueHandling.cpp:229:5:231:5 | if (...) ...  | here |
+| InconsistentReturnValueHandling.cpp:255:9:255:21 | call to target_func_6 | Function $@ return value is usually compared with some function's return value (3 of 4 times), but this call compares with sizeof(something) $@ | InconsistentReturnValueHandling.cpp:235:6:235:18 | target_func_6 | target_func_6 | InconsistentReturnValueHandling.cpp:257:5:259:5 | if (...) ...  | here |
+| InconsistentReturnValueHandling.cpp:286:9:286:21 | call to target_func_7 | Function $@ return value is usually compared within call to some function (3 of 4 times), but this call compares with some function's return value $@ | InconsistentReturnValueHandling.cpp:263:6:263:18 | target_func_7 | target_func_7 | InconsistentReturnValueHandling.cpp:288:5:290:5 | if (...) ...  | here |
+| InconsistentReturnValueHandling.cpp:317:9:317:21 | call to target_func_8 | Function $@ return value is usually compared with arithmetic expression (3 of 4 times), but this call compares with numeric literal $@ | InconsistentReturnValueHandling.cpp:294:6:294:18 | target_func_8 | target_func_8 | InconsistentReturnValueHandling.cpp:319:5:321:5 | if (...) ...  | here |
+| InconsistentReturnValueHandling.cpp:347:20:347:32 | call to target_func_9 | Function $@ return value is usually compared with null (3 of 4 times), but this call compares with numeric literal $@ | InconsistentReturnValueHandling.cpp:325:6:325:18 | target_func_9 | target_func_9 | InconsistentReturnValueHandling.cpp:347:5:349:5 | if (...) ...  | here |

From 72351a976f1ff5eb4c980165acdc7f1930a7a154 Mon Sep 17 00:00:00 2001
From: GrosQuildu <e2.8a.95@gmail.com>
Date: Tue, 9 Jul 2024 10:32:47 +0200
Subject: [PATCH 06/10] InconsistentReturnValueHandling - small changes

---
 .../InconsistentReturnValueHandling.ql                       | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.ql b/cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.ql
index c9120fa..9e905a0 100644
--- a/cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.ql
+++ b/cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.ql
@@ -52,7 +52,7 @@ class CmpClass extends TCmpClass {
     or
     this = Tarithm() and result = " arithmetic expression"
     or
-    this = Targ() and result = "in call to some function"
+    this = Targ() and result = "in a function"
   }
 }
 
@@ -238,8 +238,9 @@ from Function f, int retValsTotalAmount,
     TCmpClass buggyCategory, CmpClass buggyCategoryClass, Call buggyFc,
     IfStmt ifs
 where
+    not buggyFc.getLocation().getFile().toString().toLowerCase().regexpMatch(".*test.*")
     // we are interested only in defined (e.g., not libc) and used functions
-    exists(Call fc | fc.getTarget() = f)
+    and exists(Call fc | fc.getTarget() = f)
     and f.hasDefinition()
 
     // the function's retVal must be used in some IF statements

From 1f2598e9dcd005e2831f4635180a8c642fe499e5 Mon Sep 17 00:00:00 2001
From: GrosQuildu <e2.8a.95@gmail.com>
Date: Fri, 13 Feb 2026 11:15:30 +0100
Subject: [PATCH 07/10] fix tests

---
 .../InconsistentReturnValueHandling.ql        | 11 +++++-----
 cpp/test/include/libc/string_stubs.h          |  2 ++
 .../InconsistentReturnValueHandling.cpp       | 15 +++++++-------
 .../InconsistentReturnValueHandling.expected  | 20 +++++++++----------
 4 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.ql b/cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.ql
index 9e905a0..12af344 100644
--- a/cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.ql
+++ b/cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.ql
@@ -1,8 +1,8 @@
 /**
  * @name Inconsistent handling of return values from a specific function
  * @description If a function's return value is used in `if` statements,
- *              and in a few statements the value is compared somehow differently than it is usually,
- *              then this rare comparisons may indicate bugs.
+ *              and in a few statements the value is compared differently than it is usually,
+ *              then this rare comparisons are bugs indicators.
  *              The query categorizes uses of return values into a few categories
  *              (cmp with int, bool, nullptr, sizeof, another function, ...)
  * @kind problem
@@ -238,9 +238,10 @@ from Function f, int retValsTotalAmount,
     TCmpClass buggyCategory, CmpClass buggyCategoryClass, Call buggyFc,
     IfStmt ifs
 where
-    not buggyFc.getLocation().getFile().toString().toLowerCase().regexpMatch(".*test.*")
+    // not buggyFc.getLocation().getFile().toString().toLowerCase().regexpMatch(".*test.*") and
+
     // we are interested only in defined (e.g., not libc) and used functions
-    and exists(Call fc | fc.getTarget() = f)
+    exists(Call fc | fc.getTarget() = f)
     and f.hasDefinition()
 
     // the function's retVal must be used in some IF statements
@@ -254,7 +255,7 @@ where
     // if threshold for "most common" use case is ~75%, then remaining 25% function calls are handled somehow incorrectly
     and ((float)(categoryMax * 100) / retValsTotalAmount) >= 74
 
-    // // and finally we are looking for calls that use retVal in an uncommon way
+    // finally we are looking for calls that use retVal in an uncommon way
     and categorize(f, buggyFc, buggyCategory, ifs)
     and buggyCategory != mostCommonCategory
     and buggyCategoryClass = buggyCategory
diff --git a/cpp/test/include/libc/string_stubs.h b/cpp/test/include/libc/string_stubs.h
index c77d0b1..aa70996 100644
--- a/cpp/test/include/libc/string_stubs.h
+++ b/cpp/test/include/libc/string_stubs.h
@@ -11,7 +11,9 @@ extern "C" {
     #define NULL 0
 #endif
 
+#ifndef __cplusplus
 typedef int wchar_t;
+#endif
 extern void *memcpy(void *dst, const void *src, unsigned long n);
 extern char* strcpy_s(char* dst, int max_amount, char* src);
 extern int _mbsncat(char* dst, char* src, int count);
diff --git a/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.cpp b/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.cpp
index 38fe2c4..8721c54 100644
--- a/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.cpp
+++ b/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.cpp
@@ -32,14 +32,8 @@ int target_func_1(int a)
     return a + 1;
 }
 
-void test_1_1() {
-    // BAD: comparing with sizeof instead of hard-coded int
-    if (target_func_1(2) != sizeof(something)) {
-        puts("something");
-    }
-}
-
 void test_1_2() {
+    // the baseline for target_func_1
     if (target_func_1(2) != 1) {
         puts("something2");
     }
@@ -52,7 +46,12 @@ void test_1_2() {
     }
 }
 
-
+void test_1_1() {
+    // BAD: comparing with sizeof instead of hard-coded int
+    if (target_func_1(2) != sizeof(something)) {
+        puts("something");
+    }
+}
 
 // BAD 2
 int target_func_2(int a)
diff --git a/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.expected b/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.expected
index 6d06444..2256d3f 100644
--- a/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.expected
+++ b/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.expected
@@ -1,10 +1,10 @@
-| InconsistentReturnValueHandling.cpp:37:9:37:21 | call to target_func_1 | Function $@ return value is usually compared with numeric literal (3 of 4 times), but this call compares with sizeof(something) $@ | InconsistentReturnValueHandling.cpp:30:5:30:17 | target_func_1 | target_func_1 | InconsistentReturnValueHandling.cpp:37:5:39:5 | if (...) ...  | here |
-| InconsistentReturnValueHandling.cpp:78:9:78:21 | call to target_func_2 | Function $@ return value is usually compared with sizeof(something) (3 of 4 times), but this call compares with numeric literal $@ | InconsistentReturnValueHandling.cpp:58:5:58:17 | target_func_2 | target_func_2 | InconsistentReturnValueHandling.cpp:80:5:82:5 | if (...) ...  | here |
-| InconsistentReturnValueHandling.cpp:121:9:121:21 | call to target_func_3 | Function $@ return value is usually compared with sizeof(something) (6 of 8 times), but this call compares with sizeof(athing) $@ | InconsistentReturnValueHandling.cpp:86:5:86:17 | target_func_3 | target_func_3 | InconsistentReturnValueHandling.cpp:123:5:125:5 | if (...) ...  | here |
-| InconsistentReturnValueHandling.cpp:127:9:127:21 | call to target_func_3 | Function $@ return value is usually compared with sizeof(something) (6 of 8 times), but this call compares with sizeof(athing) $@ | InconsistentReturnValueHandling.cpp:86:5:86:17 | target_func_3 | target_func_3 | InconsistentReturnValueHandling.cpp:129:5:131:5 | if (...) ...  | here |
-| InconsistentReturnValueHandling.cpp:194:13:194:25 | call to target_func_4 | Function $@ return value is usually compared with numeric literal (8 of 9 times), but this call compares with bool $@ | InconsistentReturnValueHandling.cpp:201:5:201:28 | target_func_4 | target_func_4 | InconsistentReturnValueHandling.cpp:196:5:198:5 | if (...) ...  | here |
-| InconsistentReturnValueHandling.cpp:226:9:226:21 | call to target_func_5 | Function $@ return value is usually compared with null (3 of 4 times), but this call compares with pointer $@ | InconsistentReturnValueHandling.cpp:206:6:206:18 | target_func_5 | target_func_5 | InconsistentReturnValueHandling.cpp:229:5:231:5 | if (...) ...  | here |
-| InconsistentReturnValueHandling.cpp:255:9:255:21 | call to target_func_6 | Function $@ return value is usually compared with some function's return value (3 of 4 times), but this call compares with sizeof(something) $@ | InconsistentReturnValueHandling.cpp:235:6:235:18 | target_func_6 | target_func_6 | InconsistentReturnValueHandling.cpp:257:5:259:5 | if (...) ...  | here |
-| InconsistentReturnValueHandling.cpp:286:9:286:21 | call to target_func_7 | Function $@ return value is usually compared within call to some function (3 of 4 times), but this call compares with some function's return value $@ | InconsistentReturnValueHandling.cpp:263:6:263:18 | target_func_7 | target_func_7 | InconsistentReturnValueHandling.cpp:288:5:290:5 | if (...) ...  | here |
-| InconsistentReturnValueHandling.cpp:317:9:317:21 | call to target_func_8 | Function $@ return value is usually compared with arithmetic expression (3 of 4 times), but this call compares with numeric literal $@ | InconsistentReturnValueHandling.cpp:294:6:294:18 | target_func_8 | target_func_8 | InconsistentReturnValueHandling.cpp:319:5:321:5 | if (...) ...  | here |
-| InconsistentReturnValueHandling.cpp:347:20:347:32 | call to target_func_9 | Function $@ return value is usually compared with null (3 of 4 times), but this call compares with numeric literal $@ | InconsistentReturnValueHandling.cpp:325:6:325:18 | target_func_9 | target_func_9 | InconsistentReturnValueHandling.cpp:347:5:349:5 | if (...) ...  | here |
+| InconsistentReturnValueHandling.cpp:51:9:51:21 | call to target_func_1 | Function $@ return value is usually compared with numeric literal (3 of 4 times), but this call compares with sizeof(something) $@ | InconsistentReturnValueHandling.cpp:30:5:30:17 | target_func_1 | target_func_1 | InconsistentReturnValueHandling.cpp:51:5:53:5 | if (...) ...  | here |
+| InconsistentReturnValueHandling.cpp:77:9:77:21 | call to target_func_2 | Function $@ return value is usually compared with sizeof(something) (3 of 4 times), but this call compares with numeric literal $@ | InconsistentReturnValueHandling.cpp:57:5:57:17 | target_func_2 | target_func_2 | InconsistentReturnValueHandling.cpp:79:5:81:5 | if (...) ...  | here |
+| InconsistentReturnValueHandling.cpp:120:9:120:21 | call to target_func_3 | Function $@ return value is usually compared with sizeof(something) (6 of 8 times), but this call compares with sizeof(athing) $@ | InconsistentReturnValueHandling.cpp:85:5:85:17 | target_func_3 | target_func_3 | InconsistentReturnValueHandling.cpp:122:5:124:5 | if (...) ...  | here |
+| InconsistentReturnValueHandling.cpp:126:9:126:21 | call to target_func_3 | Function $@ return value is usually compared with sizeof(something) (6 of 8 times), but this call compares with sizeof(athing) $@ | InconsistentReturnValueHandling.cpp:85:5:85:17 | target_func_3 | target_func_3 | InconsistentReturnValueHandling.cpp:128:5:130:5 | if (...) ...  | here |
+| InconsistentReturnValueHandling.cpp:193:13:193:25 | call to target_func_4 | Function $@ return value is usually compared with numeric literal (8 of 9 times), but this call compares with bool $@ | InconsistentReturnValueHandling.cpp:200:5:200:28 | target_func_4 | target_func_4 | InconsistentReturnValueHandling.cpp:195:5:197:5 | if (...) ...  | here |
+| InconsistentReturnValueHandling.cpp:225:9:225:21 | call to target_func_5 | Function $@ return value is usually compared with null (3 of 4 times), but this call compares with pointer $@ | InconsistentReturnValueHandling.cpp:205:6:205:18 | target_func_5 | target_func_5 | InconsistentReturnValueHandling.cpp:228:5:230:5 | if (...) ...  | here |
+| InconsistentReturnValueHandling.cpp:254:9:254:21 | call to target_func_6 | Function $@ return value is usually compared with some function's return value (3 of 4 times), but this call compares with sizeof(something) $@ | InconsistentReturnValueHandling.cpp:234:6:234:18 | target_func_6 | target_func_6 | InconsistentReturnValueHandling.cpp:256:5:258:5 | if (...) ...  | here |
+| InconsistentReturnValueHandling.cpp:285:9:285:21 | call to target_func_7 | Function $@ return value is usually compared within a function (3 of 4 times), but this call compares with some function's return value $@ | InconsistentReturnValueHandling.cpp:262:6:262:18 | target_func_7 | target_func_7 | InconsistentReturnValueHandling.cpp:287:5:289:5 | if (...) ...  | here |
+| InconsistentReturnValueHandling.cpp:316:9:316:21 | call to target_func_8 | Function $@ return value is usually compared with arithmetic expression (3 of 4 times), but this call compares with numeric literal $@ | InconsistentReturnValueHandling.cpp:293:6:293:18 | target_func_8 | target_func_8 | InconsistentReturnValueHandling.cpp:318:5:320:5 | if (...) ...  | here |
+| InconsistentReturnValueHandling.cpp:346:20:346:32 | call to target_func_9 | Function $@ return value is usually compared with null (3 of 4 times), but this call compares with numeric literal $@ | InconsistentReturnValueHandling.cpp:324:6:324:18 | target_func_9 | target_func_9 | InconsistentReturnValueHandling.cpp:346:5:348:5 | if (...) ...  | here |

From f03520b9077835202b4ef34683a833724f955d62 Mon Sep 17 00:00:00 2001
From: GrosQuildu <e2.8a.95@gmail.com>
Date: Fri, 13 Feb 2026 15:54:08 +0100
Subject: [PATCH 08/10] finalize

---
 .../DecOverflowWhenComparing.ql               |   5 +-
 .../InconsistentReturnValueHandling.ql        |  39 +-
 .../InconsistentReturnValueHandling.cpp       | 372 ++++++++++++++++++
 .../InconsistentReturnValueHandling.expected  |   8 +
 4 files changed, 398 insertions(+), 26 deletions(-)

diff --git a/cpp/src/security/DecOverflowWhenComparing/DecOverflowWhenComparing.ql b/cpp/src/security/DecOverflowWhenComparing/DecOverflowWhenComparing.ql
index 8a94088..2059014 100644
--- a/cpp/src/security/DecOverflowWhenComparing/DecOverflowWhenComparing.ql
+++ b/cpp/src/security/DecOverflowWhenComparing/DecOverflowWhenComparing.ql
@@ -139,8 +139,7 @@ where
       )
     then cmp.getAFalseSuccessor().getASuccessor*() = varAccAfterOverflow
     else any()
-  ) and
-  // skip vendor code
-  not dec.getFile().getAbsolutePath().toLowerCase().matches(["%vendor%", "%third_party%"])
+  )
+
 select dec, "Unsigned decrementation in comparison ($@) - $@", cmp, cmp.toString(),
   varAccAfterOverflow, varAccAfterOverflow.toString()
diff --git a/cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.ql b/cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.ql
index 12af344..9df0cef 100644
--- a/cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.ql
+++ b/cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.ql
@@ -35,6 +35,8 @@ newtype TCmpClass =
   Targ()
   or
   Tarithm()
+  or
+  Tother()
 
 class CmpClass extends TCmpClass {
   string toString() {
@@ -53,6 +55,8 @@ class CmpClass extends TCmpClass {
     this = Tarithm() and result = " arithmetic expression"
     or
     this = Targ() and result = "in a function"
+    or
+    this = Tother() and result = " other expression"
   }
 }
 
@@ -127,19 +131,19 @@ TCmpClass operandCategory(Expr comparedVal) {
     )
     or
     (binaryComputation(comparedVal) and result = Tarithm())
+    or
+    (
+        not numericArithmLiteral(comparedVal)
+        and not (comparedVal instanceof Literal and comparedVal.getType() instanceof BoolType)
+        and not (comparedVal.getType() instanceof NullPointerType or comparedVal instanceof NULL)
+        and not comparedVal.getUnderlyingType() instanceof DerivedType
+        and not comparedVal instanceof Call
+        and not comparedVal instanceof SizeofOperator
+        and not binaryComputation(comparedVal)
+        and result = Tother()
+    )
 }
 
-// module RetValFlowConfig implements DataFlow::ConfigSig {
-//     predicate isSource(DataFlow::Node source) {
-//         source.asExpr() = any(Call f)
-//     }
-
-//     predicate isSink(DataFlow::Node sink) {
-//         exists(IfStmt ifs | ifs.getCondition().getAChild*() = sink.asExpr())
-//     }
-// }
-// module RetValFlow = DataFlow::Global<RetValFlowConfig>;
-
 /**
  * Given function's return value, find its first use in an IF statement
  * and assign proper TCmpClass category
@@ -156,7 +160,7 @@ predicate categorize(Function f, Call fc, TCmpClass comparedValCategory, IfStmt
         )
         
         // exclude far-reaching flows, when the ret val is not checked but is actually used
-        // in other words, find only the first use in an IF statement 
+        // in other words, find only the first use in an IF statement
         and not exists(IfStmt ifsPrev |
             ifsPrev != ifs
             and DataFlow::localFlow(
@@ -224,22 +228,11 @@ int mostCommonRetValType(Function f, TCmpClass mostCommonCategory) {
     )
 }
 
-// uncomment for testing:
-// from Function f, Call fc, TCmpClass comparedValCategory, CmpClass x, IfStmt ifs
-// where
-//     categorize(f, fc, comparedValCategory, ifs)
-//     and x = comparedValCategory
-//     // and f.getName() = "sshbuf_fromb"
-// select f, fc, x, ifs
-
-
 from Function f, int retValsTotalAmount,
     TCmpClass mostCommonCategory, CmpClass mostCommonCategoryClass, int categoryMax,
     TCmpClass buggyCategory, CmpClass buggyCategoryClass, Call buggyFc,
     IfStmt ifs
 where
-    // not buggyFc.getLocation().getFile().toString().toLowerCase().regexpMatch(".*test.*") and
-
     // we are interested only in defined (e.g., not libc) and used functions
     exists(Call fc | fc.getTarget() = f)
     and f.hasDefinition()
diff --git a/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.cpp b/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.cpp
index 8721c54..b6f6d05 100644
--- a/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.cpp
+++ b/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.cpp
@@ -466,4 +466,376 @@ void test_g_4_2() {
     if (www == NULL || rr > -123) {
         puts("something2");
     }
+}
+
+// BAD 10: 4 int comparisons + 1 variable comparison (variable is outlier)
+int target_func_10(int a)
+{
+    return a + 10;
+}
+
+void test_10_1() {
+    if (target_func_10(2) != 1) {
+        puts("something");
+    }
+    if (target_func_10(3) > 0) {
+        puts("something");
+    }
+    int r = target_func_10(4);
+    if (r > 0) {
+        puts("something");
+    }
+    if (target_func_10(7) != 99) {
+        puts("something");
+    }
+}
+
+void test_10_2() {
+    int threshold = 42;
+    // BAD: comparing with a variable instead of hard-coded int
+    if (target_func_10(5) != threshold) {
+        puts("something");
+    }
+}
+
+// BAD 11: 4 enum comparisons + 1 int literal (int literal is outlier)
+enum ErrorCode { EC_OK = 0, EC_FAIL = 1, EC_RETRY = 2, EC_TIMEOUT = 3 };
+
+int target_func_11(int a)
+{
+    return a > 10 ? EC_OK : EC_FAIL;
+}
+
+void test_11_1() {
+    if (target_func_11(2) != EC_OK) {
+        puts("something");
+    }
+    if (target_func_11(3) != EC_FAIL) {
+        puts("something");
+    }
+    int r = target_func_11(4);
+    if (r == EC_RETRY) {
+        puts("something");
+    }
+    r = target_func_11(5);
+    if (r != EC_TIMEOUT) {
+        puts("something");
+    }
+}
+
+void test_11_2() {
+    // BAD: comparing with int literal instead of enum constant
+    if (target_func_11(6) != 42) {
+        puts("something");
+    }
+}
+
+// BAD 12: 3 int + || with two sizeof calls (sizeof is the outlier)
+int target_func_12(int a)
+{
+    return a * 2;
+}
+
+void test_12_1() {
+    if (target_func_12(2) != 1) {
+        puts("something");
+    }
+    if (target_func_12(3) > 0) {
+        puts("something");
+    }
+    int r = target_func_12(4);
+    if (r > 0) {
+        puts("something");
+    }
+}
+
+void test_12_2() {
+    // BAD: sizeof comparison in short-circuit expression
+    if (target_func_12(5) > sizeof(something) || target_func_12(6) > sizeof(something)) {
+        puts("something");
+    }
+}
+
+// GOOD 5: 4 int comparisons + bare if(f()) which is silently dropped
+int target_func_g5(int a)
+{
+    return a + 5;
+}
+
+void test_g_5_1() {
+    if (target_func_g5(2) != 1) {
+        puts("something");
+    }
+    if (target_func_g5(3) > 0) {
+        puts("something");
+    }
+    int r = target_func_g5(4);
+    if (r > 0) {
+        puts("something");
+    }
+    if (target_func_g5(7) != 99) {
+        puts("something");
+    }
+}
+
+void test_g_5_2() {
+    // bare conditional: no comparison, should be silently dropped
+    if (target_func_g5(6)) {
+        puts("something");
+    }
+}
+
+// GOOD 6: only 3 uses, below threshold of 4
+int target_func_g6(int a)
+{
+    return a + 6;
+}
+
+void test_g_6_1() {
+    if (target_func_g6(2) != 1) {
+        puts("something");
+    }
+    if (target_func_g6(3) > 0) {
+        puts("something");
+    }
+}
+
+void test_g_6_2() {
+    if (target_func_g6(4) > sizeof(something)) {
+        puts("something");
+    }
+}
+
+// GOOD 7: 2 int + 2 sizeof, 50/50 split below 74% threshold
+int target_func_g7(int a)
+{
+    return a + 7;
+}
+
+void test_g_7_1() {
+    if (target_func_g7(2) != 1) {
+        puts("something");
+    }
+    if (target_func_g7(3) > 0) {
+        puts("something");
+    }
+}
+
+void test_g_7_2() {
+    if (target_func_g7(4) > sizeof(something)) {
+        puts("something");
+    }
+    int r = target_func_g7(5);
+    if (r > sizeof(something)) {
+        puts("something");
+    }
+}
+
+// GOOD 8: 4 bool comparisons + if(!f()) which is silently dropped
+bool target_func_g8(int a)
+{
+    return a > 10;
+}
+
+void test_g_8_1() {
+    if (target_func_g8(2) != true) {
+        puts("something");
+    }
+    if (target_func_g8(3) == false) {
+        puts("something");
+    }
+    bool r = target_func_g8(4);
+    if (r == true) {
+        puts("something");
+    }
+    if (target_func_g8(5) != false) {
+        puts("something");
+    }
+}
+
+void test_g_8_2() {
+    // negation: no comparison operator, should be silently dropped
+    if (!target_func_g8(6)) {
+        puts("something");
+    }
+}
+
+// BAD 13: bare if(func()) mixed with comparisons, should not affect categorization
+int target_func_13(int a)
+{
+    return a + 13;
+}
+
+void test_13_1() {
+    if (target_func_13(2) != 1) {
+        puts("something");
+    }
+    if (target_func_13(3) > 0) {
+        puts("something");
+    }
+    int r = target_func_13(4);
+    if (r > 0) {
+        puts("something");
+    }
+}
+
+void test_13_2() {
+    // bare conditional: no comparison, should be silently dropped
+    if (target_func_13(5)) {
+        puts("something");
+    }
+    // BAD: sizeof comparison when others use int
+    if (target_func_13(6) > sizeof(something)) {
+        puts("something");
+    }
+}
+
+// BAD 14: comparison in else-if is properly categorized
+int target_func_14(int a)
+{
+    return a + 14;
+}
+
+void test_14_1() {
+    if (target_func_14(2) != 1) {
+        puts("something");
+    }
+    if (target_func_14(3) > 0) {
+        puts("something");
+    }
+    int r = target_func_14(4);
+    if (r > 0) {
+        puts("something");
+    }
+}
+
+void test_14_2() {
+    int x = 42;
+    // BAD: sizeof comparison in else-if branch
+    if (x > 100) {
+        puts("high");
+    } else if (target_func_14(5) > sizeof(something)) {
+        puts("something");
+    }
+}
+
+// BAD 15: comparison inside && short-circuit expression
+int target_func_15(int a)
+{
+    return a + 15;
+}
+
+void test_15_1() {
+    if (target_func_15(2) != 1) {
+        puts("something");
+    }
+    if (target_func_15(3) > 0) {
+        puts("something");
+    }
+    int r = target_func_15(4);
+    if (r > 0) {
+        puts("something");
+    }
+}
+
+void test_15_2() {
+    int other = 42;
+    // BAD: sizeof comparison inside && expression
+    if (target_func_15(5) > sizeof(something) && other > 0) {
+        puts("something");
+    }
+}
+
+// BAD 16: deep variable copy chain before comparison
+int target_func_16(int a)
+{
+    return a + 16;
+}
+
+void test_16_1() {
+    if (target_func_16(2) != 1) {
+        puts("something");
+    }
+    if (target_func_16(3) > 0) {
+        puts("something");
+    }
+    int r = target_func_16(4);
+    if (r > 0) {
+        puts("something");
+    }
+}
+
+void test_16_2() {
+    int r = target_func_16(5);
+    int s = r;
+    // BAD: sizeof comparison after variable copy
+    if (s > sizeof(something)) {
+        puts("something");
+    }
+}
+
+// GOOD 9: all bare if(func()) — nothing categorized, below threshold
+int target_func_g9(int a)
+{
+    return a + 9;
+}
+
+void test_g_9_1() {
+    if (target_func_g9(1)) {
+        puts("something");
+    }
+    if (target_func_g9(2)) {
+        puts("something");
+    }
+    if (target_func_g9(3)) {
+        puts("something");
+    }
+    if (target_func_g9(4)) {
+        puts("something");
+    }
+    if (target_func_g9(5)) {
+        puts("something");
+    }
+}
+
+// GOOD 10: reversed operand order — all same category (Tint)
+int target_func_g10(int a)
+{
+    return a + 100;
+}
+
+void test_g_10_1() {
+    if (target_func_g10(1) > 0) {
+        puts("something");
+    }
+    if (target_func_g10(2) != 5) {
+        puts("something");
+    }
+    if (0 < target_func_g10(3)) {
+        puts("something");
+    }
+    if (target_func_g10(4) >= -1) {
+        puts("something");
+    }
+}
+
+// GOOD 11: return value used only in while/for, not in if
+int target_func_g12(int a)
+{
+    return a + 120;
+}
+
+void test_g_12_1() {
+    while (target_func_g12(1) > 0) {
+        break;
+    }
+    while (target_func_g12(2) > sizeof(something)) {
+        break;
+    }
+    for (int i = 0; target_func_g12(3) > 0; i++) {
+        break;
+    }
+    for (int i = 0; target_func_g12(4) > sizeof(something); i++) {
+        break;
+    }
 }
\ No newline at end of file
diff --git a/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.expected b/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.expected
index 2256d3f..b339210 100644
--- a/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.expected
+++ b/cpp/test/query-tests/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.expected
@@ -8,3 +8,11 @@
 | InconsistentReturnValueHandling.cpp:285:9:285:21 | call to target_func_7 | Function $@ return value is usually compared within a function (3 of 4 times), but this call compares with some function's return value $@ | InconsistentReturnValueHandling.cpp:262:6:262:18 | target_func_7 | target_func_7 | InconsistentReturnValueHandling.cpp:287:5:289:5 | if (...) ...  | here |
 | InconsistentReturnValueHandling.cpp:316:9:316:21 | call to target_func_8 | Function $@ return value is usually compared with arithmetic expression (3 of 4 times), but this call compares with numeric literal $@ | InconsistentReturnValueHandling.cpp:293:6:293:18 | target_func_8 | target_func_8 | InconsistentReturnValueHandling.cpp:318:5:320:5 | if (...) ...  | here |
 | InconsistentReturnValueHandling.cpp:346:20:346:32 | call to target_func_9 | Function $@ return value is usually compared with null (3 of 4 times), but this call compares with numeric literal $@ | InconsistentReturnValueHandling.cpp:324:6:324:18 | target_func_9 | target_func_9 | InconsistentReturnValueHandling.cpp:346:5:348:5 | if (...) ...  | here |
+| InconsistentReturnValueHandling.cpp:496:9:496:22 | call to target_func_10 | Function $@ return value is usually compared with numeric literal (4 of 5 times), but this call compares with other expression $@ | InconsistentReturnValueHandling.cpp:472:5:472:18 | target_func_10 | target_func_10 | InconsistentReturnValueHandling.cpp:496:5:498:5 | if (...) ...  | here |
+| InconsistentReturnValueHandling.cpp:528:9:528:22 | call to target_func_11 | Function $@ return value is usually compared with other expression (4 of 5 times), but this call compares with numeric literal $@ | InconsistentReturnValueHandling.cpp:504:5:504:18 | target_func_11 | target_func_11 | InconsistentReturnValueHandling.cpp:528:5:530:5 | if (...) ...  | here |
+| InconsistentReturnValueHandling.cpp:554:9:554:22 | call to target_func_12 | Function $@ return value is usually compared with numeric literal (3 of 4 times), but this call compares with sizeof(something) $@ | InconsistentReturnValueHandling.cpp:534:5:534:18 | target_func_12 | target_func_12 | InconsistentReturnValueHandling.cpp:554:5:556:5 | if (...) ...  | here |
+| InconsistentReturnValueHandling.cpp:554:50:554:63 | call to target_func_12 | Function $@ return value is usually compared with numeric literal (3 of 4 times), but this call compares with sizeof(something) $@ | InconsistentReturnValueHandling.cpp:534:5:534:18 | target_func_12 | target_func_12 | InconsistentReturnValueHandling.cpp:554:5:556:5 | if (...) ...  | here |
+| InconsistentReturnValueHandling.cpp:688:9:688:22 | call to target_func_13 | Function $@ return value is usually compared with numeric literal (3 of 4 times), but this call compares with sizeof(something) $@ | InconsistentReturnValueHandling.cpp:664:5:664:18 | target_func_13 | target_func_13 | InconsistentReturnValueHandling.cpp:688:5:690:5 | if (...) ...  | here |
+| InconsistentReturnValueHandling.cpp:717:16:717:29 | call to target_func_14 | Function $@ return value is usually compared with numeric literal (3 of 4 times), but this call compares with sizeof(something) $@ | InconsistentReturnValueHandling.cpp:694:5:694:18 | target_func_14 | target_func_14 | InconsistentReturnValueHandling.cpp:717:12:719:5 | if (...) ...  | here |
+| InconsistentReturnValueHandling.cpp:744:9:744:22 | call to target_func_15 | Function $@ return value is usually compared with numeric literal (3 of 4 times), but this call compares with sizeof(something) $@ | InconsistentReturnValueHandling.cpp:723:5:723:18 | target_func_15 | target_func_15 | InconsistentReturnValueHandling.cpp:744:5:746:5 | if (...) ...  | here |
+| InconsistentReturnValueHandling.cpp:769:13:769:26 | call to target_func_16 | Function $@ return value is usually compared with numeric literal (3 of 4 times), but this call compares with sizeof(something) $@ | InconsistentReturnValueHandling.cpp:750:5:750:18 | target_func_16 | target_func_16 | InconsistentReturnValueHandling.cpp:772:5:774:5 | if (...) ...  | here |

From 12f2eb4c88992d5a6430b33c2ed417d17f5795f9 Mon Sep 17 00:00:00 2001
From: GrosQuildu <e2.8a.95@gmail.com>
Date: Fri, 13 Feb 2026 16:14:51 +0100
Subject: [PATCH 09/10] metadata done

---
 README.md                                     |  1 +
 .../InconsistentReturnValueHandling.md        | 69 +++++++++++++++++++
 .../InconsistentReturnValueHandling.c         | 38 ++++++++++
 .../InconsistentReturnValueHandling.qhelp     | 55 ++++++++++++---
 .../InconsistentReturnValueHandling.ql        |  7 +-
 5 files changed, 155 insertions(+), 15 deletions(-)
 create mode 100644 cpp/src/docs/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.md
 create mode 100644 cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.c

diff --git a/README.md b/README.md
index 3669d3c..8325fcd 100644
--- a/README.md
+++ b/README.md
@@ -50,6 +50,7 @@ codeql database analyze database.db --format=sarif-latest --output=./tob.sarif -
 |[Async unsafe signal handler](./cpp/src/docs/security/AsyncUnsafeSignalHandler/AsyncUnsafeSignalHandler.md)|Async unsafe signal handler (like the one used in CVE-2024-6387)|warning|high|
 |[Decrementation overflow when comparing](./cpp/src/docs/security/DecOverflowWhenComparing/DecOverflowWhenComparing.md)|This query finds unsigned integer overflows resulting from unchecked decrementation during comparison.|error|high|
 |[Find all problematic implicit casts](./cpp/src/docs/security/UnsafeImplicitConversions/UnsafeImplicitConversions.md)|Find all implicit casts that may be problematic. That is, casts that may result in unexpected truncation, reinterpretation or widening of values.|error|high|
+|[Inconsistent handling of return values from a specific function](./cpp/src/docs/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.md)|Detects functions whose return values are compared inconsistently across call sites, which may indicate bugs|warning|medium|
 |[Invalid string size passed to string manipulation function](./cpp/src/docs/security/CStrnFinder/CStrnFinder.md)|Finds calls to functions that take as input a string and its size as separate arguments (e.g., `strncmp`, `strncat`, ...) and the size argument is wrong|error|low|
 |[Missing null terminator](./cpp/src/docs/security/NoNullTerminator/NoNullTerminator.md)|This query finds incorrectly initialized strings that are passed to functions expecting null-byte-terminated strings|error|high|
 
diff --git a/cpp/src/docs/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.md b/cpp/src/docs/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.md
new file mode 100644
index 0000000..a8e65b0
--- /dev/null
+++ b/cpp/src/docs/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.md
@@ -0,0 +1,69 @@
+# Inconsistent handling of return values from a specific function
+When a function's return value is checked in `if` statements across multiple call sites, the comparisons typically fall into a consistent pattern (e.g., compared against a numeric literal, `NULL`, or `sizeof`). If a small number of call sites compare the return value in a different way than the majority, these inconsistent comparisons may indicate a bug.
+
+The query categorizes each comparison into one of the following categories:
+
+* Numeric literal (e.g., `ret != -1`)
+* Boolean (e.g., `ret == true`)
+* Null pointer (e.g., `ret != NULL`)
+* Pointer
+* `sizeof` expression (e.g., `ret > sizeof(buf)`)
+* Another function's return value (e.g., `ret != other_func()`)
+* Passed as argument to another function (e.g., `if (check(ret))`)
+* Arithmetic expression
+
+When at least 75% of a function's return value comparisons fall into one category, the remaining comparisons in a different category are flagged as potentially incorrect.
+
+
+## Recommendation
+Review each flagged call site and verify that the comparison matches the function's return value semantics. If the function returns an error code or count, all call sites should compare it consistently. Fix any comparisons that use the wrong type of operand (e.g., comparing an integer return value against `sizeof` when all other sites compare against a numeric literal).
+
+
+## Example
+
+```c
+struct header {
+    int type;
+    int length;
+};
+
+// Returns number of items processed, or -1 on error
+int process_items(int *items, int count) {
+    int processed = 0;
+    for (int i = 0; i < count; i++) {
+        if (items[i] < 0)
+            return -1;
+        processed++;
+    }
+    return processed;
+}
+
+void example() {
+    int items[10];
+    int result;
+
+    // Typical: return value compared with a numeric literal
+    result = process_items(items, 10);
+    if (result > 0) { /* success */ }
+
+    result = process_items(items, 5);
+    if (result != -1) { /* no error */ }
+
+    result = process_items(items, 3);
+    if (result == 0) { /* empty */ }
+
+    result = process_items(items, 8);
+    if (result >= 1) { /* at least one */ }
+
+    // BAD: comparing with sizeof instead of a numeric literal.
+    // This is inconsistent with all other call sites and likely a bug.
+    result = process_items(items, 7);
+    if (result > sizeof(struct header)) { /* wrong comparison */ }
+}
+```
+In this example, `process_items` returns the number of items processed or `-1` on error. Most call sites correctly compare the return value with a numeric literal. However, one call site mistakenly compares it with `sizeof(struct header)`, which is inconsistent with how the return value is used everywhere else.
+
+
+## References
+* [CWE-252: Unchecked Return Value](https://cwe.mitre.org/data/definitions/252.html)
+* [CWE-253: Incorrect Check of Function Return Value](https://cwe.mitre.org/data/definitions/253.html)
diff --git a/cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.c b/cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.c
new file mode 100644
index 0000000..d65cf09
--- /dev/null
+++ b/cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.c
@@ -0,0 +1,38 @@
+struct header {
+    int type;
+    int length;
+};
+
+// Returns number of items processed, or -1 on error
+int process_items(int *items, int count) {
+    int processed = 0;
+    for (int i = 0; i < count; i++) {
+        if (items[i] < 0)
+            return -1;
+        processed++;
+    }
+    return processed;
+}
+
+void example() {
+    int items[10];
+    int result;
+
+    // Typical: return value compared with a numeric literal
+    result = process_items(items, 10);
+    if (result > 0) { /* success */ }
+
+    result = process_items(items, 5);
+    if (result != -1) { /* no error */ }
+
+    result = process_items(items, 3);
+    if (result == 0) { /* empty */ }
+
+    result = process_items(items, 8);
+    if (result >= 1) { /* at least one */ }
+
+    // BAD: comparing with sizeof instead of a numeric literal.
+    // This is inconsistent with all other call sites and likely a bug.
+    result = process_items(items, 7);
+    if (result > sizeof(struct header)) { /* wrong comparison */ }
+}
diff --git a/cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.qhelp b/cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.qhelp
index fc60a6c..cd867cb 100644
--- a/cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.qhelp
+++ b/cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.qhelp
@@ -1,27 +1,62 @@
 <!DOCTYPE qhelp PUBLIC
-"-//Semmle//qhelp//EN"
-"qhelp.dtd">
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
 <qhelp>
 <overview>
-<p>
-    Description of the vulnerability.
-</p>
+  <p>
+  When a function's return value is checked in <code>if</code> statements across multiple call sites,
+  the comparisons typically fall into a consistent pattern (e.g., compared against a numeric literal,
+  <code>NULL</code>, or <code>sizeof</code>). If a small number of call sites compare the return value
+  in a different way than the majority, these inconsistent comparisons may indicate a bug.
+  </p>
+
+  <p>
+  The query categorizes each comparison into one of the following categories:
+  </p>
+  <ul>
+    <li>Numeric literal (e.g., <code>ret != -1</code>)</li>
+    <li>Boolean (e.g., <code>ret == true</code>)</li>
+    <li>Null pointer (e.g., <code>ret != NULL</code>)</li>
+    <li>Pointer</li>
+    <li><code>sizeof</code> expression (e.g., <code>ret > sizeof(buf)</code>)</li>
+    <li>Another function's return value (e.g., <code>ret != other_func()</code>)</li>
+    <li>Passed as argument to another function (e.g., <code>if (check(ret))</code>)</li>
+    <li>Arithmetic expression</li>
+  </ul>
+
+  <p>
+  When at least 75% of a function's return value comparisons fall into one category,
+  the remaining comparisons in a different category are flagged as potentially incorrect.
+  </p>
 </overview>
+
 <recommendation>
 <p>
-    Do XYZ.
+Review each flagged call site and verify that the comparison matches the function's return value semantics.
+If the function returns an error code or count, all call sites should compare it consistently.
+Fix any comparisons that use the wrong type of operand (e.g., comparing an integer return value against
+<code>sizeof</code> when all other sites compare against a numeric literal).
 </p>
 </recommendation>
+
 <example>
 <p>
-    The following example shows ...:
+In this example, <code>process_items</code> returns the number of items processed or <code>-1</code>
+on error. Most call sites correctly compare the return value with a numeric literal. However, one
+call site mistakenly compares it with <code>sizeof(struct header)</code>, which is inconsistent
+with how the return value is used everywhere else.
 </p>
-<sample src="SomeFileInCWD.cpp"/>
+
+<sample src="InconsistentReturnValueHandling.c" />
 </example>
+
 <references>
 <li>
-    XYZ:
-    <a href="https://trailofbits.com">XYZ</a>
+  <a href="https://cwe.mitre.org/data/definitions/252.html">CWE-252: Unchecked Return Value</a>
+</li>
+<li>
+  <a href="https://cwe.mitre.org/data/definitions/253.html">CWE-253: Incorrect Check of Function Return Value</a>
 </li>
 </references>
+
 </qhelp>
diff --git a/cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.ql b/cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.ql
index 9df0cef..c1202e3 100644
--- a/cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.ql
+++ b/cpp/src/security/InconsistentReturnValueHandling/InconsistentReturnValueHandling.ql
@@ -1,10 +1,7 @@
 /**
  * @name Inconsistent handling of return values from a specific function
- * @description If a function's return value is used in `if` statements,
- *              and in a few statements the value is compared differently than it is usually,
- *              then this rare comparisons are bugs indicators.
- *              The query categorizes uses of return values into a few categories
- *              (cmp with int, bool, nullptr, sizeof, another function, ...)
+ * @description Detects functions whose return values are compared
+ *              inconsistently across call sites, which may indicate bugs.
  * @kind problem
  * @problem.severity warning
  * @precision medium

From be9113e4634fe38241973a500b577825c86ec0ea Mon Sep 17 00:00:00 2001
From: GrosQuildu <e2.8a.95@gmail.com>
Date: Mon, 8 Jul 2024 15:25:35 +0200
Subject: [PATCH 10/10] fix string stubs

---
 cpp/test/include/libc/string_stubs.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/cpp/test/include/libc/string_stubs.h b/cpp/test/include/libc/string_stubs.h
index aa70996..d589139 100644
--- a/cpp/test/include/libc/string_stubs.h
+++ b/cpp/test/include/libc/string_stubs.h
@@ -14,6 +14,7 @@ extern "C" {
 #ifndef __cplusplus
 typedef int wchar_t;
 #endif
+
 extern void *memcpy(void *dst, const void *src, unsigned long n);
 extern char* strcpy_s(char* dst, int max_amount, char* src);
 extern int _mbsncat(char* dst, char* src, int count);