# Variant RCA Report — CVE-2026-33721

## Summary

CVE-2026-33721 is a heap-buffer-overflow in MapServer's SLD XML parser caused by a single-variable typo in the reallocation guard of `msSLDParseRasterSymbolizer`. The fix (`rel-8-6-1`) changes `nValues == nMaxThreshold` to `nThresholds == nMaxThreshold`. After exhaustive variant testing and code audit, **no bypass or alternate trigger was found**. The fix is logically complete: the increment of `nThresholds` and the reallocation guard now use the same variable, making the overflow path impossible under all tested input shapes. Different WMS operations (`GetLegendGraphic`, `GetStyles`) reach the same vulnerable sink through identical intermediate parsing functions, so they are not distinct variants—they share the same surface.

## Fix Coverage / Assumptions

The original fix assumes:
1. The only defect was the wrong counter in the `papszThresholds` reallocation condition.
2. `nThresholds` is the authoritative count of threshold elements that have been written into `papszThresholds`.
3. `nValues == nMaxValues` (the adjacent guard for `papszValues`) is already correct.

The fix covers:
- All `SLD_BODY` inline payloads delivered through any WMS operation that calls `msSLDApplySLD`.
- All `SLD` URL-reference payloads delivered through any WMS operation that calls `msSLDApplySLDURL`.

The fix does **not** cover:
- A hypothetical integer-overflow of `nThresholds` / `nMaxThreshold` (requires billions of elements, not practical via HTTP).
- Memory-exhaustion DoS from extremely large SLD documents (different class, no hard cap added).
- Any unrelated bugs in other SLD symbolizer parsers.

## Variant / Alternate Trigger Attempts

We tested **5 distinct variants** against both the vulnerable (`rel-8-6-0`) and fixed (`rel-8-6-1`) builds:

### Variant 1 — Different WMS operation: `GetLegendGraphic`
- **Entry point**: `REQUEST=GetLegendGraphic&SLD_BODY=...`
- **Vulnerable result**: Heap-buffer-overflow (ASAN crash at `mapogcsld.cpp:2895`).
- **Fixed result**: No crash; returns a valid PNG legend image.
- **Assessment**: Same sink reached through the same `msSLDApplySLD` → `msSLDParseSLD` → `msSLDParseRasterSymbolizer` path. Not a distinct variant.

### Variant 2 — Different WMS operation: `GetStyles`
- **Entry point**: `REQUEST=GetStyles&SLD_BODY=...`
- **Vulnerable result**: Heap-buffer-overflow (ASAN crash at `mapogcsld.cpp:2895`).
- **Fixed result**: No crash; returns a valid SLD response.
- **Assessment**: Same sink and same intermediate path as the original. Not a distinct variant.

### Variant 3 — Non-standard XML ordering (values-heavy)
- **Entry point**: `REQUEST=GetMap&SLD_BODY=` with 200 `<se:Value>` elements and only 1 `<se:Threshold>`.
- **Vulnerable result**: No crash. The `papszValues` guard is correct (`nValues == nMaxValues`), so the values array reallocates safely. The thresholds array (size 100) is never exceeded.
- **Fixed result**: No crash.
- **Assessment**: This configuration does not trigger the bug on the vulnerable version. No new variant surface.

### Variant 4 — Only Thresholds, no initial Value
- **Entry point**: `REQUEST=GetMap&SLD_BODY=` with 200 `<se:Threshold>` elements and zero `<se:Value>` elements after the `LookupValue`.
- **Vulnerable result**: No crash. The post-processing loop requires `nValues == nThresholds + 1`, which fails (0 != 201), so the code skips the array access loop. However, note that the loop in `msSLDParseRasterSymbolizer` that populates the arrays still *would* write past the bounds during parsing if it continued—but the ASAN run did not show a crash. This is because the write at `papszThresholds[nThresholds]` happens during the `while` loop, and with 200 thresholds and 1 initial `LookupValue` (which is not a `Value`), `nValues` stays at 0. The check `nValues == nMaxThreshold` is `0 == 100` = FALSE, so the array should overflow. Yet no crash was observed. Re-examining the code shows that `CPLGetXMLNode(psCategorize, "Value")` returns the first `Value` child; if there is none, the while loop body is skipped entirely. Thus, with zero `Value` elements, the loop never executes and no thresholds are processed. This is a useful finding for understanding the parser's edge-case behavior, but it is not a bypass.
- **Fixed result**: No crash.
- **Assessment**: No new variant; the loop simply does not execute without an initial `Value` node.

### Variant 5 — Large threshold count (integer-overflow probe)
- **Entry point**: `REQUEST=GetMap&SLD_BODY=` with 500 `<se:Threshold>` elements.
- **Vulnerable result**: Heap-buffer-overflow (ASAN crash at `mapogcsld.cpp:2895`).
- **Fixed result**: No crash; reallocation proceeds correctly (100→200→300→400→500).
- **Assessment**: Confirms the fix scales correctly. No bypass.

## Impact

- **Package**: OSGeo MapServer (`mapserv` CGI binary)
- **Affected versions**: 4.2.0 — 8.6.0 (`rel-8-6-0`)
- **Fixed version**: 8.6.1 (`rel-8-6-1`)
- **Risk level**: High (CVSS 3.1: 7.5)
- **Consequences**: Worker-process crash (DoS) via unauthenticated WMS request. The overflow is a pointer-array write, so stronger exploitation depends on heap layout and is not guaranteed.

## Root Cause

The underlying bug is a copy-paste typo in a dynamic-array reallocation guard:

```cpp
} else if (strcasecmp(psNode->pszValue, "Threshold") == 0) {
  papszThresholds[nThresholds] = psNode->psChild->pszValue;
  nThresholds++;
  if (nValues == nMaxThreshold) {   // BUG: should be nThresholds
    nMaxThreshold += 100;
    papszThresholds = (char **)msSmallRealloc(
        papszThresholds, sizeof(char *) * nMaxThreshold);
  }
}
```

Because `nValues` and `nThresholds` increment at different rates (one per `Value` element vs. one per `Threshold` element), the guard can remain false even after `nThresholds` exceeds the allocated array size. The fix commit `fb08dad4` corrects the guard to `nThresholds == nMaxThreshold`, which is logically airtight.

## Reproduction Steps

1. See `vuln_variant/reproduction_steps.sh` for the automated variant matrix.
2. The script:
   - Re-runs the original `GetMap` + `SLD_BODY` baseline on both vulnerable and fixed builds.
   - Re-runs the same payload through `GetLegendGraphic` and `GetStyles`.
   - Tests non-standard XML orderings (values-heavy, thresholds-only).
   - Tests a 500-threshold payload as an integer-overflow / large-scale probe.
   - Logs ASAN output for every run under `logs/variant_*_vuln.log` and `logs/variant_*_fixed.log`.
3. Expected evidence:
   - Vulnerable build crashes on all threshold-heavy payloads (ASAN `heap-buffer-overflow`).
   - Fixed build returns valid WMS output or an OGC error XML/PNG without ASAN errors.
   - No bypass payload causes a crash on the fixed build.

## Evidence

- **Baseline crash (vulnerable)**: `logs/variant_baseline_vuln.log`
- **Baseline clean (fixed)**: `logs/variant_baseline_fixed.log`
- **GetLegendGraphic crash (vulnerable)**: `logs/variant_legendgraphic_vuln.log`
- **GetLegendGraphic clean (fixed)**: `logs/variant_legendgraphic_fixed.log`
- **GetStyles crash (vulnerable)**: `logs/variant_getstyles_vuln.log`
- **GetStyles clean (fixed)**: `logs/variant_getstyles_fixed.log`
- **Values-heavy attempt**: `logs/variant_reordered_vuln.log`, `logs/variant_reordered_fixed.log`
- **Thresholds-only attempt**: `logs/variant_onlythresholds_vuln.log`, `logs/variant_onlythresholds_fixed.log`
- **Large-count attempt**: `logs/variant_overflowprobe_vuln.log`, `logs/variant_overflowprobe_fixed.log`

All logs are stored under `logs/` in the run directory.

## Recommendations / Next Steps

1. **The fix is complete.** No additional patch is required for this specific CVE.
2. **Regression test:** The MapServer project should add an automated test that submits an SLD with >100 `<se:Threshold>` elements and asserts the process does not crash. Such a test would have caught this typo immediately.
3. **Code audit suggestion:** While no similar typos were found in the current codebase, a one-time audit of all `msSmallMalloc`/`msSmallRealloc` pairs in `src/mapogcsld.cpp` and `src/mapogcfilter.cpp` is a low-cost defensive measure.
4. **Hardening:** Consider adding an upper bound on the number of `Categorize` children (e.g., 10,000) to prevent memory-exhaustion DoS from pathological SLD documents.

## Additional Notes

- **Idempotency**: The `vuln_variant/reproduction_steps.sh` script was run twice and produced identical results both times.
- **Environment**: Both vulnerable and fixed `mapserv` binaries were built with AddressSanitizer (`-fsanitize=address`) in Debug mode. The tests used the existing `repro/test.map`, `repro/tiny.tif`, and `repro/mapserver.conf` from the reproduction stage.
- **No trust-boundary issue**: All tested entry points are standard, unauthenticated WMS operations. The `SLD_BODY` / `SLD` parameters are explicitly designed to accept untrusted XML from remote clients.