Comparison Using Wrong Factors

Description

Comparison Using Wrong Factors occurs when a product performs a comparison but uses the wrong attributes, factors, or criteria for the comparison context. Unlike incomplete comparisons (which miss factors), this weakness involves actively using incorrect factors that do not properly represent what should be compared. This can lead to false positive or false negative results in security decisions, causing either unnecessary denials of valid requests or allowing invalid requests through.

Risk

This vulnerability can compromise security decisions in subtle ways. Comparing user IDs when session IDs should be compared may allow session hijacking. Using creation timestamps instead of modification timestamps for cache validation can serve stale content. Comparing file paths instead of canonical paths enables path traversal bypasses. Checking only username without tenant ID in multi-tenant systems allows cross-tenant access. The risk is high because the comparison logic appears functional but fundamentally checks the wrong thing.

Solution

Carefully analyze what factors are relevant for each comparison context. Document the comparison criteria and rationale. For authentication, compare credentials against stored values for the same identity. For authorization, compare requested permissions against granted permissions for the same resource. For data validation, compare canonical/normalized forms rather than raw inputs. Use code review to verify correct factors are being compared. Consider using dedicated comparison functions that enforce correct factor usage. Write tests that verify comparisons fail when they should fail.

Common Consequences

Impact	Details
Access Control	Scope: Access Control Bypass Protection Mechanism - Using wrong factors in access control checks may grant unauthorized access.
Integrity	Scope: Integrity Modify Application Data - Incorrect comparisons can lead to wrong data being selected, modified, or deleted.
Confidentiality	Scope: Confidentiality Read Application Data - Wrong comparison factors may expose data to unauthorized parties.

Example Code

Vulnerable Code

// Vulnerable: Comparing wrong factors for session validation
public class VulnerableSessionValidator {

    public boolean validateSession(HttpServletRequest request) {
        String sessionToken = request.getHeader("X-Session-Token");
        User user = getUserFromToken(sessionToken);

        // Get stored session
        Session storedSession = sessionStore.get(sessionToken);

        if (storedSession == null) {
            return false;
        }

        // Vulnerable: Comparing user ID instead of session token!
        // This allows session hijacking if user ID matches
        return user.getId().equals(storedSession.getUserId());

        // Should compare: sessionToken.equals(storedSession.getToken())
    }
}

# Vulnerable: Comparing raw path instead of canonical path
import os

def vulnerable_check_file_access(requested_path, allowed_directory):
    # Vulnerable: Comparing raw paths
    # Attacker can use "../" to escape allowed directory

    if requested_path.startswith(allowed_directory):
        return True  # Allow access

    return False

# Attack: requested_path = "/var/www/allowed/../../../etc/passwd"
# This starts with "/var/www/allowed" but escapes it!

// Vulnerable: Using wrong timestamp for cache validation
class VulnerableCache {

    isValid(cachedItem, sourceItem) {
        // Vulnerable: Comparing creation time instead of modification time
        // Stale data will be served if modified after creation

        return cachedItem.createdAt >= sourceItem.createdAt;

        // Should compare: cachedItem.cachedAt >= sourceItem.modifiedAt
    }

    get(key) {
        const cached = this.cache.get(key);
        const source = this.source.get(key);

        if (cached && this.isValid(cached, source)) {
            return cached.data;  // May return stale data!
        }

        return this.fetchAndCache(key);
    }
}

<?php
// Vulnerable: Multi-tenant query without tenant comparison
class VulnerableDocumentService {

    public function getDocument($documentId, $userId) {
        // Vulnerable: Only checking document ID and user ID
        // Missing tenant ID comparison!

        $sql = "SELECT * FROM documents
                WHERE id = ? AND owner_id = ?";

        $result = $this->db->query($sql, [$documentId, $userId]);

        // User from Tenant A can access documents if they have
        // the same user ID as document owner in Tenant B
        return $result;
    }
}

// Vulnerable: Comparing string representation instead of numeric value
func vulnerableCheckBalance(requested float64, available string) bool {
    // Vulnerable: Comparing float to string representation
    // Precision issues and format differences cause problems

    requestedStr := fmt.Sprintf("%f", requested)

    // "100.000000" vs "100.0" - may not match even if equal
    return requestedStr == available
}

// Vulnerable: Using wrong enum value
type Permission int

const (
    Read Permission = iota
    Write
    Delete
    Admin
)

func vulnerableCheckPermission(userPerm, requiredPerm Permission) bool {
    // Vulnerable: Checking if user permission equals OR EXCEEDS required
    // But permission values aren't meant to be compared numerically!

    return userPerm >= requiredPerm

    // Read=0, Write=1, Delete=2, Admin=3
    // User with Delete(2) would pass Admin(3) check? No.
    // User with Write(1) would fail Read(0) check? Yes, incorrectly!
}

// Vulnerable: Comparing memory addresses instead of values
#include <string.h>

int vulnerable_compare_tokens(const char *provided, const char *stored) {
    // Vulnerable: Comparing pointers instead of string content
    // Only returns true if both point to same memory location

    return provided == stored;  // Wrong: compares addresses!

    // Should be: strcmp(provided, stored) == 0
}

// Vulnerable: Using sizeof on pointer instead of array
int vulnerable_compare_arrays(int *arr1, int *arr2) {
    // Vulnerable: sizeof(arr1) is pointer size, not array size!
    return memcmp(arr1, arr2, sizeof(arr1)) == 0;

    // This only compares first 4 or 8 bytes (pointer size)
}

Fixed Code

// Fixed: Comparing correct factors for session validation
public class FixedSessionValidator {

    public boolean validateSession(HttpServletRequest request) {
        String sessionToken = request.getHeader("X-Session-Token");

        if (sessionToken == null || sessionToken.isEmpty()) {
            return false;
        }

        // Get stored session by token
        Session storedSession = sessionStore.get(sessionToken);

        if (storedSession == null) {
            return false;
        }

        // Fixed: Compare the session token itself
        if (!secureCompare(sessionToken, storedSession.getToken())) {
            return false;
        }

        // Fixed: Also verify session hasn't expired
        if (storedSession.isExpired()) {
            return false;
        }

        // Fixed: Verify IP binding if enabled
        String clientIP = request.getRemoteAddr();
        if (storedSession.isIPBound() &&
            !clientIP.equals(storedSession.getBoundIP())) {
            return false;
        }

        return true;
    }

    private boolean secureCompare(String a, String b) {
        return MessageDigest.isEqual(
            a.getBytes(StandardCharsets.UTF_8),
            b.getBytes(StandardCharsets.UTF_8)
        );
    }
}

# Fixed: Comparing canonical paths
import os

def fixed_check_file_access(requested_path, allowed_directory):
    # Fixed: Resolve to canonical (real) paths
    canonical_requested = os.path.realpath(requested_path)
    canonical_allowed = os.path.realpath(allowed_directory)

    # Fixed: Ensure allowed directory ends with separator
    if not canonical_allowed.endswith(os.sep):
        canonical_allowed += os.sep

    # Fixed: Compare canonical paths
    # Also check the file itself is within allowed (not just starts with)
    return canonical_requested.startswith(canonical_allowed) or \
           canonical_requested == canonical_allowed.rstrip(os.sep)


# Alternative: Using pathlib for cleaner comparison
from pathlib import Path

def fixed_check_file_access_pathlib(requested_path, allowed_directory):
    try:
        requested = Path(requested_path).resolve()
        allowed = Path(allowed_directory).resolve()

        # Fixed: Check if requested path is relative to allowed directory
        requested.relative_to(allowed)
        return True
    except ValueError:
        # Not relative to allowed directory
        return False

// Fixed: Using correct timestamp for cache validation
class FixedCache {

    isValid(cachedItem, sourceItem) {
        // Fixed: Compare cached time against source modification time
        return cachedItem.cachedAt >= sourceItem.modifiedAt;
    }

    get(key) {
        const cached = this.cache.get(key);

        if (!cached) {
            return this.fetchAndCache(key);
        }

        const source = this.source.getMetadata(key);

        // Fixed: Use modification time for comparison
        if (this.isValid(cached, source)) {
            return cached.data;
        }

        return this.fetchAndCache(key);
    }

    fetchAndCache(key) {
        const data = this.source.get(key);
        const metadata = this.source.getMetadata(key);

        this.cache.set(key, {
            data: data,
            cachedAt: Date.now(),
            sourceModifiedAt: metadata.modifiedAt
        });

        return data;
    }
}

<?php
// Fixed: Multi-tenant query with proper tenant isolation
class FixedDocumentService {

    public function getDocument($documentId, $userId, $tenantId) {
        // Fixed: Include tenant ID in comparison
        $sql = "SELECT * FROM documents
                WHERE id = ?
                AND owner_id = ?
                AND tenant_id = ?";

        $result = $this->db->query($sql, [$documentId, $userId, $tenantId]);

        return $result;
    }

    // Alternative: Use tenant-scoped repository
    public function getDocumentScoped($documentId, $tenantContext) {
        // Tenant context is set at request level, can't be spoofed
        $sql = "SELECT * FROM documents
                WHERE id = ?
                AND tenant_id = ?";

        $result = $this->db->query($sql, [
            $documentId,
            $tenantContext->getTenantId()
        ]);

        if ($result && $result['owner_id'] !== $tenantContext->getUserId()) {
            // Check ownership within tenant
            if (!$this->canAccessDocument($result, $tenantContext)) {
                throw new UnauthorizedException();
            }
        }

        return $result;
    }
}

// Fixed: Comparing numeric values correctly
import (
    "math"
)

func fixedCheckBalance(requested float64, available float64) bool {
    // Fixed: Compare as float64 values
    // Use epsilon for floating point comparison
    const epsilon = 0.0001

    return requested <= available || math.Abs(requested-available) < epsilon
}

// If available comes as string, parse it first
func fixedCheckBalanceFromString(requested float64, availableStr string) (bool, error) {
    available, err := strconv.ParseFloat(availableStr, 64)
    if err != nil {
        return false, fmt.Errorf("invalid balance format: %w", err)
    }

    return fixedCheckBalance(requested, available), nil
}

// Fixed: Using proper permission checking
type Permission int

const (
    Read Permission = 1 << iota  // 1
    Write                        // 2
    Delete                       // 4
    Admin = Read | Write | Delete // 7 (all permissions)
)

func fixedCheckPermission(userPerm, requiredPerm Permission) bool {
    // Fixed: Use bitwise AND to check if user has required permission
    return (userPerm & requiredPerm) == requiredPerm
}

// Usage:
// User with Read|Write (3) checking for Read (1) -> true
// User with Read (1) checking for Write (2) -> false
// User with Admin (7) checking for Delete (4) -> true

// Fixed: Comparing string values correctly
#include <string.h>
#include <openssl/crypto.h>

int fixed_compare_tokens(const char *provided, const char *stored) {
    if (provided == NULL || stored == NULL) {
        return 0;  // Treat NULL as non-matching
    }

    size_t provided_len = strlen(provided);
    size_t stored_len = strlen(stored);

    if (provided_len != stored_len) {
        return 0;
    }

    // Fixed: Compare string content, not pointers
    // Use constant-time comparison for security
    return CRYPTO_memcmp(provided, stored, stored_len) == 0;
}

// Fixed: Comparing arrays with correct size
int fixed_compare_arrays(const int *arr1, const int *arr2, size_t count) {
    // Fixed: Pass array size as parameter
    return memcmp(arr1, arr2, count * sizeof(int)) == 0;
}

// Usage:
// int a[10], b[10];
// fixed_compare_arrays(a, b, 10);  // Compares all 10 elements

# Fixed: Comparing the right factors for object equality
class Document:
    def __init__(self, doc_id, tenant_id, owner_id, content):
        self.doc_id = doc_id
        self.tenant_id = tenant_id
        self.owner_id = owner_id
        self.content = content

    def __eq__(self, other):
        if not isinstance(other, Document):
            return False

        # Fixed: Compare all identity factors
        return (self.doc_id == other.doc_id and
                self.tenant_id == other.tenant_id)

    def __hash__(self):
        # Fixed: Hash based on same factors as equality
        return hash((self.doc_id, self.tenant_id))


class FixedAccessChecker:
    def can_access(self, user, document):
        # Fixed: Compare correct factors for access decision

        # Factor 1: Same tenant
        if user.tenant_id != document.tenant_id:
            return False

        # Factor 2: Owner or has permission
        if user.user_id == document.owner_id:
            return True

        # Factor 3: Check explicit permissions
        return self.has_permission(user, document, 'read')

CVE Examples

CVE-2006-5393: Product compared IP address against wrong stored value.
CVE-2005-3025: Authentication compared password hash against wrong user's hash.

CWE-697: Incorrect Comparison (parent)
CWE-1023: Incomplete Comparison with Missing Factors (sibling)
CWE-183: Permissive List of Allowed Inputs (related)

References

MITRE Corporation. "CWE-1025: Comparison Using Wrong Factors." https://cwe.mitre.org/data/definitions/1025.html
OWASP. "Access Control Design Guidelines."
SANS. "Secure Coding Practices."

Comparison Using Wrong Factors

Description

Risk

Solution

Common Consequences

Example Code

Vulnerable Code

Fixed Code

CVE Examples

References

Platform

Resources

Company

Latest Articles

Comparison Using Wrong Factors

Description

Risk

Solution

Common Consequences

Example Code

Vulnerable Code

Fixed Code

CVE Examples

Related CWEs

References