fix(sdk): harden ZipReader central directory parsing against hostile fields (DSPX-5103)

[dmihalcik-virtru]

Jira: https://virtru.atlassian.net/browse/DSPX-5103. This is the Java counterpart of DSPX-5102 / opentdf/platform#4202.

Stacked on #398 (DSPX-4589-02-zip-format). Retarget this PR to main after #398 merges.

Summary

A hostile archive could hang SDK.isTDF / TDF.loadTDF, or make them throw unchecked exceptions that callers screening untrusted input don't catch. #398 already reads the central directory fields unsigned and bounds-checks the EOCD, zip64 locator, CD offset and entry count. This PR closes the remaining gaps:

1. Extra-field loop (DoS). The loop now walks the extra field record by record, and every step moves at least one 4-byte header forward. A record whose data size runs past the extra field is rejected with InvalidZipException, and so is a zip64 record too short for the values its sentinels promise. On main, a data size of 0xFFFC sign-extends to -4, and the loop spins forever on it. Fewer than 4 trailing bytes are ignored, as Go's archive/zip does.
2. Negative or out-of-range sizes and offsets. Zip64 sizes and offsets above Long.MAX_VALUE are now rejected. Before, they read as negative, so position(-1) threw IllegalArgumentException or a size of -1 read the manifest as "". Also rejected: an extra field or file comment that runs past the central directory, and an entry whose local header offset or compressed size doesn't fit in it. The uncompressed size is still checked lazily (fix(sdk): DSPX-4589 zip64 EOCD sentinels, truncated archive detection, and UTF-8 entry names #398 reports a short entry as truncated).
   • Central directory extent. Each central directory header is now bounded by the directory size the end of central directory record declares (plain or zip64), not by the end of the archive. A directory that runs into the record after it is rejected, and the entry count is bounded by the declared size. Before, an inflated filename length on the last header read the end of central directory record into the entry name. Every SDK release records this size exactly, so archives it wrote still read. Raised by CodeRabbit on fix(sdk): DSPX-4589 zip64 EOCD sentinels, truncated archive detection, and UTF-8 entry names #398.
3. No manifest size cap, on purpose. There is no fixed cap on manifest size because DSPX-4502 targets 50 TiB payloads, whose manifests exceed 1 GiB. A hostile declared size is handled by the archive bounds check (offset + compressed size <= channel.size()). A manifest whose declared size runs past the archive fails as truncated, so the reader never buffers more than the archive holds. Scaling memory use for huge manifests is DSPX-4584. (An earlier commit in this PR added a 10 MiB cap. A follow-up commit removes it.)
4. Typed failures.
   • TDF.loadTDF and SDK.readManifest now go through the new TDFReader.open / TDFReader.readManifest. These turn container and manifest malformation into SDK.MalformedTDFException. The underlying causes are InvalidZipException, IllegalArgumentException, Gson's JsonParseException, IllegalStateException and UnsupportedOperationException.
   • SDK.isTDF also returns false for any other RuntimeException, as a commented backstop for its boolean contract.
   • A payload whose declared size runs past the end of the archive is only detected when it is read. TDFReader.readPayloadBytes now wraps that InvalidZipException as SDK.MalformedTDFException too, as manifest() does, instead of letting the unchecked zip exception escape.
   • New additive constructor: MalformedTDFException(String, Exception).
5. Null statement fields. In an assertion statement, a JSON null for format, schema or value now reads as null. Before, format and schema threw UnsupportedOperationException, and value became the string "null". A non-string format or schema is now a JsonParseException.

Behavior change: callers of loadTDF / SDK.readManifest that caught IllegalArgumentException or Gson exceptions for bad input will now get SDK.MalformedTDFException (an SDKException). TDFReader's constructor itself is unchanged. A truncated payload now fails Reader.readPayload with SDK.MalformedTDFException. Before #398 it returned short data and failed with an IllegalStateException about the segment; with #398 alone it fails with a bare InvalidZipException.

Still open: trusting payload.isEncrypted = false (DSPX-4748) is out of scope here and has not been fixed.

Tests

• New HostileZips test helper builds stored archives by hand, with 11 hostile central-directory shapes.
• Every shape runs under a 5 s assertTimeoutPreemptively against:
  • ZipReader: InvalidZipException
  • SDK.isTDF: false
  • SDK.readManifest: MalformedTDFException
  • TDF.loadTDF: MalformedTDFException
• Valid-archive controls: the unmodified hand-built archive, and a zip64 extra field placed among other records with trailing padding.
• loadTDF tests for a manifest whose declared size runs past the archive (MalformedTDFException caused by InvalidZipException) and for invalid manifest JSON (not JSON, [], wrong types, a statement with an object format).
• ZipReaderTest: a filename running past the central directory, and a declared directory size running into the next record (plain and zip64), are both rejected. Both fail against the previous reader.
• TDFReaderTest.reportsATruncatedPayloadAsMalformed: a payload whose declared size runs past the archive fails as MalformedTDFException, caused by InvalidZipException.
• A test that reads an 11 MiB manifest whole, so a size limit can't creep back in.
• ManifestTest tests for null and non-string statement fields.
• The shapes are added as Jazzer seeds in FuzzingInputs/fuzzZipRead and FuzzingInputs/fuzzTDF, with dspx5103- names so they don't collide with the seeds chore(ci): fuzz Jazzer targets on relevant merges, tighten fuzzZipRead, add zip seeds #416 added on main.
• I checked that the new tests fail against fix(sdk): DSPX-4589 zip64 EOCD sentinels, truncated archive detection, and UTF-8 entry names #398's ZipReader: on the 0xFFFC shape isTDF returns true, and the zip64 size of -1 gets through.

Checks run (locally, JDK 21, compiling to release 11)

• After rebasing onto fix(sdk): DSPX-4589 zip64 EOCD sentinels, truncated archive detection, and UTF-8 entry names #398's review follow-up, adding the payload wrap and the central directory bound: mvn -o -pl sdk test -Denforcer.skip=true -Dmaven.antrun.skip=true: 327 tests, 0 failures, 0 errors, 8 skipped. Codegen was skipped; generated sources came from an earlier build.
• mvn -pl sdk -am test -Dmaven.antrun.skip=true: 322 tests, 0 failures, 0 errors, 8 skipped (re-run after the cap was removed; the skips were already there: FIPS, E2E, etc.). Proto codegen was skipped with -Dmaven.antrun.skip=true, both times. The first time, the buf BSR was rate-limiting me (resource_exhausted). For the re-run I skipped it again without retrying buf. The generated sources from an earlier successful build were used.
• mvn -pl sdk -am test -Dtest=Fuzzing -Dmaven.antrun.skip=true (Jazzer regression mode, seeds only): 34 inputs, 0 failures, new seeds included. This ran before the cap was removed; none of the seeds depend on the cap.
• Not run: the full mvn clean verify -P coverage, actual fuzzing (-P fuzz), the cmdline/examples modules, and xtest.

[coderabbitai]

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

• Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}