srtp: read Cryptex header extension from the input buffer on unprotect

[Kimdir01]

Follow-up to the report I sent to libsrtp-security@lists.packetizer.com on 2026-08-03. Pascal asked for a public PR with a small failing test, so here it is. This is a correctness bug, not a security issue, and I am not requesting a CVE for it.

What is wrong

srtp_cryptex_unprotect_init() reads the RTP header-extension profile_specific and length fields from the rtp parameter. On the unprotect path rtp is the output buffer:

static srtp_err_status_t srtp_cryptex_unprotect_init(
    const srtp_stream_ctx_t *stream,
    const srtp_hdr_t *hdr,
    const uint8_t *srtp,      /* input, and what hdr points into */
    const uint8_t *rtp,       /* output */
    ...
{
    if (stream->use_cryptex && hdr->x == 1) {
        uint16_t profile = srtp_get_rtp_hdr_xtnd_profile(hdr, rtp);   /* srtp/srtp.c:247 */

srtp_unprotect() documents that rtp "can be the same as srtp to support in-place io", so a distinct output buffer is a supported calling mode. In that mode the output buffer has not been written when this function runs — the first write is memcpy(rtp, srtp, enc_start) in the caller, at srtp/srtp.c:2346, after the call returns.

So Cryptex detection, and the enc_start adjustment derived from the extension length, come from whatever the caller's output buffer happened to contain rather than from the packet that arrived.

Two lines above the call site the same field is already read from srtp:

    enc_start = srtp_get_rtp_hdr_len(hdr);
    if (hdr->x == 1) {
        enc_start += srtp_get_rtp_hdr_xtnd_len(hdr, srtp);   /* :2304 — reads srtp */
    }
    status = srtp_cryptex_unprotect_init(stream, hdr, srtp, rtp, ...);  /* :2308 → reads rtp */

Both call sites (srtp_unprotect_aead and srtp_unprotect) behave the same way.

The fix

Read both fields from srtp. In-place callers are unaffected, since srtp == rtp for them. srtp_cryptex_protect_init() is left alone: on the protect path rtp is the input, so its use of rtp is correct.

Why the existing tests did not catch it

call_srtp_unprotect2() in test/srtp_driver.c, under use_srtp_not_in_place_io_api, does:

memcpy(in_buf, srtp, srtp_len);
status = srtp_unprotect(ctx, in_buf, srtp_len, srtp, rtp_len);

The input is a scratch copy and the output is the original packet buffer — which still holds the ciphertext. Reading the profile from the output buffer therefore returns the correct bytes by accident, and srtp_driver -n passes.

The test

srtp_test_cryptex_not_in_place_distinct_buffer() unprotects the existing 1-byte-header-extension reference Cryptex packet into a zeroed buffer that is distinct from the input, which is what a caller handing libsrtp a fresh output buffer would do.

Before the fix:

testing cryptex_not_in_place_distinct_buffer()...
error at test/srtp_driver.c:3471, buffer1 != buffer2 at index: 12 (c0 != be)

Offset 12 is the extension profile: the output still carries c0de (Cryptex, i.e. the ciphertext form) where bede is expected, because Cryptex was never detected and the header extension was never restored.

After the fix:

testing cryptex_not_in_place_distinct_buffer()...passed

Verification

• ctest: 12/12 pass, including srtp_driver and srtp_driver_not_in_place_io
• srtp_driver -v and srtp_driver -v -n: both exit 0 with no failures
• New test fails before the change and passes after
• Added lines are within 80 columns

I could not run clang-format — it is not available on this machine — so if CI flags formatting, tell me and I will fix it up.

Happy to extend the test to the 2-byte and CSRC vectors, or to fold it into the existing srtp_validate_cryptex() loop instead of a standalone test, if you would prefer either shape.

[pabuhler]

Thank you for reporting and fixing this.

There is a single white space needed for the format check, if you make that change then this ready for merging. (If you like I can also fix that).

I think it is fine to have a separate test at this point. I have made a note to change call_srtp_unprotect2() in such away that this would have been caught earlier.