From 4d28bc833edbfd32998b53403b95a69359e34fd0 Mon Sep 17 00:00:00 2001 From: Fatullayev Asadbek Date: Mon, 10 Aug 2026 09:47:17 +0500 Subject: [PATCH 1/2] srtp: read Cryptex header extension from the input buffer on unprotect srtp_cryptex_unprotect_init() read the RTP header-extension profile and length from the rtp parameter, which on the unprotect path is the output buffer. 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, and in that mode the output buffer has not been written yet when this function runs -- the first write, memcpy(rtp, srtp, enc_start), happens in the caller afterwards. Cryptex detection and the enc_start adjustment were therefore derived 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 extension length is already read from srtp, so the two paths disagreed. Read both fields from srtp instead. In-place callers are unaffected because srtp == rtp there. Add a regression test that unprotects a reference Cryptex packet into a zeroed output buffer distinct from the input. The existing not-in-place coverage copies the packet into a scratch input buffer and passes the original packet buffer as the output, so the output buffer already holds the ciphertext and the wrong-buffer read returns the right bytes by accident; that is why this was not caught. Without the fix the new test fails at offset 12 with c0 (the Cryptex profile, still ciphertext) where be (the restored plaintext profile) is expected. With the fix the full suite passes in both in-place and not-in-place modes. --- srtp/srtp.c | 4 +- test/srtp_driver.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 2 deletions(-) diff --git a/srtp/srtp.c b/srtp/srtp.c index 987e43623..0f1555798 100644 --- a/srtp/srtp.c +++ b/srtp/srtp.c @@ -244,7 +244,7 @@ static srtp_err_status_t srtp_cryptex_unprotect_init( size_t *enc_start) { if (stream->use_cryptex && hdr->x == 1) { - uint16_t profile = srtp_get_rtp_hdr_xtnd_profile(hdr, rtp); + uint16_t profile = srtp_get_rtp_hdr_xtnd_profile(hdr, srtp); *inuse = profile == cryptex_one_byte_profile || profile == cryptex_two_byte_profile; } else { @@ -255,7 +255,7 @@ static srtp_err_status_t srtp_cryptex_unprotect_init( if (*inuse) { *enc_start -= - (srtp_get_rtp_hdr_xtnd_len(hdr, rtp) - octets_in_rtp_xtn_hdr); + (srtp_get_rtp_hdr_xtnd_len(hdr, srtp) - octets_in_rtp_xtn_hdr); if (*inplace) { *enc_start -= (hdr->cc * 4); } diff --git a/test/srtp_driver.c b/test/srtp_driver.c index 15a0850de..bd2b59c5e 100644 --- a/test/srtp_driver.c +++ b/test/srtp_driver.c @@ -145,6 +145,7 @@ srtp_err_status_t srtp_test_set_sender_roc(void); srtp_err_status_t srtp_test_cryptex_csrc_but_no_extension_header(void); srtp_err_status_t srtp_test_cryptex_disable(void); +srtp_err_status_t srtp_test_cryptex_not_in_place_distinct_buffer(void); srtp_err_status_t srtp_test_missing_session_keys(void); @@ -1002,6 +1003,15 @@ int main(int argc, char *argv[]) exit(1); } + printf("testing cryptex_not_in_place_distinct_buffer()..."); + if (srtp_test_cryptex_not_in_place_distinct_buffer() == + srtp_err_status_ok) { + printf("passed\n"); + } else { + printf("failed\n"); + exit(1); + } + printf("testing missing session keys handling()..."); if (srtp_test_missing_session_keys() == srtp_err_status_ok) { printf("passed\n"); @@ -3381,6 +3391,91 @@ srtp_err_status_t srtp_validate_cryptex(void) return srtp_err_status_ok; } +/* + * srtp_test_cryptex_not_in_place_distinct_buffer() unprotects a Cryptex + * (RFC 9335) packet using the not-in-place form of the API, with an output + * buffer that is genuinely distinct from the input buffer and does not + * already contain a copy of the ciphertext. + * + * srtp_unprotect() documents that rtp "can be the same as srtp to support + * in-place io", so a separate buffer is a supported calling mode. The + * existing not-in-place coverage in this driver copies the packet into a + * scratch input buffer and passes the original packet buffer as the output, + * so the output buffer happens to hold the ciphertext already. That masks + * any read of the header extension from the output buffer instead of the + * input buffer. + */ +srtp_err_status_t srtp_test_cryptex_not_in_place_distinct_buffer(void) +{ + // clang-format off + /* Plaintext packet with 1-byte header extension */ + const char *plaintext_ref = + "900f1235" + "decafbad" + "cafebabe" + "bede0001" + "51000200" + "abababab" + "abababab" + "abababab" + "abababab"; + + /* AES-CTR/HMAC-SHA1 Cryptex ciphertext of the packet above */ + const char *ciphertext_ref = + "900f1235" + "decafbad" + "cafebabe" + "c0de0001" + "eb923652" + "51c3e036" + "f8de27e9" + "c27ee3e0" + "b4651d9f" + "bc4218a7" + "0244522f" + "34a5"; + // clang-format on + + srtp_t srtp_recv; + srtp_policy_t policy; + uint8_t reference[1400]; + uint8_t ciphertext[1400]; + uint8_t output[1400]; + size_t ref_len, enc_len, out_len; + + ref_len = hex_string_to_octet_string(reference, plaintext_ref, + sizeof(reference)) / + 2; + enc_len = hex_string_to_octet_string(ciphertext, ciphertext_ref, + sizeof(ciphertext)) / + 2; + + CHECK_OK(srtp_policy_create(&policy)); + CHECK_OK(srtp_policy_set_profile(policy, srtp_profile_aes128_cm_sha1_80)); + CHECK_OK(srtp_policy_set_ssrc(policy, + (srtp_ssrc_t){ ssrc_specific, 0xcafebabe })); + CHECK_OK(policy_set_key(policy, test_key)); + CHECK_OK(srtp_policy_set_cryptex(policy, true)); + + CHECK_OK(srtp_create(&srtp_recv, policy)); + + /* + * The output buffer is deliberately not seeded with the ciphertext. A + * caller that hands libsrtp a fresh output buffer is doing nothing wrong. + */ + memset(output, 0, sizeof(output)); + out_len = sizeof(output); + + CHECK_OK(srtp_unprotect(srtp_recv, ciphertext, enc_len, output, &out_len)); + CHECK(out_len == ref_len); + CHECK_BUFFER_EQUAL(output, reference, ref_len); + + CHECK_OK(srtp_dealloc(srtp_recv)); + srtp_policy_destroy(policy); + + return srtp_err_status_ok; +} + srtp_err_status_t srtp_test_cryptex_csrc_but_no_extension_header(void) { // clang-format off From 76776dd3d957457b543e859d2e007dbbd0393693 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pascal=20B=C3=BChler?= Date: Thu, 13 Aug 2026 09:30:19 +0200 Subject: [PATCH 2/2] fix formmating --- test/srtp_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/srtp_driver.c b/test/srtp_driver.c index bd2b59c5e..8f1e9aa92 100644 --- a/test/srtp_driver.c +++ b/test/srtp_driver.c @@ -3444,7 +3444,7 @@ srtp_err_status_t srtp_test_cryptex_not_in_place_distinct_buffer(void) size_t ref_len, enc_len, out_len; ref_len = hex_string_to_octet_string(reference, plaintext_ref, - sizeof(reference)) / + sizeof(reference)) / 2; enc_len = hex_string_to_octet_string(ciphertext, ciphertext_ref, sizeof(ciphertext)) /