CWE-498: Cloneable Class Containing Sensitive Information - Java

Overview

Java's Cloneable interface and Object.clone() create a duplication path that bypasses the class's constructor - none of the validation, authorization checks, or business logic it performs runs on a cloned instance. For a class holding credentials, keys, tokens, or a security context, that means any code able to call the class's clone() can create an unauthorized, untracked copy of the sensitive data it holds, and clone()'s default shallow copy means the clone may even share the same backing array as the original.

Primary Defence: Don't implement Cloneable on classes carrying sensitive data. Make the class final (so a subclass can't add cloning support) and provide no clone() method at all - the inherited Object.clone() throws CloneNotSupportedException for any class that isn't Cloneable.

Common Vulnerable Patterns

Cloneable Class with Shallow Copy of Sensitive Data

// VULNERABLE - Cloneable with sensitive data, and a shallow copy at that
public class User implements Cloneable {
    private String username;
    private String password;   // sensitive
    private byte[] secretKey;  // sensitive

    @Override
    public Object clone() {
        try {
            return super.clone();  // shallow copy - shares the same secretKey array
        } catch (CloneNotSupportedException e) {
            return null;
        }
    }
}

// Attack:
User user = authenticate();
User clonedUser = (User) user.clone();  // unauthorized copy, no constructor logic ran
// clonedUser now holds the same password and secretKey reference as user

Why this is vulnerable: Any code holding a reference to user can call clone() and obtain a fully-populated copy without going through authentication, authorization, or any constructor-time validation. Because super.clone() performs a shallow copy, clonedUser.secretKey is the same array instance as user.secretKey - clearing one array with Arrays.fill() clears both, or mutating one mutates the other, neither of which is obvious from the calling code.

Cloned Security Context Outliving Its Authorization

// VULNERABLE - Cloned Security Context Outliving Its Authorization
// Attack: clone a privileged context and use it after the original should have expired
SecurityContext context = getSecurityContext();
if (context.isAdmin()) {
    SecurityContext cloned = (SecurityContext) context.clone();
    // cloned continues to report isAdmin() == true even after the real
    // context is revoked or the user's session/role changes
}

Why this is vulnerable: A security context is meant to reflect the current, authoritative authorization state. A clone is a frozen snapshot that keeps reporting the privileges it had at clone time, regardless of what happens to the original afterward - effectively a second, untracked credential with no expiry tied to the real one.

Secure Patterns

Final Class, No Cloneable, Factory Method Instead

import java.util.Arrays;

// SECURE - immutable, final, no cloning support
public final class SecureCredentials {
    private final String username;
    private final char[] password;

    public SecureCredentials(String user, char[] pass) {
        this.username = user;
        this.password = Arrays.copyOf(pass, pass.length);  // defensive copy on construction
    }

    // Factory method instead of clone() - always goes through the constructor
    public static SecureCredentials create(String user, char[] pass) {
        return new SecureCredentials(user, pass);
    }

    public void clear() {
        Arrays.fill(password, '\0');
    }
}

Why this works: final prevents a subclass from adding Cloneable support that the base class deliberately omits. Not implementing Cloneable means any call to clone() (inherited from Object) throws CloneNotSupportedException - there is no code path that produces an unauthorized copy. The constructor's defensive copy (Arrays.copyOf) ensures the caller's original array can't be mutated to affect the stored credential after construction.

clear() scrubs this object's array, and only this object's array. The defensive copy in the constructor deliberately makes a second one, so the caller's pass array still holds the password after clear() returns. That is the right trade (the alternative is an object whose contents the caller can rewrite later), but it means the caller owns a copy and has to clear it too. What the class rules out is a copy nobody knows about; what it cannot rule out is the one the caller already had.

If Cloning Is Genuinely Required, Gate It Explicitly

public class ApiKey implements Cloneable {
    private String keyId;
    private byte[] secretKey;
    private Set<String> permissions;

    @Override
    protected Object clone() throws CloneNotSupportedException {
        if (!currentUser().hasRole("ADMIN")) {
            throw new CloneNotSupportedException("Only admins can clone API keys");
        }

        ApiKey cloned = (ApiKey) super.clone();
        cloned.secretKey = this.secretKey.clone();       // deep copy, not shared
        cloned.permissions = new HashSet<>(this.permissions);

        auditLog.log("ApiKey cloned by " + currentUser().getName(),
                     "keyId=" + keyId, "timestamp=" + Instant.now());
        registerClone(cloned);  // track the new instance like any other credential

        return cloned;
    }
}

Why this works: If a domain genuinely needs a copy-on-demand operation (e.g. renewing an API key with the same permission set), the override adds the checks clone() skips by default: an authorization check before the copy is created, a deep copy so the clone doesn't alias the original's mutable fields, and audit logging so the copy is tracked like any other credential issuance. This is a controlled copy operation that happens to use the clone() hook, not a bypass of one.

Two things to settle before shipping this shape. clone() here keeps Object's protected access, so callers outside the package cannot reach it - if the operation is meant to be part of the API, widen it to public, and if it is not, a named factory method reads better than a clone() nobody can call. And the class is not final, which the primary defence above relies on: any subclass inherits Cloneable, and while it cannot skip this gate (its own super.clone() runs this method, and the fields are private), it can widen the access. Prefer a factory method on a final class unless the clone() hook is genuinely what a framework calls.

Testing

Assert the structural property directly, which is the check that holds on any JDK:
```
assertFalse(Cloneable.class.isAssignableFrom(SecureCredentials.class));
assertTrue(Modifier.isFinal(SecureCredentials.class.getModifiers()));
```
Reaching for Object.clone() reflectively instead does not work on a modern JDK: Object.clone() is protected, so setAccessible(true) on it throws InaccessibleObjectException - "module java.base does not \"opens java.lang\" to unnamed module" - and the test fails before it can assert anything about cloning. Running with --add-opens java.base/java.lang=ALL-UNNAMED does reach the call, but then CloneNotSupportedException arrives wrapped in an InvocationTargetException and has to be unwrapped. Both were measured on JDK 26. The two assertions above need neither. - For classes where cloning is intentionally supported, test that an unauthorized caller's clone attempt is rejected, and that a successful clone's mutable fields are independent objects from the original - assert clone.getSecretKey() != original.getSecretKey() by reference, not by contents, since a shallow copy compares equal either way. - Do not reach for SpotBugs' CN_IMPLEMENTS_CLONE_BUT_NOT_CLONEABLE here. It detects the opposite arrangement - a class that defines a clone() method without implementing Cloneable - so it stays silent on exactly the class this CWE is about, one that implements Cloneable and holds secrets. The CN_ family is about clone() idiom correctness, not about what the class contains; no shipped rule in it knows which of your classes hold credentials, which is why the structural assertion above is written per class.