Invokable Control Element with Excessive Volume of Commented-out Code

Description

Invokable Control Element with Excessive Volume of Commented-out Code occurs when a function, method, or similar code element contains an excessive amount of code that has been commented out within its body. CISQ recommends using a 2% threshold as the default measure for what constitutes excessive commented code. Commented-out code clutters the codebase, makes functions harder to understand, and often indicates incomplete changes, debug code left behind, or uncertainty about whether code is needed.

Risk

Excessive commented-out code has indirect security implications. It creates confusion about what code is actually executing, complicating security reviews. Old commented code may contain security vulnerabilities that developers might accidentally uncomment. The commented code may reference deprecated APIs or contain outdated security practices. It increases cognitive load, making developers more likely to miss actual security issues. Version control makes commented code unnecessary - if it's needed, it can be retrieved from history. Commented code may contain sensitive information like passwords or API keys.

Solution

Remove commented-out code - use version control to preserve history. If code might be needed, create a branch or tag before removing it. Document why code was removed in commit messages. Use feature flags instead of commenting out code for A/B testing. Set up linting rules to detect commented code blocks. Review commented code during code reviews and require removal. If code is temporarily disabled, use proper feature toggle mechanisms. Never leave debug code or sensitive information in comments.

Common Consequences

Impact	Details
Other	Scope: Other Reduce Maintainability - Commented code clutters the codebase and confuses readers.
Other	Scope: Other Increase Analytical Complexity - Security review is harder with dead code mixed with active code.
Confidentiality	Scope: Confidentiality Information Disclosure - Commented code may contain sensitive information.

Example Code

Vulnerable Code

// Vulnerable: Method with excessive commented-out code
public class VulnerablePaymentProcessor {

    public PaymentResult processPayment(PaymentRequest request) {
        // Initialize payment
        PaymentResult result = new PaymentResult();

        // Old validation logic - keeping just in case
        // if (request.getAmount() == null) {
        //     throw new ValidationException("Amount required");
        // }
        // if (request.getAmount().compareTo(BigDecimal.ZERO) <= 0) {
        //     throw new ValidationException("Amount must be positive");
        // }
        // String cardNumber = request.getCardNumber();
        // if (cardNumber == null || cardNumber.length() < 13) {
        //     throw new ValidationException("Invalid card number");
        // }

        // Validate request
        validateRequest(request);

        // Old payment gateway integration - DO NOT DELETE
        // PaymentGateway gateway = new OldPaymentGateway();
        // gateway.setApiKey("sk_live_old_key_12345"); // OLD API KEY!
        // gateway.setEndpoint("https://old-api.payment.com/v1");
        // try {
        //     GatewayResponse response = gateway.charge(
        //         request.getCardNumber(),
        //         request.getExpiry(),
        //         request.getCvv(),
        //         request.getAmount()
        //     );
        //     if (response.isSuccess()) {
        //         result.setTransactionId(response.getTransactionId());
        //     }
        // } catch (GatewayException e) {
        //     logger.error("Payment failed", e);
        //     throw new PaymentException("Payment processing failed");
        // }

        // Debug code - remove before production
        // System.out.println("Card: " + request.getCardNumber());
        // System.out.println("CVV: " + request.getCvv());
        // System.out.println("Amount: " + request.getAmount());

        // Current implementation
        PaymentGateway gateway = gatewayFactory.createGateway();
        GatewayResponse response = gateway.charge(request);

        // Old response handling
        // if (response.getCode() == 200) {
        //     result.setSuccess(true);
        // } else if (response.getCode() == 401) {
        //     // Auth failed - retry with backup key
        //     gateway.setApiKey("sk_live_backup_key_67890");
        //     response = gateway.charge(request);
        // } else if (response.getCode() == 500) {
        //     // Server error - wait and retry
        //     Thread.sleep(1000);
        //     response = gateway.charge(request);
        // }

        result.setTransactionId(response.getTransactionId());
        result.setSuccess(response.isSuccess());

        // Logging we might need later
        // logTransaction(request, result);
        // notifyFraudDetection(request);
        // updateMetrics(result);

        return result;
    }

    // Issues:
    // 1. ~40% of method is commented code
    // 2. Contains old API keys (security risk!)
    // 3. Contains debug code printing sensitive data
    // 4. Hard to understand what code actually executes
    // 5. Old code may contain vulnerabilities
}

# Vulnerable: Function with excessive commented code
class VulnerableUserService:

    def create_user(self, user_data: dict) -> User:
        """Create a new user account."""

        # Old validation - replaced with schema validation
        # if not user_data.get('email'):
        #     raise ValueError("Email is required")
        # if not user_data.get('password'):
        #     raise ValueError("Password is required")
        # if len(user_data.get('password', '')) < 8:
        #     raise ValueError("Password too short")
        # if not '@' in user_data.get('email', ''):
        #     raise ValueError("Invalid email")

        # Validate using new schema
        validated = UserSchema().load(user_data)

        # Old password hashing - DO NOT USE
        # import md5
        # password_hash = md5.new(user_data['password']).hexdigest()

        # Slightly better but still bad
        # import hashlib
        # password_hash = hashlib.sha256(
        #     user_data['password'].encode()
        # ).hexdigest()

        # Current secure hashing
        password_hash = bcrypt.hashpw(
            validated['password'].encode(),
            bcrypt.gensalt()
        )

        # Old user creation with raw SQL
        # cursor = self.db.cursor()
        # cursor.execute(
        #     f"INSERT INTO users (email, password) VALUES "
        #     f"('{validated['email']}', '{password_hash}')"
        # )  # SQL INJECTION!
        # self.db.commit()
        # user_id = cursor.lastrowid

        # Debug code
        # print(f"Creating user: {validated['email']}")
        # print(f"Password hash: {password_hash}")
        # import pdb; pdb.set_trace()

        # Create user using ORM
        user = User(
            email=validated['email'],
            password_hash=password_hash
        )
        self.session.add(user)
        self.session.commit()

        # Old notification code
        # send_welcome_email(user.email)
        # notify_admin_new_user(user)
        # update_user_count_metric()

        return user

    # Problems:
    # - Shows evolution of vulnerable to secure code
    # - Contains SQL injection vulnerability
    # - Contains MD5/SHA256 password hashing (insecure)
    # - Contains debug code with sensitive data

Fixed Code

// Fixed: Clean method without commented-out code
public class FixedPaymentProcessor {

    private final PaymentGatewayFactory gatewayFactory;
    private final PaymentValidator validator;
    private final TransactionLogger transactionLogger;

    /**
     * Process a payment request.
     *
     * @param request The payment request to process
     * @return The result of the payment processing
     * @throws PaymentValidationException if request is invalid
     * @throws PaymentProcessingException if payment fails
     */
    public PaymentResult processPayment(PaymentRequest request) {
        // Validate request
        validator.validate(request);

        // Process through gateway
        PaymentGateway gateway = gatewayFactory.createGateway();
        GatewayResponse response = gateway.charge(request);

        // Build result
        PaymentResult result = new PaymentResult();
        result.setTransactionId(response.getTransactionId());
        result.setSuccess(response.isSuccess());

        // Log transaction (async, non-blocking)
        transactionLogger.logAsync(request, result);

        return result;
    }
}

// Supporting documentation in separate files:

// CHANGELOG.md or git history:
// ## Payment Gateway Migration (2024-01-15)
// - Migrated from OldPaymentGateway to NewPaymentGateway
// - Old implementation available in git history: commit abc123
// - Migration guide: docs/payment-migration.md

// docs/payment-migration.md:
// ## Why we changed payment gateways
// - Old gateway deprecated as of 2023-12
// - New gateway has better fraud detection
// - See commit abc123 for old implementation if rollback needed

# Fixed: Clean function without commented-out code
class FixedUserService:
    """User management service."""

    def __init__(self, session, password_hasher, event_publisher):
        self._session = session
        self._password_hasher = password_hasher
        self._event_publisher = event_publisher

    def create_user(self, user_data: dict) -> User:
        """Create a new user account.

        Args:
            user_data: Dictionary with email and password

        Returns:
            The created User object

        Raises:
            ValidationError: If user data is invalid
        """
        # Validate using schema
        validated = UserSchema().load(user_data)

        # Hash password securely
        password_hash = self._password_hasher.hash(validated['password'])

        # Create user
        user = User(
            email=validated['email'],
            password_hash=password_hash
        )
        self._session.add(user)
        self._session.commit()

        # Publish event for async notifications
        self._event_publisher.publish(UserCreatedEvent(user))

        return user


# Password hasher implementation (separate file)
class BcryptPasswordHasher:
    """Secure password hashing using bcrypt.

    Note: Replaced MD5/SHA256 hashing in v2.0.
    See CHANGELOG.md for migration details.
    """

    def __init__(self, rounds: int = 12):
        self._rounds = rounds

    def hash(self, password: str) -> bytes:
        return bcrypt.hashpw(
            password.encode(),
            bcrypt.gensalt(rounds=self._rounds)
        )

    def verify(self, password: str, hash: bytes) -> bool:
        return bcrypt.checkpw(password.encode(), hash)


# CHANGELOG.md:
"""
## v2.0.0 (2024-01-15)

### Security
- **BREAKING**: Replaced MD5/SHA256 password hashing with bcrypt
- Previous implementations are security vulnerabilities
- See migration guide for upgrading existing password hashes

### Removed
- Raw SQL queries replaced with ORM
- Debug print statements removed
- Inline validation replaced with schema validation

### For historical reference
- Old implementations available in git tag v1.9.9
- Do not use old password hashing methods
"""

// Fixed: Clean JavaScript without commented code
class FixedUserController {
    constructor(userService, logger) {
        this.userService = userService;
        this.logger = logger;
    }

    /**
     * Create a new user.
     * @param {Object} req - Express request
     * @param {Object} res - Express response
     */
    async createUser(req, res) {
        try {
            const user = await this.userService.create(req.body);

            this.logger.info('User created', { userId: user.id });

            return res.status(201).json({
                success: true,
                user: user.toPublicJSON()
            });

        } catch (error) {
            this.logger.error('User creation failed', { error: error.message });

            if (error instanceof ValidationError) {
                return res.status(400).json({
                    success: false,
                    error: error.message
                });
            }

            return res.status(500).json({
                success: false,
                error: 'Internal server error'
            });
        }
    }
}

// If you need to preserve old implementations for reference:
// 1. Use git tags: git tag v1.0-before-refactor
// 2. Document in CHANGELOG.md
// 3. Create migration guides in docs/
// 4. NEVER leave old code as comments

---

CVE Examples

While commented code itself rarely causes direct vulnerabilities, it has been associated with information disclosure (exposing API keys, passwords) and has contributed to confusion that led to security bugs.

---

Related CWEs

• CWE-1071: Empty Code Block (parent category)
• CWE-546: Suspicious Comment (related)
• CWE-615: Inclusion of Sensitive Information in Source Code Comments (related)

---

References

1. MITRE Corporation. "CWE-1085: Invokable Control Element with Excessive Volume of Commented-out Code." https://cwe.mitre.org/data/definitions/1085.html
2. CISQ. "Automated Source Code Quality Measures."
3. Martin, Robert C. "Clean Code" - Comments chapter.