C/C++: Detect ambiguous assignment of comparison results

cpp/ql/integration-tests/query-suite/cpp-code-quality-extended.qls.expected

@@ -1 +1 @@
-
+ql/cpp/ql/src/Likely Bugs/Likely Typos/AmbiguousAssignmentOfComparison.ql

cpp/ql/integration-tests/query-suite/cpp-code-quality.qls.expected

@@ -1 +1 @@
-
+ql/cpp/ql/src/Likely Bugs/Likely Typos/AmbiguousAssignmentOfComparison.ql

cpp/ql/integration-tests/query-suite/cpp-security-and-quality.qls.expected

@@ -65,6 +65,7 @@ ql/cpp/ql/src/Likely Bugs/InconsistentCheckReturnNull.ql
 ql/cpp/ql/src/Likely Bugs/Leap Year/Adding365DaysPerYear.ql
 ql/cpp/ql/src/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification.ql
 ql/cpp/ql/src/Likely Bugs/Leap Year/UncheckedReturnValueForTimeFunctions.ql
+ql/cpp/ql/src/Likely Bugs/Likely Typos/AmbiguousAssignmentOfComparison.ql
 ql/cpp/ql/src/Likely Bugs/Likely Typos/AssignWhereCompareMeant.ql
 ql/cpp/ql/src/Likely Bugs/Likely Typos/CompareWhereAssignMeant.ql
 ql/cpp/ql/src/Likely Bugs/Likely Typos/DubiousNullCheck.ql

cpp/ql/src/Likely Bugs/Likely Typos/AmbiguousAssignmentOfComparison.cpp

@@ -0,0 +1,12 @@
+int read_status();
+
+int check_status() {
+  int status;
+  if ((status = read_status() < 0)) // BAD: assigns the comparison result.
+    return status;
+
+  if ((status = read_status()) < 0) // GOOD: assigns first, then compares.
+    return status;
+
+  return 0;
+}

cpp/ql/src/Likely Bugs/Likely Typos/AmbiguousAssignmentOfComparison.qhelp

@@ -0,0 +1,31 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>Assignment operators have lower precedence than comparison operators. For example,
+<code>status = read_status() &lt; 0</code> assigns the comparison result (zero or one) to
+<code>status</code>. This can be unintended when the programmer meant to assign the return value
+first and then compare it with zero.</p>
+</overview>
+
+<recommendation>
+<p>Use parentheses to make the intended operation order explicit. To assign first and compare the
+assigned value, parenthesize the assignment. To intentionally assign the comparison result,
+parenthesize the comparison. An explicit cast around the comparison also makes that order clear.</p>
+</recommendation>
+
+<example>
+<p>In the first condition, <code>status</code> receives either zero or one instead of the value
+returned by <code>read_status</code>. The second condition explicitly performs the assignment
+before the comparison.</p>
+<sample src="AmbiguousAssignmentOfComparison.cpp" />
+</example>
+
+<references>
+<li>SEI CERT C Coding Standard: <a href="https://wiki.sei.cmu.edu/confluence/display/c/EXP00-C.+Use+parentheses+for+precedence+of+operation">EXP00-C. Use parentheses for precedence of operation</a>.</li>
+<li>C++ reference: <a href="https://en.cppreference.com/w/cpp/language/operator_precedence.html">Operator precedence</a>.</li>
+</references>
+
+</qhelp>

cpp/ql/src/Likely Bugs/Likely Typos/AmbiguousAssignmentOfComparison.ql

@@ -0,0 +1,60 @@
+/**
+ * @name Ambiguous assignment of comparison in condition
+ * @description Assigning the result of an unparenthesized comparison in a condition may indicate
+ *              that the assignment and comparison are grouped incorrectly.
+ * @kind problem
+ * @problem.severity warning
+ * @precision high
+ * @id cpp/ambiguous-assignment-of-comparison
+ * @tags quality
+ *       reliability
+ *       correctness
+ *       external/cwe/cwe-783
+ */
+
+import cpp
+
+/** Gets a condition that controls branching. */
+private Expr getACondition() {
+  result = any(IfStmt s).getCondition()
+  or
+  result = any(WhileStmt s).getCondition()
+  or
+  result = any(DoStmt s).getCondition()
+  or
+  result = any(ForStmt s).getCondition()
+  or
+  result = any(ConditionalExpr e).getCondition()
+}
+
+/**
+ * Holds if `assignment` occurs within a condition that controls branching.
+ *
+ * This includes nested expressions, such as function arguments and either operand of a comma
+ * expression, because the ambiguous syntax still occurs within the condition.
+ */
+private predicate occursInCondition(Assignment assignment) {
+  assignment.getParent*() = getACondition()
+}
+
+/**
+ * Holds if `comparison` is explicitly grouped using parentheses or an explicit cast.
+ */
+private predicate isExplicitlyGrouped(ComparisonOperation comparison) {
+  comparison.isParenthesised()
+  or
+  exists(Cast cast | cast = comparison.getConversion+() and not cast.isImplicit())
+}
+
+from Assignment assignment, ComparisonOperation comparison
+where
+  assignment.getRValue() = comparison and
+  not isExplicitlyGrouped(comparison) and
+  occursInCondition(assignment) and
+  // Assigning a comparison result to a Boolean is normally intentional.
+  not assignment.getLValue().getUnspecifiedType() instanceof BoolType and
+  not assignment.isUnevaluated() and
+  not assignment.isFromUninstantiatedTemplate(_)
+select assignment,
+  "The '" + assignment.getOperator() +
+    "' operation assigns the result of an unparenthesized comparison used in a condition."

cpp/ql/src/change-notes/2026-08-13-ambiguous-assignment-of-comparison.md

@@ -0,0 +1,5 @@
+---
+category: newQuery
+---
+* Added a new query, `cpp/ambiguous-assignment-of-comparison`, to detect assignments of
+  unparenthesized comparison results in conditions.

cpp/ql/test/query-tests/Likely Bugs/Likely Typos/AmbiguousAssignmentOfComparison/AmbiguousAssignmentOfComparison.expected

@@ -0,0 +1,24 @@
+| test.c:6:8:6:31 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison used in a condition. |
+| test.c:13:30:13:54 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison used in a condition. |
+| test.c:20:8:20:33 | ... /= ... | The '/=' operation assigns the result of an unparenthesized comparison used in a condition. |
+| test.c:27:8:27:33 | ... %= ... | The '%=' operation assigns the result of an unparenthesized comparison used in a condition. |
+| test.c:34:8:34:32 | ... \|= ... | The '\|=' operation assigns the result of an unparenthesized comparison used in a condition. |
+| test.c:41:8:41:33 | ... >>= ... | The '>>=' operation assigns the result of an unparenthesized comparison used in a condition. |
+| test.c:51:3:51:26 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison used in a condition. |
+| test.cpp:8:8:8:30 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison used in a condition. |
+| test.cpp:15:11:15:34 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison used in a condition. |
+| test.cpp:22:7:22:29 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison used in a condition. |
+| test.cpp:29:11:29:40 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison used in a condition. |
+| test.cpp:36:8:36:30 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison used in a condition. |
+| test.cpp:43:11:43:33 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison used in a condition. |
+| test.cpp:48:8:48:33 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison used in a condition. |
+| test.cpp:55:8:55:32 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison used in a condition. |
+| test.cpp:64:13:64:35 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison used in a condition. |
+| test.cpp:70:29:70:51 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison used in a condition. |
+| test.cpp:77:8:77:30 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison used in a condition. |
+| test.cpp:84:8:84:38 | ... <<= ... | The '<<=' operation assigns the result of an unparenthesized comparison used in a condition. |
+| test.cpp:91:9:91:31 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison used in a condition. |
+| test.cpp:101:3:101:20 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison used in a condition. |
+| test.cpp:253:8:253:24 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison used in a condition. |
+| test.cpp:280:16:280:38 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison used in a condition. |
+| test.cpp:287:8:287:30 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison used in a condition. |

cpp/ql/test/query-tests/Likely Bugs/Likely Typos/AmbiguousAssignmentOfComparison/AmbiguousAssignmentOfComparison.qlref

@@ -0,0 +1,2 @@
+query: Likely Bugs/Likely Typos/AmbiguousAssignmentOfComparison.ql
+postprocess: utils/test/InlineExpectationsTestQuery.ql

cpp/ql/test/query-tests/Likely Bugs/Likely Typos/AmbiguousAssignmentOfComparison/test.c

@@ -0,0 +1,91 @@
+int read_value(void);
+int read_other_value(void);
+
+int c_direct_condition(void) {
+  int value;
+  if ((value = read_value() < 0)) // $ Alert // BAD
+    return value;
+  return 0;
+}
+
+int c_logical_condition(void) {
+  int value;
+  if (read_other_value() && (value = read_value() >= 0)) // $ Alert // BAD
+    return value;
+  return 0;
+}
+
+int c_compound_divide(void) {
+  int value = 8;
+  if ((value /= read_value() != 0)) // $ Alert // BAD
+    return value;
+  return 0;
+}
+
+int c_compound_remainder(void) {
+  int value = 8;
+  if ((value %= read_value() != 0)) // $ Alert // BAD
+    return value;
+  return 0;
+}
+
+int c_compound_bitwise_or(void) {
+  int value = 0;
+  if ((value |= read_value() > 0)) // $ Alert // BAD
+    return value;
+  return 0;
+}
+
+int c_compound_right_shift(void) {
+  int value = 8;
+  if ((value >>= read_value() > 0)) // $ Alert // BAD
+    return value;
+  return 0;
+}
+
+#define C_AMBIGUOUS_CHECK(VALUE) \
+  if (((VALUE) = read_value() < 0)) return (VALUE)
+
+int c_macro_condition(void) {
+  int value;
+  C_AMBIGUOUS_CHECK(value); // $ Alert // BAD
+  return 0;
+}
+
+int c_explicit_assign_then_compare(void) {
+  int value;
+  if ((value = read_value()) < 0) // GOOD
+    return value;
+  return 0;
+}
+
+int c_explicit_compare_then_assign(void) {
+  int value;
+  if ((value = (read_value() < 0))) // GOOD
+    return value;
+  return 0;
+}
+
+int c_explicit_cast_of_comparison(void) {
+  int value;
+  if ((value = (int)(read_value() < 0))) // GOOD: The cast explicitly groups the comparison.
+    return value;
+  return 0;
+}
+
+int c_boolean_result_assignment(void) {
+  _Bool negative;
+  if ((negative = read_value() < 0)) // GOOD: Assigning a comparison result to a Boolean is natural.
+    return negative;
+  return 0;
+}
+
+int c_switch_expression(void) {
+  int value;
+  switch (value = read_value() < 0) { // GOOD: This query only covers branching conditions.
+  case 0:
+    return value;
+  default:
+    return 0;
+  }
+}