<p>Sean Bright has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/13369">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">res_pjsip_registrar.c: Prevent potential double free if AOR is not found<br><br>The simple fix here is simply to NULL out username and password after we call<br>ast_free on them. Unfortunately, I noticed that we weren't checking for<br>allocation failures for username and password, and adding those checks made<br>things noisy and cumbersome.<br><br>So instead we partially rollback the recent LGTM patch, and move the alloca<br>calls into find_aor_name().<br><br>ASTERISK-28641 #close<br>Reported by: Ross Beer<br><br>Change-Id: Ic9d01624e717a020be0b0aee31f0814e7f1ffbe2<br>---<br>M res/res_pjsip_registrar.c<br>1 file changed, 22 insertions(+), 21 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/69/13369/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/res/res_pjsip_registrar.c b/res/res_pjsip_registrar.c</span><br><span>index af9aa4d..75db9f1 100644</span><br><span>--- a/res/res_pjsip_registrar.c</span><br><span>+++ b/res/res_pjsip_registrar.c</span><br><span>@@ -960,14 +960,21 @@</span><br><span> return 0;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-static char *find_aor_name(const char *username, const char *domain, const char *aors)</span><br><span style="color: hsl(120, 100%, 40%);">+static char *find_aor_name(const pj_str_t *pj_username, const pj_str_t *pj_domain, const char *aors)</span><br><span> {</span><br><span> char *configured_aors;</span><br><span> char *aors_buf;</span><br><span> char *aor_name;</span><br><span> char *id_domain;</span><br><span style="color: hsl(120, 100%, 40%);">+ char *username, *domain;</span><br><span> struct ast_sip_domain_alias *alias;</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ /* Turn these into C style strings for convenience */</span><br><span style="color: hsl(120, 100%, 40%);">+ username = ast_alloca(pj_strlen(pj_username) + 1);</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_copy_pj_str(username, pj_username, pj_strlen(pj_username) + 1);</span><br><span style="color: hsl(120, 100%, 40%);">+ domain = ast_alloca(pj_strlen(pj_domain) + 1);</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_copy_pj_str(domain, pj_domain, pj_strlen(pj_domain) + 1);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> id_domain = ast_alloca(strlen(username) + strlen(domain) + 2);</span><br><span> sprintf(id_domain, "%s@%s", username, domain);</span><br><span> </span><br><span>@@ -1017,11 +1024,10 @@</span><br><span> {</span><br><span> struct ast_sip_aor *aor = NULL;</span><br><span> char *aor_name = NULL;</span><br><span style="color: hsl(0, 100%, 40%);">- char *domain_name = NULL;</span><br><span style="color: hsl(0, 100%, 40%);">- char *username = NULL;</span><br><span> int i;</span><br><span> </span><br><span> for (i = 0; i < AST_VECTOR_SIZE(&endpoint->ident_method_order); ++i) {</span><br><span style="color: hsl(120, 100%, 40%);">+ pj_str_t username;</span><br><span> pjsip_sip_uri *uri;</span><br><span> pjsip_authorization_hdr *header = NULL;</span><br><span> </span><br><span>@@ -1029,18 +1035,22 @@</span><br><span> case AST_SIP_ENDPOINT_IDENTIFY_BY_USERNAME:</span><br><span> uri = pjsip_uri_get_uri(rdata->msg_info.to->uri);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- domain_name = ast_malloc(uri->host.slen + 1);</span><br><span style="color: hsl(0, 100%, 40%);">- ast_copy_pj_str(domain_name, &uri->host, uri->host.slen + 1);</span><br><span style="color: hsl(0, 100%, 40%);">- username = ast_malloc(uri->user.slen + 1);</span><br><span style="color: hsl(0, 100%, 40%);">- ast_copy_pj_str(username, &uri->user, uri->user.slen + 1);</span><br><span style="color: hsl(120, 100%, 40%);">+ pj_strassign(&username, &uri->user);</span><br><span> </span><br><span> /*</span><br><span> * We may want to match without any user options getting</span><br><span> * in the way.</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * Logic adapted from AST_SIP_USER_OPTIONS_TRUNCATE_CHECK for pj_str_t.</span><br><span> */</span><br><span style="color: hsl(0, 100%, 40%);">- AST_SIP_USER_OPTIONS_TRUNCATE_CHECK(username);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ast_sip_get_ignore_uri_user_options()) {</span><br><span style="color: hsl(120, 100%, 40%);">+ pj_ssize_t semi = pj_strcspn2(&username, ";");</span><br><span style="color: hsl(120, 100%, 40%);">+ if (semi < pj_strlen(&username)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ username.slen = semi;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- aor_name = find_aor_name(username, domain_name, endpoint->aors);</span><br><span style="color: hsl(120, 100%, 40%);">+ aor_name = find_aor_name(&username, &uri->host, endpoint->aors);</span><br><span> if (aor_name) {</span><br><span> ast_debug(3, "Matched aor '%s' by To username\n", aor_name);</span><br><span> }</span><br><span>@@ -1049,12 +1059,8 @@</span><br><span> while ((header = pjsip_msg_find_hdr(rdata->msg_info.msg, PJSIP_H_AUTHORIZATION,</span><br><span> header ? header->next : NULL))) {</span><br><span> if (header && !pj_stricmp2(&header->scheme, "digest")) {</span><br><span style="color: hsl(0, 100%, 40%);">- username = ast_malloc(header->credential.digest.username.slen + 1);</span><br><span style="color: hsl(0, 100%, 40%);">- ast_copy_pj_str(username, &header->credential.digest.username, header->credential.digest.username.slen + 1);</span><br><span style="color: hsl(0, 100%, 40%);">- domain_name = ast_malloc(header->credential.digest.realm.slen + 1);</span><br><span style="color: hsl(0, 100%, 40%);">- ast_copy_pj_str(domain_name, &header->credential.digest.realm, header->credential.digest.realm.slen + 1);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- aor_name = find_aor_name(username, domain_name, endpoint->aors);</span><br><span style="color: hsl(120, 100%, 40%);">+ aor_name = find_aor_name(&header->credential.digest.username,</span><br><span style="color: hsl(120, 100%, 40%);">+ &header->credential.digest.realm, endpoint->aors);</span><br><span> if (aor_name) {</span><br><span> ast_debug(3, "Matched aor '%s' by Authentication username\n", aor_name);</span><br><span> break;</span><br><span>@@ -1069,9 +1075,6 @@</span><br><span> if (aor_name) {</span><br><span> break;</span><br><span> }</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- ast_free(domain_name);</span><br><span style="color: hsl(0, 100%, 40%);">- ast_free(username);</span><br><span> }</span><br><span> </span><br><span> if (ast_strlen_zero(aor_name) || !(aor = ast_sip_location_retrieve_aor(aor_name))) {</span><br><span>@@ -1079,11 +1082,9 @@</span><br><span> pjsip_endpt_respond_stateless(ast_sip_get_pjsip_endpoint(), rdata, 404, NULL, NULL, NULL);</span><br><span> ast_sip_report_req_no_support(endpoint, rdata, "registrar_requested_aor_not_found");</span><br><span> ast_log(LOG_WARNING, "AOR '%s' not found for endpoint '%s'\n",</span><br><span style="color: hsl(0, 100%, 40%);">- username ?: "", ast_sorcery_object_get_id(endpoint));</span><br><span style="color: hsl(120, 100%, 40%);">+ aor_name ?: "", ast_sorcery_object_get_id(endpoint));</span><br><span> }</span><br><span> ast_free(aor_name);</span><br><span style="color: hsl(0, 100%, 40%);">- ast_free(domain_name);</span><br><span style="color: hsl(0, 100%, 40%);">- ast_free(username);</span><br><span> return aor;</span><br><span> }</span><br><span> </span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/13369">change 13369</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/c/asterisk/+/13369"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 13 </div>
<div style="display:none"> Gerrit-Change-Id: Ic9d01624e717a020be0b0aee31f0814e7f1ffbe2 </div>
<div style="display:none"> Gerrit-Change-Number: 13369 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Sean Bright <sean.bright@gmail.com> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>