Skip to content

CWE-398: 7PK - Code Quality

Overview

Complex logic, dead code, inconsistent error handling, missing input validation and unmanaged resources are indicators of poor code quality. They make code harder to review, test and maintain securely, which is the condition security vulnerabilities survive in.

Relationship to Other CWEs

CWE-398 is a Category, and MITRE's mapping guidance for it is Prohibited. The rationale is the general one for categories: "Using categories for mapping has been discouraged since 2019. Categories are informal organizational groupings of weaknesses that can help CWE users with data aggregation, navigation, and browsing. However, they are not weaknesses in themselves." A finding should carry a member weakness's number, not this one.

Its members in MITRE's own listing are narrower than this page's subject and are mostly memory and API misuse, six of which have pages here:

  • CWE-401 - Missing Release of Memory after Effective Lifetime
  • CWE-404 - Improper Resource Shutdown or Release
  • CWE-415 - Double Free
  • CWE-416 - Use After Free
  • CWE-476 - NULL Pointer Dereference
  • CWE-477 - Use of Obsolete Function
  • CWE-457 (Use of Uninitialized Variable), CWE-474 (Use of Function with Inconsistent Implementations) and CWE-475 (Undefined Behavior for Input to API) - no pages here

The code smells this page is about are not on that list, which is worth knowing before you go looking for a member weakness to file instead. Scanners emit CWE-398 for empty catch blocks, dead code and unused variables because the category's name fits, and MITRE's own membership does not cover them. Where a more specific number exists, it is usually outside this category: CWE-252 (Unchecked Return Value) for a result nobody inspects, CWE-248 (Uncaught Exception) for a failure that escapes the frame meant to contain it, CWE-390 (Detection of Error Condition Without Action) for the empty catch itself, and CWE-561 (Dead Code) or CWE-1164 (Irrelevant Code) for the rest - none of the last three has a page here. Where none of them fits, the finding is a maintainability defect with no CWE to carry, and this page is the remediation reference for it.

OWASP Classification

A10:2025 - Mishandling of Exceptional Conditions

Risk

Medium: Vulnerabilities get overlooked, security reviews take longer, edge cases go untested, resources leak, and security controls end up applied inconsistently. Later maintenance on the same code introduces further bugs.

Remediation Steps

Core Principle: Treat poor error/edge-case handling as security risk; make defensive checks explicit and consistent.

Locate the poor code quality indicators

  • Read the flaw details for the file, line number and code pattern involved
  • Name the quality issue: empty catch block, dead code, unused variable, missing validation, resource leak, high complexity
  • Work out the security impact - whether the issue creates a vulnerability or hides one
  • Trace the code to understand what it is supposed to do and why the issue is there

Implement consistent error handling (Primary Defense)

  • Replace empty catch blocks such as catch (Exception e) { } with handling that records the failure and acts on it
  • Log the exception message and stack trace, with PII and credentials redacted
  • Return a user-friendly message to the client and keep the detail in the logs
  • Catch specific exceptions such as FileNotFoundException rather than the base Exception

Remove dead/unreachable code

  • Delete commented-out code and rely on version control for the history
  • Delete variables and functions that are declared but never used
  • Remove unreachable branches: if (false) blocks, code after a return statement
  • Remove debugging leftovers such as System.out.println(), console.log() and temporary test code

Reduce complexity

  • Extract methods when a function runs past 50 lines or does more than one thing
  • Cut cyclomatic complexity by simplifying nested if/else chains and reducing the number of decision points
  • Flatten nested conditionals: return early on error conditions instead of nesting the success path, combine conditions with &&/||, or extract a condition into a named method

Follow secure coding standards

  • Validate parameters for null, range, format and length before use
  • Use parameterized queries and prepared statements to prevent SQL injection
  • Release resources with try-with-resources, finally blocks or using statements
  • Apply least privilege - do not run with admin or root unless it is necessary

Test the code quality improvements

  • Run static analysis with SonarQube, PMD or SpotBugs to confirm the findings have gone. Not FindBugs - it was last released in 2015 and SpotBugs is its maintained successor, so a build still calling FindBugs is analysing modern bytecode with a tool that predates it
  • Check that cyclomatic complexity has come down
  • Trigger the error conditions and confirm the handler returns what it should
  • Run code coverage to confirm no unreachable paths remain
  • Re-scan with the security scanner to confirm the issue is resolved

Common Vulnerable Patterns

// Empty catch - swallows the error, hides the failure
try:
    dangerous()
catch Exception e:
    // TODO: handle

// Dead code
if false:
    // never executes

// Unused variable that still holds sensitive data in memory
password = getPassword()  // never used

// No validation
function setAge(age):
    this.age = age  // no range check

// Resource leak
stream = openFile(file)
// no close()

Why this is vulnerable: none of these is a vulnerability, and that is the point. Each one removes a signal that something has gone wrong, so a real defect nearby stops being visible. The empty catch destroys the evidence that an operation failed, and execution continues into code written on the assumption that it succeeded. The unused password stays resident in memory for the lifetime of the object, extending the window in which a crash dump or a memory disclosure can reach it. The missing range check means every caller's assumption about age is untested, so the first one to be wrong is found in production. The unclosed stream holds a descriptor that is only exhausted under load.

A finding here is a pointer rather than a diagnosis. There is no exploit to write and no severity to assign; the value is that these constructs cluster where review attention has been thin, which is where the exploitable defects have also been found. Fixing the indicator is cheap and worth doing, but the reason to act on it is to make the surrounding code legible enough that the next reader can see what it does.

Secure Patterns

// Specific exception, logged, wrapped with context
try:
    dangerous()
catch SpecificException e:
    log.error("Operation failed", e)
    throw ServiceException("Unable to process", e)

// Input validation
function setAge(age):
    if age < 0 or age > 150:
        throw IllegalArgumentException("Invalid age")
    this.age = age

// Automatic resource management
with stream = openFile(file):
    process(stream)

Why this works: Catching a specific exception type instead of the base class prevents unrelated failures from being silently swallowed. Validating inputs at the boundary rejects bad state before it propagates. Automatic resource management guarantees cleanup runs on every exit path, including exceptions.

Additional Resources