# Patch Analysis — CVE-2026-43825 (Apache OpenNLP SvmDoccatModel)

## Fix identity

- **Commit:** `3cf42d4a0145cefd9dd06138873a91312052c767`
- **Subject:** "OPENNLP-1823: Harden SvmDoccatModel.deserialize() with
  ObjectInputFilter and resource limits (#1029)"
- **Release tag:** `opennlp-3.0.0-M4`
- **Files changed (production):** exactly one —
  `opennlp-core/opennlp-ml/opennlp-ml-libsvm/src/main/java/opennlp/tools/ml/libsvm/doccat/SvmDoccatModel.java`
  (+ `SvmDoccatModelTest.java` for tests). `git show --stat` confirms 2 files,
  +336/-3. **No other production file in the module or sibling modules is
  touched.**

## What the fix changes (files, functions, logic)

### Before (`3cb7232e`, parent of fix)

```java
public static SvmDoccatModel deserialize(InputStream in)
    throws IOException, ClassNotFoundException {
  try (ObjectInputStream ois = new ObjectInputStream(in)) {
    return (SvmDoccatModel) ois.readObject();
  }
}
```

A bare `ObjectInputStream.readObject()` with **no filter, no null check, no
limits**. Any class on the consumer's classpath is materialised during
`readObject()`; the cast to `SvmDoccatModel` happens only afterward (the
original RCE mechanism).

### After (`3cf42d4a`)

1. `deserialize(InputStream)` now delegates to
   `deserialize(InputStream, DeserializationLimits)` with
   `DeserializationLimits.DEFAULT`.
2. New overload `deserialize(InputStream, DeserializationLimits)`:
   - null-checks `in` and `limits`;
   - creates `ObjectInputStream` and calls
     `ois.setObjectInputFilter(buildFilter(limits))` **before** `readObject()`;
   - `return (SvmDoccatModel) ois.readObject();`.
3. New `record DeserializationLimits(long maxDepth, long maxRefs, long
   maxArrayLength)` with `DEFAULT = (64, 5_000_000, 10_000_000)` and
   validation that each is `> 0`.
4. New `ALLOWED_CLASSES` constant (~23 fully-qualified names) and
   `buildFilter(limits)` returning a lambda `ObjectInputFilter`:
   ```java
   return info -> {
     if (info.depth() > limits.maxDepth()
         || info.references() > limits.maxRefs()
         || info.arrayLength() > limits.maxArrayLength()) {
       return REJECTED;
     }
     Class<?> serialClass = info.serialClass();
     if (serialClass == null) return UNDECIDED;
     Class<?> componentType = serialClass;
     while (componentType.isArray()) componentType = componentType.getComponentType();
     if (componentType.isPrimitive()) return ALLOWED;          // (*)
     return ALLOWED_CLASSES.contains(componentType.getName()) ? ALLOWED : REJECTED;
   };
   ```
5. Javadoc updated to describe the filter as *"defense in depth, not a license
   to deserialize from untrusted sources"*.

`ALLOWED_CLASSES` = OpenNLP doccat types (`SvmDoccatModel`,
`SvmDoccatConfiguration`, `TermWeightingStrategy`,
`FeatureSelectionStrategy`); zlibsvm `de.hhn.mi.*` (`SvmConfigurationImpl`,
`SvmType`, `KernelType`, `SvmModelImpl`, `SvmMetaInformationImpl`,
`SvmFeatureImpl`, `SvmClassLabelImpl`); libsvm (`svm_model`, `svm_node`,
`svm_parameter`); JDK (`java.lang.String`, `Number`, `Integer`, `Double`,
`Boolean`, `Enum`, `HashMap`, `Map$Entry`).

## Target threat model / scope

- **No `SECURITY.md` or formal threat-model document exists** in the repo
  (verified by `find . -iname 'SECURITY*'`).
- The fix's own javadoc states the filter is *"defense in depth, not as a
  license to deserialize from untrusted sources: callers should still ensure
  the input stream originates from a location they trust."* The fix therefore
  positions the filter as a mitigation, not a complete guarantee, for callers
  that ingest semi-trusted model bytes.
- The CVE description and the repro RCA note that other OpenNLP modules use
  different (non-Java-object) serialization (e.g. `BaseModel`'s custom
  binary/zip format) and are out of this CVE's scope. This analysis agrees:
  `SvmDoccatModel.deserialize()` is the only `ObjectInputStream.readObject()`
  sink in `opennlp-ml-libsvm` main source; `ObjectDataReader` reads primitives
  only; `BaseModel.readObject` is OpenNLP's custom format (not raw object
  deserialisation of arbitrary classes).

## Assumptions the fix makes

1. **Foreign gadget classes are the threat; the allow-list excludes them.**
   True and verified: a foreign `MaliciousGadget` class is rejected with
   `InvalidClassException: filter status: REJECTED` on the fixed build, and no
   gadget code runs. The original CVE's RCE vector is closed.
2. **The allow-list is complete for legitimate `SvmDoccatModel` graphs** (so
   real models still round-trip). Not challenged by this variant.
3. **The numeric limits "bound pathological streams."** This is the assumption
   the variant breaks (see below).
4. **Primitive arrays are safe to allow up to `maxArrayLength`.** The line
   marked `(*)` returns `ALLOWED` for any primitive-component array based
   solely on the per-array `arrayLength` bound — it does **not** account for
   how many such arrays exist or their cumulative size.

## Code paths / inputs the fix does NOT cover

- **Cumulative memory.** The filter exposes `depth()`, `references()`, and
  `arrayLength()` — there is **no per-stream byte/allocation budget**. `n`
  primitive arrays each of length ≤ `10_000_000` are each allowed (line `(*)`)
  and each counts as ~1 reference, so a stream of `n` × `double[10_000_000]`
  passes the filter whenever `n ≤ ~5_000_000` yet allocates `n × 80 MiB`.
  `HashMap.readObject()` then allocates them all during `readObject()` →
  `OutOfMemoryError` on any realistic heap. **This is the confirmed bypass.**
- **Type-blind `HashMap` contents.** The filter checks class *names* per
  descriptor, not that a `HashMap` value conforms to the declared field type.
  An attacker can place `HashMap<String,double[]>` inside any `HashMap` field
  of a crafted `SvmDoccatModel` graph; the filter allows it and the OOM lands
  during `readObject()` (before any use-time `ClassCastException`).
- **`DeserializationLimits` are caller-supplied** (the
  `deserialize(InputStream, DeserializationLimits)` overload). A caller that
  raises limits to fit a large legitimate model widens the DoS window. The
  attacker controls the stream, not the limits, so this is a caller-side
  risk amplifier rather than an attacker-controlled bypass — noted for
  completeness.

## Is the fix complete?

**Partially.** The fix **completely closes the original CVE's RCE vector**
(foreign gadget classes are rejected; all allow-listed classes are plain
data/record/enum/struct types with no exploitable
`readObject`/`readResolve`/`writeReplace` — verified by `javap` bytecode
inspection of every allow-listed `de.hhn.mi.*`, `libsvm.*`, and OpenNLP
class). However, it **leaves a gap in its stated resource-limit goal**: total
deserialisation memory is unbounded, enabling a memory-exhaustion DoS that
reaches the same sink on the fixed build. So the fix is complete *for RCE
prevention* but incomplete *for pathological-stream bounding*.

## Behavior comparison (before vs after)

| Scenario | Before (`3cb7232e`) | After (`3cf42d4a`) |
|---|---|---|
| Foreign gadget stream (`MaliciousGadget`) | gadget `readObject()` runs → RCE; then `ClassCastException` | `InvalidClassException: filter status: REJECTED` (no execution) |
| `HashMap<String,double[8M]>` × 4 (256 MiB), small heap | `OutOfMemoryError` during `readObject()` | **`OutOfMemoryError` during `readObject()`** (filter allows it) |
| `HashMap<String,double[8M]>` × 4, large heap | `ClassCastException` after full allocation | `ClassCastException` after full allocation (filter allows it) |
| Legitimate `SvmDoccatModel` stream | deserialises | deserialises (allow-list covers the graph) |

The bottom two rows show the bypass: the filter changes **nothing** about the
memory-exhaustion outcome because the payload uses only allow-listed classes
and stays within every numeric limit.

## Recommendation for a complete fix

Bound **total deserialisation cost**, not only per-array length and reference
count (see `rca_report.md` Recommendations): add a cumulative allocation
budget (custom `ObjectInputStream` accounting or pre-scan), tighten primitive
array aggregate handling, and/or cap `HashMap` entry counts / value types.
Until then, callers must continue to treat `deserialize()` input as untrusted
— the filter is not a complete guarantee against pathological streams.