Excessive McCabe Cyclomatic Complexity

Description

Excessive McCabe Cyclomatic Complexity occurs when code contains McCabe cyclomatic complexity that exceeds a desirable maximum threshold. Cyclomatic complexity is a quantitative measure of the number of linearly independent paths through a program's source code, calculated based on the control flow graph. Each decision point (if, else, switch case, loop, logical operator) increases complexity. High cyclomatic complexity indicates code that is difficult to understand, test, and maintain, making security vulnerabilities harder to identify and fix.

Risk

High cyclomatic complexity has significant indirect security implications. More execution paths mean more potential for bugs, including security bugs. Testing all paths becomes impractical, leaving untested edge cases. Code reviews are less effective when reviewers cannot follow all paths. Security auditors may miss vulnerabilities in complex functions. Maintenance changes have higher risk of breaking existing functionality. Conditional security checks may be inconsistently applied across paths. Static analysis tools may timeout or produce unreliable results. Emergency security patches are riskier in complex code.

Solution

Set maximum cyclomatic complexity thresholds (commonly 10 or 15). Refactor functions exceeding thresholds into smaller units. Extract complex conditional logic into separate methods. Use polymorphism instead of complex switch statements. Apply guard clauses to reduce nesting and complexity. Use lookup tables instead of long if-else chains. Automate complexity checking in CI/CD pipelines. Prioritize refactoring security-critical high-complexity code. Use design patterns like Strategy or State for complex behavior. Track complexity metrics over time.

Common Consequences

ImpactDetails
OtherScope: Other

Reduce Maintainability - The issue makes it more difficult to understand and maintain the product, indirectly affecting security by making vulnerabilities harder to find and fix. May facilitate introducing new vulnerabilities.
OtherScope: Other

Increase Analytical Complexity - High cyclomatic complexity makes thorough security analysis impractical.

Example Code

Vulnerable Code

// Vulnerable: Cyclomatic complexity of approximately 25+
// Each if/else/case/&&/|| adds to complexity

public class PaymentValidator {

    public ValidationResult validatePayment(Payment payment, User user, Context ctx) {
        // Complexity starts at 1, increases with each branch

        // +1 for if
        if (payment == null) {
            return ValidationResult.error("null_payment");
        }

        // +1 for if
        if (user == null) {
            return ValidationResult.error("null_user");
        }

        // +2 for if and &&
        if (!user.isActive() && !user.isGracePeriod()) {
            return ValidationResult.error("inactive_user");
        }

        // +1 for if
        if (payment.getAmount() <= 0) {
            return ValidationResult.error("invalid_amount");
        }

        // +1 for if
        if (payment.getAmount() > ctx.getMaxAmount()) {
            return ValidationResult.error("amount_too_large");
        }

        // +3 for if and two ||
        if (ctx.isWeekend() || ctx.isHoliday() || ctx.isMaintenanceWindow()) {
            // +1 for nested if
            if (!user.isPremium()) {
                return ValidationResult.error("processing_unavailable");
            }
        }

        // +4 for switch with 4 cases
        switch (payment.getMethod()) {
            case CREDIT_CARD:
                // +1 for if
                if (payment.getCardNumber() == null) {
                    return ValidationResult.error("missing_card");
                }
                // +1 for if
                if (!validateLuhn(payment.getCardNumber())) {
                    return ValidationResult.error("invalid_card");
                }
                // +1 for if
                if (isExpired(payment.getExpiry())) {
                    return ValidationResult.error("card_expired");
                }
                // +2 for if and ||
                if (payment.getCvv() == null || payment.getCvv().length() < 3) {
                    return ValidationResult.error("invalid_cvv");
                }
                break;

            case DEBIT_CARD:
                // +1 for if
                if (payment.getPin() == null) {
                    return ValidationResult.error("missing_pin");
                }
                // +1 for if
                if (!validateDebitCard(payment)) {
                    return ValidationResult.error("invalid_debit");
                }
                break;

            case BANK_TRANSFER:
                // +2 for if and ||
                if (payment.getAccountNumber() == null || payment.getRoutingNumber() == null) {
                    return ValidationResult.error("missing_bank_info");
                }
                // +1 for if
                if (!validateBankAccount(payment)) {
                    return ValidationResult.error("invalid_bank");
                }
                break;

            case CRYPTO:
                // +1 for if
                if (payment.getWalletAddress() == null) {
                    return ValidationResult.error("missing_wallet");
                }
                // +1 for if
                if (!validateWalletAddress(payment.getWalletAddress())) {
                    return ValidationResult.error("invalid_wallet");
                }
                break;

            default:
                return ValidationResult.error("unsupported_method");
        }

        // More complexity for fraud checks
        // +1 for if
        if (ctx.isHighRiskRegion()) {
            // +1 for if
            if (payment.getAmount() > 1000) {
                // +1 for if
                if (!user.hasVerifiedIdentity()) {
                    return ValidationResult.error("verification_required");
                }
            }
        }

        // Total cyclomatic complexity: ~27
        return ValidationResult.success();
    }
}
# Vulnerable: High cyclomatic complexity function

def process_transaction(transaction, account, config):
    """
    Process a financial transaction.
    Cyclomatic complexity: approximately 20+
    """
    result = {"status": "pending", "errors": []}

    # Branch 1
    if transaction is None:
        return {"status": "error", "errors": ["null_transaction"]}

    # Branch 2
    if account is None:
        return {"status": "error", "errors": ["null_account"]}

    # Branch 3, 4 (and condition)
    if not account.is_active and not account.in_grace_period:
        return {"status": "error", "errors": ["inactive_account"]}

    # Branch 5
    if transaction.amount <= 0:
        return {"status": "error", "errors": ["invalid_amount"]}

    # Branch 6
    if transaction.amount > account.daily_limit:
        # Branch 7
        if not account.has_override_permission:
            return {"status": "error", "errors": ["exceeds_limit"]}

    # Branch 8-11 (multiple cases in type check)
    if transaction.type == "withdrawal":
        # Branch 12
        if account.balance < transaction.amount:
            return {"status": "error", "errors": ["insufficient_funds"]}
        # Branch 13
        if transaction.amount > config.max_withdrawal:
            return {"status": "error", "errors": ["exceeds_max_withdrawal"]}

    elif transaction.type == "deposit":
        # Branch 14
        if transaction.source not in config.approved_sources:
            return {"status": "error", "errors": ["unapproved_source"]}
        # Branch 15, 16 (and condition)
        if transaction.amount > 10000 and not transaction.has_documentation:
            return {"status": "error", "errors": ["documentation_required"]}

    elif transaction.type == "transfer":
        # Branch 17
        if transaction.destination is None:
            return {"status": "error", "errors": ["no_destination"]}
        # Branch 18
        if not validate_account(transaction.destination):
            return {"status": "error", "errors": ["invalid_destination"]}
        # Branch 19
        if is_same_owner(account, transaction.destination):
            # Branch 20
            if transaction.amount > config.internal_transfer_limit:
                return {"status": "error", "errors": ["internal_limit_exceeded"]}
        else:
            # Branch 21
            if transaction.amount > config.external_transfer_limit:
                return {"status": "error", "errors": ["external_limit_exceeded"]}

    else:
        return {"status": "error", "errors": ["unknown_transaction_type"]}

    # More branches for fraud detection...
    return execute_transaction(transaction, account)

Fixed Code

// Fixed: Refactored to reduce cyclomatic complexity
// Each method has complexity < 10

public class PaymentValidator {

    private final Map<PaymentMethod, PaymentMethodValidator> validators;

    public PaymentValidator() {
        // Strategy pattern replaces complex switch
        validators = Map.of(
            PaymentMethod.CREDIT_CARD, new CreditCardValidator(),
            PaymentMethod.DEBIT_CARD, new DebitCardValidator(),
            PaymentMethod.BANK_TRANSFER, new BankTransferValidator(),
            PaymentMethod.CRYPTO, new CryptoValidator()
        );
    }

    /**
     * Main validation entry point.
     * Cyclomatic complexity: 4
     */
    public ValidationResult validatePayment(Payment payment, User user, Context ctx) {
        // Validate inputs
        ValidationResult inputValidation = validateInputs(payment, user);
        if (!inputValidation.isSuccess()) {
            return inputValidation;
        }

        // Validate context
        ValidationResult contextValidation = validateContext(user, ctx);
        if (!contextValidation.isSuccess()) {
            return contextValidation;
        }

        // Validate amount
        ValidationResult amountValidation = validateAmount(payment, ctx);
        if (!amountValidation.isSuccess()) {
            return amountValidation;
        }

        // Delegate to method-specific validator
        return validatePaymentMethod(payment);
    }

    /**
     * Input validation.
     * Cyclomatic complexity: 3
     */
    private ValidationResult validateInputs(Payment payment, User user) {
        if (payment == null) {
            return ValidationResult.error("null_payment");
        }
        if (user == null) {
            return ValidationResult.error("null_user");
        }
        if (!user.isActive() && !user.isGracePeriod()) {
            return ValidationResult.error("inactive_user");
        }
        return ValidationResult.success();
    }

    /**
     * Context validation.
     * Cyclomatic complexity: 3
     */
    private ValidationResult validateContext(User user, Context ctx) {
        boolean processingRestricted = ctx.isWeekend() || ctx.isHoliday()
                                       || ctx.isMaintenanceWindow();

        if (processingRestricted && !user.isPremium()) {
            return ValidationResult.error("processing_unavailable");
        }
        return ValidationResult.success();
    }

    /**
     * Amount validation.
     * Cyclomatic complexity: 3
     */
    private ValidationResult validateAmount(Payment payment, Context ctx) {
        if (payment.getAmount() <= 0) {
            return ValidationResult.error("invalid_amount");
        }
        if (payment.getAmount() > ctx.getMaxAmount()) {
            return ValidationResult.error("amount_too_large");
        }
        return ValidationResult.success();
    }

    /**
     * Payment method validation using Strategy pattern.
     * Cyclomatic complexity: 2
     */
    private ValidationResult validatePaymentMethod(Payment payment) {
        PaymentMethodValidator validator = validators.get(payment.getMethod());
        if (validator == null) {
            return ValidationResult.error("unsupported_method");
        }
        return validator.validate(payment);
    }
}

// Separate validator classes with low complexity

interface PaymentMethodValidator {
    ValidationResult validate(Payment payment);
}

class CreditCardValidator implements PaymentMethodValidator {
    /**
     * Credit card validation.
     * Cyclomatic complexity: 5
     */
    @Override
    public ValidationResult validate(Payment payment) {
        if (payment.getCardNumber() == null) {
            return ValidationResult.error("missing_card");
        }
        if (!LuhnValidator.validate(payment.getCardNumber())) {
            return ValidationResult.error("invalid_card");
        }
        if (ExpiryChecker.isExpired(payment.getExpiry())) {
            return ValidationResult.error("card_expired");
        }
        if (!isValidCvv(payment.getCvv())) {
            return ValidationResult.error("invalid_cvv");
        }
        return ValidationResult.success();
    }

    private boolean isValidCvv(String cvv) {
        return cvv != null && cvv.length() >= 3;
    }
}
# Fixed: Refactored with lower cyclomatic complexity

class TransactionProcessor:
    """Process transactions with manageable complexity per method."""

    def __init__(self, config):
        self.config = config
        # Strategy pattern for transaction types
        self.handlers = {
            "withdrawal": self._handle_withdrawal,
            "deposit": self._handle_deposit,
            "transfer": self._handle_transfer,
        }

    def process_transaction(self, transaction, account):
        """
        Main entry point for transaction processing.
        Cyclomatic complexity: 4
        """
        # Validate inputs
        error = self._validate_inputs(transaction, account)
        if error:
            return TransactionResult.error(error)

        # Validate amount
        error = self._validate_amount(transaction, account)
        if error:
            return TransactionResult.error(error)

        # Delegate to type-specific handler
        handler = self.handlers.get(transaction.type)
        if not handler:
            return TransactionResult.error("unknown_transaction_type")

        return handler(transaction, account)

    def _validate_inputs(self, transaction, account):
        """
        Validate input parameters.
        Cyclomatic complexity: 4
        """
        if transaction is None:
            return "null_transaction"
        if account is None:
            return "null_account"
        if not account.is_active and not account.in_grace_period:
            return "inactive_account"
        return None

    def _validate_amount(self, transaction, account):
        """
        Validate transaction amount.
        Cyclomatic complexity: 3
        """
        if transaction.amount <= 0:
            return "invalid_amount"
        if transaction.amount > account.daily_limit:
            if not account.has_override_permission:
                return "exceeds_limit"
        return None

    def _handle_withdrawal(self, transaction, account):
        """
        Handle withdrawal transactions.
        Cyclomatic complexity: 3
        """
        if account.balance < transaction.amount:
            return TransactionResult.error("insufficient_funds")
        if transaction.amount > self.config.max_withdrawal:
            return TransactionResult.error("exceeds_max_withdrawal")
        return self._execute(transaction, account)

    def _handle_deposit(self, transaction, account):
        """
        Handle deposit transactions.
        Cyclomatic complexity: 3
        """
        if transaction.source not in self.config.approved_sources:
            return TransactionResult.error("unapproved_source")
        if transaction.amount > 10000 and not transaction.has_documentation:
            return TransactionResult.error("documentation_required")
        return self._execute(transaction, account)

    def _handle_transfer(self, transaction, account):
        """
        Handle transfer transactions.
        Cyclomatic complexity: 4
        """
        validation_error = self._validate_transfer_destination(transaction)
        if validation_error:
            return TransactionResult.error(validation_error)

        limit_error = self._check_transfer_limits(transaction, account)
        if limit_error:
            return TransactionResult.error(limit_error)

        return self._execute(transaction, account)

    def _validate_transfer_destination(self, transaction):
        """Cyclomatic complexity: 3"""
        if transaction.destination is None:
            return "no_destination"
        if not validate_account(transaction.destination):
            return "invalid_destination"
        return None

    def _check_transfer_limits(self, transaction, account):
        """Cyclomatic complexity: 3"""
        if is_same_owner(account, transaction.destination):
            limit = self.config.internal_transfer_limit
        else:
            limit = self.config.external_transfer_limit

        if transaction.amount > limit:
            return "transfer_limit_exceeded"
        return None

    def _execute(self, transaction, account):
        """Execute the validated transaction."""
        return execute_transaction(transaction, account)

CVE Examples

This CWE is marked as PROHIBITED for direct CVE mapping as it represents a code quality concern rather than a direct security vulnerability.


  • CWE-1120: Excessive Code Complexity (parent)
  • CWE-1122: Excessive Halstead Complexity (related)
  • CWE-1226: Complexity Issues (category member)
  • CWE-1130: CISQ Quality Measures - Maintainability (category member)

References

  1. MITRE Corporation. "CWE-1121: Excessive McCabe Cyclomatic Complexity." https://cwe.mitre.org/data/definitions/1121.html
  2. McCabe, T.J. (1976). "A Complexity Measure." IEEE Transactions on Software Engineering.
  3. NIST Complexity Guidelines
  4. CISQ Automated Quality Characteristic Measures