[Asterisk-code-review] res rtp asterisk: Resolve 2 discrete memory leaks in DTLS (asterisk[master])

[Matt Jordan]

Matt Jordan has submitted this change and it was merged.

Change subject: res_rtp_asterisk: Resolve 2 discrete memory leaks in DTLS
......................................................................


res_rtp_asterisk: Resolve 2 discrete memory leaks in DTLS

ao2 ref leak in res_rtp_asterisk.c when a DTLS policy is created.
The resources are linked into a table, but the original alloc refs
are never released. ast_strdup leak in rtp_engine.c. If
ast_rtp_dtls_cfg_copy() is called twice on the same destination struct,
a pointer to an alloc'd string is overwritten before the string is free'd.

ASTERISK-25022
Reported by: one47

Change-Id: I62a8ceb8679709f6c3769136dc6aa9a68202ff9b
---
M main/rtp_engine.c
M res/res_rtp_asterisk.c
2 files changed, 6 insertions(+), 2 deletions(-)

Approvals:
  Matt Jordan: Looks good to me, approved; Verified
  Joshua Colp: Looks good to me, but someone else must approve



diff --git a/main/rtp_engine.c b/main/rtp_engine.c
index 34e6a94..9c9ad4e 100644
--- a/main/rtp_engine.c
+++ b/main/rtp_engine.c
@@ -1659,6 +1659,8 @@
 
 void ast_rtp_dtls_cfg_copy(const struct ast_rtp_dtls_cfg *src_cfg, struct ast_rtp_dtls_cfg *dst_cfg)
 {
+	ast_rtp_dtls_cfg_free(dst_cfg);         /* Prevent a double-call leaking memory via ast_strdup */
+
 	dst_cfg->enabled = src_cfg->enabled;
 	dst_cfg->verify = src_cfg->verify;
 	dst_cfg->rekey = src_cfg->rekey;
diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c
index 0513c11..62601dc 100644
--- a/res/res_rtp_asterisk.c
+++ b/res/res_rtp_asterisk.c
@@ -1869,6 +1869,7 @@
 	unsigned char *local_key, *local_salt, *remote_key, *remote_salt;
 	struct ast_srtp_policy *local_policy, *remote_policy = NULL;
 	struct ast_rtp_instance_stats stats = { 0, };
+	int res = -1;
 
 	/* If a fingerprint is present in the SDP make sure that the peer certificate matches it */
 	if (rtp->dtls_verify & AST_RTP_DTLS_VERIFY_FINGERPRINT) {
@@ -1983,16 +1984,17 @@
 		}
 	}
 
-	return 0;
+	res = 0;
 
 error:
+	/* policy->destroy() called even on success to release local reference to these resources */
 	res_srtp_policy->destroy(local_policy);
 
 	if (remote_policy) {
 		res_srtp_policy->destroy(remote_policy);
 	}
 
-	return -1;
+	return res;
 }
 #endif
 

-- 
To view, visit https://gerrit.asterisk.org/270
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I62a8ceb8679709f6c3769136dc6aa9a68202ff9b
Gerrit-PatchSet: 4
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Steve Davies <steve at one47.co.uk>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Steve Davies <steve at one47.co.uk>