VNX-1025: Comparison Using Wrong Factors

A comparison that examines the wrong property of its operands, so it can succeed or fail regardless of the values the author meant to compare. Reported: in Java, == or != between object references where a value comparison was meant (a String literal operand, a String-returning call such as getParameter/getHeader/trim/toString, a variable declared String a few dozen lines above, or two variables declared Integer/Long/BigDecimal and other boxed types), and classes compared by name (getClass().getName()/getSimpleName() with equals or ==); in Python, `is`/`is not` against a str, bytes, number, list, dict or empty-tuple literal in an if/elif/while/assert/return statement (also the first line of a condition that wraps) or an assignment, and type(x).__name__ or x.__class__.__name__ compared with ==/!=; in JavaScript/TypeScript, ==/=== against a fresh [] or {} literal (always a different object), x.constructor.name compared with ==/===, and typeof compared with a string typeof never returns ('array', 'null', 'date', ...); in Go, err.Error() text compared with ==/!= with a non-empty literal or another error's text instead of errors.Is (directly or through a variable assigned err.Error() a few lines above), and switch err.Error() {, and reflect.TypeOf(x).Name()/String() or fmt.Sprintf("%T") compared with ==; in PHP, loose ==/!= with an md5/sha1/hash/hash_hmac/crypt result, directly or through a variable assigned one a few lines above ("0e..." hashes compare as numbers), strcmp/strcasecmp(...) == 0 and !strcmp(...) (NULL == 0 when an argument is an array), and get_class() compared with a string literal. Not reported: err.Error() != "" (an emptiness test), comparisons with null/None/True/False, enums, primitives and char literals, equals()/Objects.equals/MessageDigest.isEqual/hash_equals/password_verify/errors.Is/instanceof/isinstance, strict === in PHP, a.intern() == b.intern(), get_class() against ::class or __CLASS__, timing-unsafe comparison of secrets (a different weakness), test code, comments and the pattern inside string literals. Two comparisons are reported by a more precise rule, so the line is reported once: a Java String compared with a string literal by == or != by VNX-597 (String compared with == instead of equals()), and a PHP loose comparison VNX-PHP-010 reports (a hash compared with ==, strcmp() == 0 on request data: PHP type juggling).

Field	Value
Rule ID	`VNX-1025`
Severity	Low
SARIF level	`note`
Kind	`sast`
Languages	Java, Python, JavaScript / TypeScript, Go, PHP
CWE	CWE-1025
CAPEC	CAPEC-194
ATT&CK	T1190
CWSS vector	`CWSS:1.0/TI:L/AP:RU/AL:S/IC:N/FC:LT/RP:RU/RL:S/AV:L/AS:N/IN:A/SC:A/BI:L/DI:H/EX:L/EC:N/P:L`
Tags	`comparison` `wrong-factors` `equality` `security-check` `cwe-1025`
Source	`internal/sast/rules/vnx-cwe1025-auto.rego`

Overview

VNX-1025 reports CWE-1025: Comparison Using Wrong Factors: a comparison that examines the wrong property of its operands, so it can succeed or fail regardless of the values the author meant to compare.

What is reported:

Java: == or != between object references where a value comparison was meant. That covers a String literal operand, a String-returning call (getParameter, getHeader, trim, toString …), a variable declared String in the enclosing method, and two variables declared Integer/Long/BigDecimal or another boxed type. Classes compared by name (getClass().getName()/getSimpleName() with equals or ==) are also reported.
Python: is/is not against a str, bytes, number, list, dict or empty-tuple literal in an if/elif/while/assert/return, and type(x).__name__ or x.__class__.__name__ compared with ==/!=.
JavaScript/TypeScript: ==/=== against a fresh [] or {} (always a different object), x.constructor.name compared with ==/===, and typeof compared with a string typeof never returns ('array', 'null', 'date' …).
Go: err.Error() text compared with ==/!= instead of errors.Is (directly, or through a variable assigned err.Error() a few lines above), and reflect.TypeOf(x).Name()/String() or fmt.Sprintf("%T") compared with ==.
PHP: loose ==/!= with an md5/sha1/hash/hash_hmac/crypt result ("0e..." hashes compare as numbers), strcmp/strcasecmp(...) == 0 (NULL == 0 when an argument is an array), and get_class() compared with a string literal.

Not reported: comparisons with null/None/True/False, enums, primitives and char literals; equals(), Objects.equals, MessageDigest.isEqual, hash_equals, password_verify, errors.Is, instanceof and isinstance; strict === in PHP; a.intern() == b.intern(); get_class() against ::class or __CLASS__; timing-unsafe comparison of secrets (a different weakness); test code, comments and the pattern inside string literals.

Two comparisons are reported by a more precise rule, so the same line is not reported twice: a Java String compared with a string literal (referer != "NULL") by VNX-597, and a PHP loose comparison that decides an authentication or integrity check ($token == md5(...), strcmp($_POST['password'], $stored) == 0) by VNX-PHP-010.

Why This Matters

These comparisons look right and pass simple tests, then decide security checks wrongly. In Java, request.getParameter("role") == "admin" compares object identity and is false for every request, while two interned constants compare equal by accident. PHP’s md5($token) == $hash treats "0e1234..." hashes as the number zero, so unrelated tokens match. strcmp($_POST['password'], $stored) == 0 passes when the password is sent as an array. A class or error compared by name is fooled by any class or message that reuses the name.

What Gets Flagged

return request.getParameter("role") == expectedRole;                        // FLAGGED
if (plugin.getClass().getName().equals("com.acme.plugins.TrustedPlugin")) { // FLAGGED

if role is "admin":                                                          # FLAGGED
return type(user).__name__ == "Superuser"                                    # FLAGGED

if (typeof filter === 'array') {                                             // FLAGGED
if (user.roles === []) {                                                     // FLAGGED

if err.Error() == "connection reset by peer" {                               // FLAGGED

if (md5($token) == '5d41402abc4b2a76b9719d911017c592') {                     // FLAGGED
if (get_class($handler) == 'AdminHandler') {                                 // FLAGGED

What Is Not Flagged

return "admin".equals(request.getParameter("role"));

if (hash_equals($user['reset_hash'], md5($token))) {
// reported by VNX-PHP-010: if (md5($token) == $user['reset_hash']) {

Remediation

Java: compare values with equals()/Objects.equals, and types with instanceof or getClass() == Other.class.
Python: use == for values and isinstance for types.
JavaScript: use Array.isArray, .length, instanceof or === null.
Go: use errors.Is/errors.As and type switches.
PHP: use ===, hash_equals and password_verify.

References

Suppressing this rule

Put a nosec comment on the line, or on line 1 to cover the whole file. Use the comment marker of the file’s language (#, //, -- or ;):

# nosec vnx-1025
// nosec vnx-1025

Or record an ignore rule: vulnetix ignore add --rule vnx-1025 --reason "...". See Suppressing findings.