Java Security Code Review Cheat Sheet

A practical reference for reviewing Java code through a security lens — the ten threat categories that matter most, and a deep dive on one of the most damaging real-world vulnerability classes: unsafe polymorphic deserialization in Jackson.

---

10 Threat Categories for Secure Java Code Review

A secure Java code review thinks in threat categories, not files. The 10 areas that matter most:

1. Input Validation — validate at trust boundaries (HTTP, queues, DB). Use allow-lists, not block-lists. Validate type, length, format.
2. Broken Access Control — always check ownership. Never assume a resource belongs to the caller just because they are authenticated.
3. Injection (SQL, NoSQL, Command) — never concatenate user input into queries. Use parameterised queries or JPA named params.
4. Sensitive Data — never log card numbers, IBAN, tokens. Mask before logging. Encrypt PII at field level.
5. Cryptography — ban MD5/SHA-1 for security. Ban AES/ECB mode. Use AES/GCM, PBKDF2/bcrypt for passwords, SecureRandom for nonces.
6. Error Handling — return generic messages to clients. Log detail internally only. Never send stack traces to clients.
7. Deserialization — ban ObjectInputStream.readObject() on untrusted data. Ban @JsonTypeInfo(use = Id.CLASS) — see next section.
8. Dependencies — audit transitive dependencies (OWASP Dependency Check, Snyk, Dependabot). Old Jackson/Spring versions have known gadget chains.
9. Concurrency & Race Conditions — check-then-act without synchronisation is a race condition. Use DB constraints + idempotency keys for payment flows.
10. Business Logic Flaws — never trust client-supplied amounts or status. Recompute critical values server-side. Ask: can this be replayed? skipped? abused?

// ❌ Red flag: trusting raw input without validation
String id = request.getParameter("id");
repository.findById(id);

// ✅ Allow-list validation with Bean Validation
@Pattern(regexp = "[A-Z0-9_-]{1,50}")
String id;

// ❌ Broken access control — no ownership check
Payment p = paymentRepo.findById(paymentId);
return p;

// ✅ Always verify ownership
Payment p = paymentRepo.findById(paymentId);
if (!p.getClientId().equals(authenticatedClientId)) {
    throw new AccessDeniedException("Payment does not belong to caller");
}

// ❌ SQL injection via string concat
String q = "SELECT * FROM users WHERE id = " + userId;

// ✅ Parameterised JPA query
@Query("SELECT u FROM User u WHERE u.id = :id")
User findById(@Param("id") String id);

// ❌ Logging sensitive data
log.info("Payment request: {}", request); // request may contain IBAN/card

// ✅ Mask before logging
log.info("Payment request for clientId={}, amount={}", req.getClientId(), req.getAmount());

// ❌ Dangerous crypto
MessageDigest.getInstance("MD5");
Cipher.getInstance("AES/ECB/PKCS5Padding");

// ✅ Modern crypto
Cipher.getInstance("AES/GCM/NoPadding");
PasswordEncoder encoder = new BCryptPasswordEncoder();

// ❌ Leaking internals to client
return ResponseEntity.badRequest().body(e.getMessage());

// ✅ Generic client message, detailed internal log
log.error("Validation failed for clientId={}", clientId, e);
return ResponseEntity.badRequest().body("Invalid request. Contact support.");

---

Jackson Polymorphic Deserialization RCE

@JsonTypeInfo(use = Id.CLASS) tells Jackson: “the JSON payload will tell you which Java class to instantiate — trust it.” This is the core problem.

When enabled, Jackson reads a @class field from the incoming JSON and instantiates that class via reflection. An attacker can supply any fully-qualified class name on the JVM classpath. Certain classes (gadgets) have dangerous side effects during construction or property setting — TemplatesImpl being the most well-known, capable of triggering arbitrary bytecode execution.

This is not theoretical. Real CVEs (CVE-2017-7525 and many follow-ons) exploited exactly this in Jackson 2.x.

Id.MINIMAL_CLASS is equally dangerous — it just shortens the class name. enableDefaultTyping() / activateDefaultTyping() on ObjectMapper have the same effect.

Fix: use Id.NAME with an explicit @JsonSubTypes allow-list of known safe types. Or use manual dispatch (switch on a type string). Never let the payload control which class to instantiate.

// ❌ DANGEROUS — attacker controls which class Jackson instantiates
@JsonTypeInfo(use = JsonTypeInfo.Id.CLASS)
public abstract class Event {}

// Attacker sends:
// { "@class": "com.sun.org.apache.xalan.internal.xsltc.trax.TemplatesImpl",
//   "bytecodes": ["<malicious>"], "transletName": "evil" }
// Jackson instantiates TemplatesImpl → triggers RCE

// ❌ Also dangerous — same attack surface, shorter name
@JsonTypeInfo(use = JsonTypeInfo.Id.MINIMAL_CLASS)

// ❌ Also dangerous on ObjectMapper level
objectMapper.enableDefaultTyping(); // deprecated but still dangerous
objectMapper.activateDefaultTyping(LaissezFaireSubTypeValidator.instance);

// ✅ SAFE — explicit allow-list, attacker cannot inject arbitrary classes
@JsonTypeInfo(
    use = JsonTypeInfo.Id.NAME,
    include = JsonTypeInfo.As.PROPERTY,
    property = "type"
)
@JsonSubTypes({
    @JsonSubTypes.Type(value = PaymentEvent.class, name = "payment"),
    @JsonSubTypes.Type(value = RefundEvent.class,  name = "refund"),
    @JsonSubTypes.Type(value = ChargebackEvent.class, name = "chargeback")
})
public abstract class Event {}

// JSON the attacker sends: { "type": "hacked" } → Jackson returns null, no gadget

// ✅ Safest — manual dispatch, zero reflection risk
public Event deserialize(String json) throws IOException {
    JsonNode node = mapper.readTree(json);
    return switch (node.get("type").asText()) {
        case "payment"    -> mapper.treeToValue(node, PaymentEvent.class);
        case "refund"     -> mapper.treeToValue(node, RefundEvent.class);
        default           -> throw new IllegalArgumentException("Unknown event type");
    };
}