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
| Impact | Details |
|---|---|
| Other | Scope: 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. |
| Other | Scope: 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.
Related CWEs
- 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
- MITRE Corporation. "CWE-1121: Excessive McCabe Cyclomatic Complexity." https://cwe.mitre.org/data/definitions/1121.html
- McCabe, T.J. (1976). "A Complexity Measure." IEEE Transactions on Software Engineering.
- NIST Complexity Guidelines
- CISQ Automated Quality Characteristic Measures