Upload Hardening: Traversal, Deserialization, and XXE

The vulnerability is that user-controlled text is joined to a base path without normalization, so .. segments escape. Paths.get(base, name) performs no check; and if name is absolute, resolve/get discards the base entirely.

The strongest fix is to stop using the client's name as the storage path at all:

private static final Path BASE = Paths.get("/srv/docs").toAbsolutePath().normalize();

String id = UUID.randomUUID().toString();
Path target = BASE.resolve(id + ".bin").normalize();
if (!target.startsWith(BASE)) throw new IllegalArgumentException("invalid target");
try (OutputStream os = Files.newOutputStream(target, StandardOpenOption.CREATE_NEW)) {
    part.getInputStream().transferTo(os);
}

Store the client filename as metadata in the database and encode/escape it on display; never trust it for I/O.

Points a reviewer will check:

• normalize() must happen before startsWith, and use Path.startsWith (element-wise) rather than String.startsWith - the string form would wrongly accept /srv/docs-evil/x.
• CREATE_NEW refuses to reuse an existing path, which also blocks a pre-planted symlink at the target; Files.copy without that guard can overwrite an existing file.
• toRealPath() catches symlinked parents but requires the path to exist and introduces a TOCTOU window; combine it with NOFOLLOW_LINKS where a pre-existing path is involved.
• Enforce a size limit, an extension/content-type allowlist, and reject control characters and NUL in the name; store uploads outside the web root with no execute permission.

Java native deserialization instantiates whatever classes the stream names, before any application-level type check. With a gadget library on the classpath, an attacker can chain readObject implementations so that merely calling readObject() executes arbitrary code - the classic gadget-chain remote code execution. Even without a gadget, a small hostile stream can cause denial of service via deeply nested graphs, enormous arrays, or hash-collision payloads.

Mitigation, in order of preference:

1. Do not deserialize untrusted data. Replace the format with JSON or another schema-based format and bind to an explicit DTO with a fixed field set. This removes the entire class of bug rather than filtering it.
2. If native serialization must stay, add a JEP 290 filter before the first read:

ObjectInputFilter filter = ObjectInputFilter.Config.createFilter(
    "com.acme.DocumentMeta;java.base/*;!*;maxdepth=20;maxrefs=1000;maxbytes=1000000;maxarray=10000");
ObjectInputStream ois = new ObjectInputStream(in);
ois.setObjectInputFilter(filter);
DocumentMeta meta = (DocumentMeta) ois.readObject();

Rules are evaluated in order, so the reject-all !* must come last. The filter is consulted for every class in the graph and for array sizes and graph depth, which is what stops the DoS variants as well as the gadget. 3. Add a process-wide backstop with -Djdk.serialFilter=... or the jdk.serialFilter security property, so any stream created elsewhere in the application is still constrained. 4. Defence in depth: override resolveClass to allowlist local classes only, validate fields after deserialization, and keep the gadget library off the web application's classpath if it is not needed there.

A filter set after readObject has already returned is worthless - the damage occurs during the read, not in the returned object.

Predictable tokens. java.util.Random is a 48-bit linear congruential generator. It is thread-safe and fine for jitter or sampling, but it is not a cryptographic generator: observing a handful of outputs allows an attacker to recover the internal state and compute all past and future values, and the effective search space is far too small for a reset token. Use SecureRandom:

byte[] raw = new byte[32];
SecureRandom.getInstanceStrong().nextBytes(raw);   // or new SecureRandom()
String token = Base64.getUrlEncoder().withoutPadding().encodeToString(raw);

getInstanceStrong() can block on entropy-starved systems; new SecureRandom() (NativePRNG, non-blocking) is usually the pragmatic choice. Store only a SHA-256 hash of the token with an expiry and a single-use flag, and compare with MessageDigest.isEqual to avoid timing leaks.

XXE. DocumentBuilderFactory.newInstance() enables DOCTYPE declarations and external entity resolution by default, so the parser fetched file:///etc/passwd and substituted it into the title. Fix by disabling DTDs and external entities, then failing closed:

DocumentBuilderFactory f = DocumentBuilderFactory.newInstance();
f.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
f.setFeature("http://xml.org/sax/features/external-general-entities", false);
f.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
f.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
f.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
f.setXIncludeAware(false);
f.setExpandEntityReferences(false);

Apply the same hardening to SAX, StAX and TransformerFactory (ACCESS_EXTERNAL_DTD and ACCESS_EXTERNAL_STYLESHEET set to the empty string), since a hardened DOM factory does not protect the other parsers. Wrap setFeature in a check that fails startup if the feature is unsupported rather than silently continuing with defaults.