kunwaratulhax0r opened a new pull request, #74:
URL: https://github.com/apache/apr/pull/74
## Summary
Two bugs in Compact Serialization JWE decoding, both in
`jose/apr_jose_decode.c` / `jose/apr_jose.c`:
**1. NULL pointer dereference in `apr_jose_jwe_make()` (`jose/apr_jose.c`)**
```c
jose->cty = payload->cty;
```
`payload` is dereferenced unconditionally, but it's documented and declared
as optional — the function's own `__attribute__((nonnull(6)))` covers only the
`pool` parameter, deliberately excluding `payload`.
`apr_jose_decode_compact_jwe()` always calls this with `payload == NULL` (no
payload object exists yet at that point in decryption), so **every call to
`apr_jose_decode()` on a JWE in Compact Serialization crashes**, for any
well-formed input, not just malformed input.
The sibling constructors `apr_jose_jws_make`, `apr_jose_jwe_json_make`, and
`apr_jose_jws_json_make` all correctly guard the identical line with `if
(payload) { ... }`. `apr_jose_jwe_make` is the only one of the four missing it.
**2. Out-of-bounds read in `apr_jose_decode_compact_jwe()`
(`jose/apr_jose_decode.c`)**
```c
left = dot + 1;
/* tag */
jwe->encryption->tag.data = apr_pdecode_base64_binary(pool, left,
dot - left, APR_ENCODE_BASE64URL, &jwe->encryption->tag.len);
```
`dot` is the delimiter found for the *previous* (ciphertext) segment, and
`left` was just set to `dot + 1` on the line above — so `dot - left` is always
exactly `-1`. Passed as the signed `slen` argument to
`apr_pdecode_base64_binary()`, `-1` is `APR_ENCODE_STRING`, which tells the
decoder to scan for a NUL terminator instead of respecting a length, reading
past the end of the real input buffer. The tag is the last segment in Compact
Serialization (no trailing delimiter), so its length should be measured against
the end of the input, `right`, not against the previous segment's delimiter.
This second bug was masked by the first — the NULL deref happens earlier in
the same call path, so the OOB read was never actually reached until bug 1 was
fixed.
## Verification
Built `apache/apr` trunk from source and reproduced both issues with a
standalone test calling `apr_jose_decode()` with a real `decrypt` callback on a
well-formed Compact JWE string:
- Before any fix: crashes (`SIGSEGV`) inside `apr_jose_jwe_make`, called
from `apr_jose_decode_compact_jwe`, before the decrypt callback is ever reached.
- After fixing bug 1 only: no crash, decrypt callback is reached, but
reports a tag length that reads past the intended segment into adjacent memory
(confirmed by placing known marker bytes after the legitimate buffer).
- After fixing both: no crash, and the reported tag length exactly matches
the actual encoded tag, with no overread.
## Scope note
Neither released APR (≤ 1.7.6) nor apr-util (≤ 1.6.5) ships the `jose`
module — it exists only on unreleased development branches (APR 2.0.0-dev on
trunk, and an apr-util `1.7.x` branch that has never had a tagged release).
This is a pre-release bug fix, not a report against any shipped version.
## Test plan
- [x] Confirmed crash reproduces on trunk before this change
- [x] Confirmed no crash after this change
- [x] Confirmed the OOB read (wrong tag length reading adjacent memory)
before the second fix
- [x] Confirmed correct tag length after the second fix
- [ ] Existing `apr_jose` test suite (if any) — please let me know if
there's a specific test target to run
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
