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,72 @@
+/**
+ * @name Ambiguous assignment of comparison used as truth value
+ * @description Assigning the result of an unparenthesized comparison when the assignment is used
+ *              as a truth value 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
+
+/** Holds if the value of `expression` is directly used as a truth value. */
+private predicate isDirectlyUsedAsTruthValue(Expr expression) {
+  expression.isCondition()
+  or
+  expression = any(UnaryLogicalOperation operation).getAnOperand()
+  or
+  expression = any(BinaryLogicalOperation operation).getAnOperand()
+}
+
+/**
+ * Holds if the value of `expression` is used as a truth value, possibly after contributing to a
+ * comma, conditional, or comparison expression.
+ */
+private predicate isUsedAsTruthValue(Expr expression) {
+  isDirectlyUsedAsTruthValue(expression)
+  or
+  exists(CommaExpr comma |
+    expression = comma.getRightOperand() and
+    isUsedAsTruthValue(comma)
+  )
+  or
+  exists(ConditionalExpr conditional |
+    expression = [conditional.getThen(), conditional.getElse()] and
+    isUsedAsTruthValue(conditional)
+  )
+  or
+  exists(ComparisonOperation comparison |
+    expression = comparison.getAnOperand() and
+    isUsedAsTruthValue(comparison)
+  )
+}
+
+/**
+ * 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
+  isUsedAsTruthValue(assignment) and
+  // A Boolean lvalue makes assigning the comparison result type-appropriate and 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, and its result is used as " +
+    "a truth value."

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 when the assignment is used as a truth value.

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

@@ -0,0 +1,36 @@
+| test.c:6:8:6:31 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.c:13:30:13:54 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.c:20:8:20:33 | ... /= ... | The '/=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.c:27:8:27:33 | ... %= ... | The '%=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.c:34:8:34:32 | ... \|= ... | The '\|=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.c:41:8:41:33 | ... >>= ... | The '>>=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.c:51:3:51:26 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.c:102:28:102:51 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.c:109:15:109:38 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.c:116:49:116:72 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.c:130:11:130:34 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:8:8:8:30 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:15:11:15:34 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:22:7:22:29 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:29:11:29:40 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:36:8:36:30 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:43:11:43:33 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:48:8:48:33 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:55:8:55:32 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:64:13:64:35 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:70:29:70:51 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:77:8:77:30 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:84:8:84:38 | ... <<= ... | The '<<=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:91:9:91:31 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:101:3:101:20 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:253:8:253:24 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:294:27:294:49 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:301:27:301:49 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:308:9:308:31 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:315:15:315:37 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:322:23:322:45 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:329:47:329:69 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:343:47:343:69 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:350:11:350:33 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:355:12:355:34 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |
+| test.cpp:360:14:360:36 | ... = ... | The '=' operation assigns the result of an unparenthesized comparison, and its result is used as a truth value. |

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,136 @@
+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: The switch operand is not used as a truth value.
+  case 0:
+    return value;
+  default:
+    return 0;
+  }
+}
+
+int c_discarded_assignment_in_comma_expression(void) {
+  int value;
+  if ((value = read_value() < 0, read_other_value())) // GOOD: The assignment result is discarded.
+    return value;
+  return 0;
+}
+
+int c_truth_valued_assignment_in_comma_expression(void) {
+  int value;
+  if ((read_other_value(), value = read_value() < 0)) // $ Alert // BAD
+    return value;
+  return 0;
+}
+
+int c_truth_valued_conditional_then_arm(int flag) {
+  int value;
+  if (flag ? (value = read_value() < 0) : 0) // $ Alert // BAD
+    return value;
+  return 0;
+}
+
+int c_nested_truth_valued_comma_expression(void) {
+  int value;
+  if ((read_other_value(), (read_other_value(), value = read_value() < 0))) // $ Alert // BAD
+    return value;
+  return 0;
+}
+
+int c_nested_discarded_comma_expression(void) {
+  int value;
+  if (((read_other_value(), value = read_value() < 0), read_other_value())) // GOOD: Discarded.
+    return value;
+  return 0;
+}
+
+int c_logical_value_outside_branch(void) {
+  int value;
+  return (value = read_value() < 0) && read_other_value(); // $ Alert // BAD
+}
+
+int c_returned_assignment(void) {
+  int value;
+  return value = read_value() < 0; // GOOD: The assignment result is not used as a truth value.
+}