C/C++: Detect ambiguous assignment of comparison results by theinfosecguy · Pull Request #22336 · github/codeql

theinfosecguy · 2026-08-13T12:27:10Z

Adds cpp/ambiguous-assignment-of-comparison to flag ambiguous assignments when the assignment result is used as a truth value, such as:

if ((status = read_status() < 0))

The query distinguishes this from explicitly grouped assign-then-compare and compare-then-assign expressions. It includes C and C++ tests, query help, and query-suite integration.

Local targeted and neighboring tests pass. The motivating regression is detected, and a run against git/git produced no alerts.

Fixes #22286

ryao

While I am happy you did the work for me to get this into a PR and made what appear to be improvements, would you add an Original-patch-by: to the commit message to credit my prior work?

Also, have you run your variant against any major corpora (e.g. Linux, curl, OpenZFS) to verify the lack of FPs in production code, like I did with the original version? If it helps:

https://docs.github.com/en/code-security/how-tos/find-and-fix-code-vulnerabilities/scan-from-the-command-line/download-databases

ryao · 2026-08-15T01:04:39Z

+  not isExplicitlyGrouped(comparison) and
+  occursInCondition(assignment) and
+  // Assigning a comparison result to a Boolean is normally intentional.
+  not assignment.getLValue().getUnspecifiedType() instanceof BoolType and


While this is normally intentional, I believe not isExplicitlyGrouped(comparison) precludes this.

Why is that? It seems like the types of the expressions and the bracketing are mostly independent concerns.

My reasoning was that if it is a comparison operation, then it is implicitly bool. Maybe with C++ operator overloading, this might not be the case. I had only been imagining C when I made this remark.

geoffw0

I've reviewed the code and docs from a technical perspective (I haven't looked through all the test cases yet). This looks quite promising. I've also done a mass (MRVA) run and found quite a high rate of true positive results!

Original-patch-by: Richard Yao <richard@ryao.dev>

github-actions · 2026-08-21T12:00:49Z

QHelp previews:

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

Ambiguous assignment of comparison used as truth value

Assignment operators have lower precedence than comparison operators. For example, status = read_status() < 0 assigns the comparison result (zero or one) to status. This can be unintended when the programmer meant to assign the return value first and then compare it with zero.

Recommendation

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.

Example

In the first condition, status receives either zero or one instead of the value returned by read_status. The second condition explicitly performs the assignment before the comparison.

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;
}

References

SEI CERT C Coding Standard: EXP00-C. Use parentheses for precedence of operation.
C++ reference: Operator precedence.
Common Weakness Enumeration: CWE-783.

geoffw0

Nice thorough tests. I've nit-picked a couple of cases but they're not important ones, and you might have more insight into them than I have, from knowing the cases that motivated this query.

I've gained a lot of confidence in this query from reviewing it - just a few small things to discuss and decide if we want to make any changes or not. 👍

geoffw0 · 2026-08-24T17:57:16Z

Thank you for making the changes. The CodeQL looks good to me, as do the tests and MRVA (bulk testing) results. I think I said it before, but I'm seeing lots of true positive results in real world data. :)

I've requested a quick review of the .qhelp from the docs team, we should be able to merge soon after that.

jclement136 · 2026-08-28T16:01:16Z

👋 Hello from docs! Just confirming that I've added this to our review board for you to make sure it gets attention and a review soon. If you still need any assistance just reach out on Slack via the #docs-content channel.

mchammer01

@theinfosecguy 👋🏻 - Hi from Technical Content (used to be Docs). Overall, this PR looks good, so I’m approving it ✨
I’ve left a few suggestions to make the query help, metadata, and change note easier to read. It would be great if some of these could be applied or tweaked before merging, but I don’t think they need to block the PR. Thanks for adding clear examples and references 🙏

mchammer01 · 2026-09-01T08:27:24Z

@@ -0,0 +1,72 @@
+/**
+ * @name Ambiguous assignment of comparison used as truth value


The query name is grammatically unclear, suggesting the following:

Suggested change

* @name Ambiguous assignment of comparison used as truth value

* @name Ambiguous assignment of comparison result used as truth value

mchammer01 · 2026-09-01T08:29:21Z

+ * @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.


The description is complex and doesn’t closely follow the preferred “Syntax X causes behavior Y” pattern.
Suggesting something like: (let me know what you think!)

Suggested change

* @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.

* @description Assigning the result of an unparenthesized comparison when the assignment is used

* as a truth value can obscure the intended grouping of the operations.

mchammer01 · 2026-09-01T08:31:48Z

+</overview>
+
+<recommendation>
+<p>Use parentheses to make the intended operation order explicit. To assign first and compare the


Nit: use more natural language:

Suggested change

<p>Use parentheses to make the intended operation order explicit. To assign first and compare the

<p>Use parentheses to make the intended order of operations explicit. To assign first and compare the

mchammer01 · 2026-09-01T08:34:33Z

+
+<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


Change read_status to read_status() when referring to the function call

Suggested change

returned by <code>read_status</code>. The second condition explicitly performs the assignment

returned by <code>read_status()</code>. The second condition explicitly performs the assignment

mchammer01 · 2026-09-01T08:43:58Z

+* 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.


Nit: Whilst this is technically accurate, the wording is dense. “Assignments of unparenthesized comparison results” takes a moment to parse. What about:

Suggested change

* 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.

* Added a new query, `cpp/ambiguous-assignment-of-comparison`, to detect potentially

ambiguous expressions where a comparison result is assigned to a variable and the

assignment is used as a truth value.

theinfosecguy requested a review from a team as a code owner August 13, 2026 12:27

github-actions Bot added C++ documentation labels Aug 13, 2026

ryao reviewed Aug 15, 2026

View reviewed changes

andersfugmann requested a review from geoffw0 August 19, 2026 11:28

andersfugmann assigned geoffw0 Aug 19, 2026

geoffw0 mentioned this pull request Aug 20, 2026

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

Closed

geoffw0 reviewed Aug 20, 2026

View reviewed changes

Comment thread cpp/ql/src/Likely Bugs/Likely Typos/AmbiguousAssignmentOfComparison.cpp Outdated

Comment thread cpp/ql/src/Likely Bugs/Likely Typos/AmbiguousAssignmentOfComparison.ql Outdated

Comment thread cpp/ql/src/Likely Bugs/Likely Typos/AmbiguousAssignmentOfComparison.ql Outdated

geoffw0 reviewed Aug 20, 2026

View reviewed changes

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

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

8f0ea61

Original-patch-by: Richard Yao <richard@ryao.dev>

theinfosecguy force-pushed the cpp-ambiguous-assignment-condition branch from 7784270 to 8f0ea61 Compare August 21, 2026 07:01

geoffw0 reviewed Aug 21, 2026

View reviewed changes

Comment thread cpp/ql/test/query-tests/Likely Bugs/Likely Typos/AmbiguousAssignmentOfComparison/test.cpp Outdated

Comment thread cpp/ql/test/query-tests/Likely Bugs/Likely Typos/AmbiguousAssignmentOfComparison/test.cpp Outdated

Refine truth-value detection

3715b86

geoffw0 added the ready-for-doc-review This PR requires and is ready for review from the GitHub docs team. label Aug 24, 2026

Merge branch 'main' into cpp-ambiguous-assignment-condition

7917e14

mchammer01 self-requested a review September 1, 2026 07:09

mchammer01 approved these changes Sep 1, 2026

View reviewed changes

		@@ -0,0 +1,72 @@
		/**
		* @name Ambiguous assignment of comparison used as truth value

	<p>Use parentheses to make the intended operation order explicit. To assign first and compare the
	<p>Use parentheses to make the intended order of operations explicit. To assign first and compare the

	returned by <code>read_status</code>. The second condition explicitly performs the assignment
	returned by <code>read_status()</code>. The second condition explicitly performs the assignment

		* 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.

Uh oh!

Conversation

theinfosecguy commented Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ryao left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

geoffw0 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Ambiguous assignment of comparison used as truth value

Recommendation

Example

References

Uh oh!

geoffw0 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

geoffw0 commented Aug 24, 2026

Uh oh!

jclement136 commented Aug 28, 2026

Uh oh!

mchammer01 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

theinfosecguy commented Aug 13, 2026 •

edited

Loading

github-actions Bot commented Aug 21, 2026 •

edited

Loading