<p>Joshua Colp <strong>merged</strong> this change.</p><p><a href="https://gerrit.asterisk.org/7015">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Corey Farrell: Looks good to me, but someone else must approve
Sean Bright: Looks good to me, but someone else must approve
Joshua Colp: Looks good to me, approved; Approved for Submit
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">Fix ast_(v)asprintf() malloc failure usage conditions.<br><br>When (v)asprintf() fails, the state of the allocated buffer is undefined.<br>The library had better not leave an allocated buffer as a result or no one<br>will know to free it. The most likely way it can return failure is for an<br>allocation failure. If the printf conversion fails then you actually have<br>a threading problem which is much worse because another thread modified<br>the parameter values.<br><br>* Made __ast_asprintf()/__ast_vasprintf() set the returned buffer to NULL<br>on failure. That is much more useful than either an uninitialized pointer<br>or a pointer that has already been freed. Many uses won't have to check<br>for failure to ensure that the buffer won't be double freed or prevent an<br>attempt to free an uninitialized pointer.<br><br>* stasis.c: Fixed memory leak in multi_object_blob_to_ami() allocated by<br>ast_asprintf().<br><br>* ari/resource_bridges.c:ari_bridges_play_helper(): Remove assignment to<br>the wrong thing which is now not needed even if assigning to the right<br>thing.<br><br>Change-Id: Ib5252fb8850ecf0f78ed0ee2ca0796bda7e91c23<br>---<br>M apps/app_mixmonitor.c<br>M bridges/bridge_softmix.c<br>M include/asterisk/utils.h<br>M main/db.c<br>M main/json.c<br>M main/stasis.c<br>M main/utils.c<br>M res/ari/resource_bridges.c<br>M res/res_xmpp.c<br>9 files changed, 31 insertions(+), 13 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/apps/app_mixmonitor.c b/apps/app_mixmonitor.c<br>index 2922e0c..bb47cfc 100644<br>--- a/apps/app_mixmonitor.c<br>+++ b/apps/app_mixmonitor.c<br>@@ -813,6 +813,8 @@<br> <br> if (ast_asprintf(datastore_id, "%p", mixmonitor_ds) == -1) {<br> ast_log(LOG_ERROR, "Failed to allocate memory for MixMonitor ID.\n");<br>+ ast_free(mixmonitor_ds);<br>+ return -1;<br> }<br> <br> ast_mutex_init(&mixmonitor_ds->lock);<br>diff --git a/bridges/bridge_softmix.c b/bridges/bridge_softmix.c<br>index 9197df6..f490967 100644<br>--- a/bridges/bridge_softmix.c<br>+++ b/bridges/bridge_softmix.c<br>@@ -502,7 +502,6 @@<br> <br> if (ast_asprintf(&stream_clone_name, "%s_%s_%s", SOFTBRIDGE_VIDEO_DEST_PREFIX,<br> channel_name, ast_stream_get_name(stream)) < 0) {<br>- ast_free(stream_clone_name);<br> return -1;<br> }<br> <br>diff --git a/include/asterisk/utils.h b/include/asterisk/utils.h<br>index 423d73b..0a12b1d 100644<br>--- a/include/asterisk/utils.h<br>+++ b/include/asterisk/utils.h<br>@@ -622,7 +622,13 @@<br> <br> DEBUG_CHAOS_RETURN(DEBUG_CHAOS_ALLOC_CHANCE, -1);<br> <br>- if ((res = vasprintf(ret, fmt, ap)) == -1) {<br>+ res = vasprintf(ret, fmt, ap);<br>+ if (res < 0) {<br>+ /*<br>+ * *ret is undefined so set to NULL to ensure it is<br>+ * initialized to something useful.<br>+ */<br>+ *ret = NULL;<br> MALLOC_FAILURE_MSG;<br> }<br> <br>diff --git a/main/db.c b/main/db.c<br>index 9432435..fa125d7 100644<br>--- a/main/db.c<br>+++ b/main/db.c<br>@@ -191,9 +191,11 @@<br> char *cmd;<br> int res;<br> <br>- ast_asprintf(&cmd, "%s/astdb2sqlite3 '%s'\n", ast_config_AST_SBIN_DIR, ast_config_AST_DB);<br>- res = ast_safe_system(cmd);<br>- ast_free(cmd);<br>+ res = ast_asprintf(&cmd, "%s/astdb2sqlite3 '%s'\n", ast_config_AST_SBIN_DIR, ast_config_AST_DB);<br>+ if (0 <= res) {<br>+ res = ast_safe_system(cmd);<br>+ ast_free(cmd);<br>+ }<br> <br> return res;<br> }<br>diff --git a/main/json.c b/main/json.c<br>index 9004978..56df7f4 100644<br>--- a/main/json.c<br>+++ b/main/json.c<br>@@ -460,7 +460,7 @@<br> <br> if (format) {<br> int err = ast_vasprintf(&str, format, args);<br>- if (err > 0) {<br>+ if (err >= 0) {<br> ret = json_string(str);<br> ast_free(str);<br> }<br>diff --git a/main/stasis.c b/main/stasis.c<br>index a82e938..6dc5dbf 100644<br>--- a/main/stasis.c<br>+++ b/main/stasis.c<br>@@ -1342,7 +1342,7 @@<br> <br> for (type = 0; type < STASIS_UMOS_MAX; ++type) {<br> for (i = 0; i < AST_VECTOR_SIZE(&multi->snapshots[type]); ++i) {<br>- char *name = "";<br>+ char *name = NULL;<br> void *snapshot = AST_VECTOR_GET(&multi->snapshots[type], i);<br> ami_snapshot = NULL;<br> <br>@@ -1352,11 +1352,11 @@<br> <br> switch (type) {<br> case STASIS_UMOS_CHANNEL:<br>- ami_snapshot = ast_manager_build_channel_state_string_prefix(snapshot, name);<br>+ ami_snapshot = ast_manager_build_channel_state_string_prefix(snapshot, name ?: "");<br> break;<br> <br> case STASIS_UMOS_BRIDGE:<br>- ami_snapshot = ast_manager_build_bridge_state_string_prefix(snapshot, name);<br>+ ami_snapshot = ast_manager_build_bridge_state_string_prefix(snapshot, name ?: "");<br> break;<br> <br> case STASIS_UMOS_ENDPOINT:<br>@@ -1367,6 +1367,7 @@<br> ast_str_append(&ami_str, 0, "%s", ast_str_buffer(ami_snapshot));<br> ast_free(ami_snapshot);<br> }<br>+ ast_free(name);<br> }<br> }<br> <br>diff --git a/main/utils.c b/main/utils.c<br>index a73bf9d..c553207 100644<br>--- a/main/utils.c<br>+++ b/main/utils.c<br>@@ -2340,7 +2340,13 @@<br> va_list ap;<br> <br> va_start(ap, fmt);<br>- if ((res = vasprintf(ret, fmt, ap)) == -1) {<br>+ res = vasprintf(ret, fmt, ap);<br>+ if (res < 0) {<br>+ /*<br>+ * *ret is undefined so set to NULL to ensure it is<br>+ * initialized to something useful.<br>+ */<br>+ *ret = NULL;<br> MALLOC_FAILURE_MSG;<br> }<br> va_end(ap);<br>diff --git a/res/ari/resource_bridges.c b/res/ari/resource_bridges.c<br>index aab78ce..cd289aa 100644<br>--- a/res/ari/resource_bridges.c<br>+++ b/res/ari/resource_bridges.c<br>@@ -392,7 +392,6 @@<br> <br> if (ast_asprintf(playback_url, "/playbacks/%s",<br> stasis_app_playback_get_id(playback)) == -1) {<br>- playback_url = NULL;<br> ast_ari_response_alloc_failed(response);<br> return -1;<br> }<br>diff --git a/res/res_xmpp.c b/res/res_xmpp.c<br>index 8a06a6c..f683557 100644<br>--- a/res/res_xmpp.c<br>+++ b/res/res_xmpp.c<br>@@ -3909,8 +3909,11 @@<br> struct ast_json_error error;<br> RAII_VAR(struct ast_json *, jobj, NULL, ast_json_unref);<br> <br>- ast_asprintf(&cmd, "CURL(%s,client_id=%s&client_secret=%s&refresh_token=%s&grant_type=refresh_token)",<br>- url, cfg->oauth_clientid, cfg->oauth_secret, cfg->refresh_token);<br>+ if (ast_asprintf(&cmd,<br>+ "CURL(%s,client_id=%s&client_secret=%s&refresh_token=%s&grant_type=refresh_token)",<br>+ url, cfg->oauth_clientid, cfg->oauth_secret, cfg->refresh_token) < 0) {<br>+ return -1;<br>+ }<br> <br> ast_debug(2, "Performing OAuth 2.0 authentication for client '%s' using command: %s\n",<br> cfg->name, cmd);<br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/7015">change 7015</a>. To unsubscribe, 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/7015"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: Ib5252fb8850ecf0f78ed0ee2ca0796bda7e91c23 </div>
<div style="display:none"> Gerrit-Change-Number: 7015 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Richard Mudgett <rmudgett@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Corey Farrell <git@cfware.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Sean Bright <sean.bright@gmail.com> </div>