From 703f2e9b71c377624cbb03949a721b63429c279f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pascal=20B=C3=BChler?= Date: Tue, 11 Jun 2024 21:11:38 +0200 Subject: [PATCH] explicitly check dst len is >= tag size This is a safety check to ensure we do not wrap dst len when removing tag size. All of this code would get much cleaner if the tag could just be returned instead of cached. see #714 --- crypto/cipher/aes_gcm_nss.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/crypto/cipher/aes_gcm_nss.c b/crypto/cipher/aes_gcm_nss.c index c3c3647ae..db17b918b 100644 --- a/crypto/cipher/aes_gcm_nss.c +++ b/crypto/cipher/aes_gcm_nss.c @@ -337,8 +337,8 @@ static srtp_err_status_t srtp_aes_gcm_nss_encrypt(void *cv, { srtp_aes_gcm_ctx_t *c = (srtp_aes_gcm_ctx_t *)cv; - // When we get a non-NULL buffer, we know that the caller is - // prepared to also take the tag. When we get a NULL buffer, + // When we get a non-NULL src buffer, we know that the caller is + // prepared to also take the tag. When we get a NULL src buffer, // even though there's no data, we need to give NSS a buffer // where it can write the tag. We can't just use c->tag because // memcpy has undefined behavior on overlapping ranges. @@ -359,6 +359,10 @@ static srtp_err_status_t srtp_aes_gcm_nss_encrypt(void *cv, return status; } + if (*dst_len < c->tag_size) { + return srtp_err_status_bad_param; + } + memcpy(c->tag, non_null_dst_buf + (*dst_len - c->tag_size), c->tag_size); *dst_len -= c->tag_size; return srtp_err_status_ok;