Fix ast_(v)asprintf() malloc failure usage conditions.

When (v)asprintf() fails, the state of the allocated buffer is undefined.
The library had better not leave an allocated buffer as a result or no one
will know to free it.  The most likely way it can return failure is for an
allocation failure.  If the printf conversion fails then you actually have
a threading problem which is much worse because another thread modified
the parameter values.

* Made __ast_asprintf()/__ast_vasprintf() set the returned buffer to NULL
on failure.  That is much more useful than either an uninitialized pointer
or a pointer that has already been freed.  Many uses won't have to check
for failure to ensure that the buffer won't be double freed or prevent an
attempt to free an uninitialized pointer.

* stasis.c: Fixed memory leak in multi_object_blob_to_ami() allocated by
ast_asprintf().

* ari/resource_bridges.c:ari_bridges_play_helper(): Remove assignment to
the wrong thing which is now not needed even if assigning to the right
thing.

Change-Id: Ib5252fb8850ecf0f78ed0ee2ca0796bda7e91c23

diff --git a/apps/app_mixmonitor.c b/apps/app_mixmonitor.c
index 2922e0cd37cb65362f2efb851598883a105e9ca1..bb47cfc340efdf411f2da6726a67144f5ff62939 100644 (file)
--- a/apps/app_mixmonitor.c
+++ b/apps/app_mixmonitor.c
@@ -813,6 +813,8 @@ static int setup_mixmonitor_ds(struct mixmonitor *mixmonitor, struct ast_channel
 
        if (ast_asprintf(datastore_id, "%p", mixmonitor_ds) == -1) {
                ast_log(LOG_ERROR, "Failed to allocate memory for MixMonitor ID.\n");
+               ast_free(mixmonitor_ds);
+               return -1;
        }
 
        ast_mutex_init(&mixmonitor_ds->lock);

diff --git a/bridges/bridge_softmix.c b/bridges/bridge_softmix.c
index 9197df6fb5f1a226e1748d2905048810bb234717..f490967e2aca2bc751edc1ca8057fee237c0c28c 100644 (file)
--- a/bridges/bridge_softmix.c
+++ b/bridges/bridge_softmix.c
@@ -502,7 +502,6 @@ static int append_source_streams(struct ast_stream_topology *dest,
 
                if (ast_asprintf(&stream_clone_name, "%s_%s_%s", SOFTBRIDGE_VIDEO_DEST_PREFIX,
                        channel_name, ast_stream_get_name(stream)) < 0) {
-                       ast_free(stream_clone_name);
                        return -1;
                }
 

diff --git a/include/asterisk/utils.h b/include/asterisk/utils.h
index 423d73b26a595699a1ec80e22df822f64c602c64..0a12b1d8a56c2b505cd501212c329488c858ad28 100644 (file)
--- a/include/asterisk/utils.h
+++ b/include/asterisk/utils.h
@@ -622,7 +622,13 @@ int __ast_vasprintf(char **ret, const char *fmt, va_list ap, const char *file, i
 
        DEBUG_CHAOS_RETURN(DEBUG_CHAOS_ALLOC_CHANCE, -1);
 
-       if ((res = vasprintf(ret, fmt, ap)) == -1) {
+       res = vasprintf(ret, fmt, ap);
+       if (res < 0) {
+               /*
+                * *ret is undefined so set to NULL to ensure it is
+                * initialized to something useful.
+                */
+               *ret = NULL;
                MALLOC_FAILURE_MSG;
        }
 

diff --git a/main/db.c b/main/db.c
index 94324355f2a554a425ace694f4646c9f93b67f57..fa125d749d6af3bb7751ff9ab1291cf438a6008e 100644 (file)
--- a/main/db.c
+++ b/main/db.c
@@ -191,9 +191,11 @@ static int convert_bdb_to_sqlite3(void)
        char *cmd;
        int res;
 
-       ast_asprintf(&cmd, "%s/astdb2sqlite3 '%s'\n", ast_config_AST_SBIN_DIR, ast_config_AST_DB);
-       res = ast_safe_system(cmd);
-       ast_free(cmd);
+       res = ast_asprintf(&cmd, "%s/astdb2sqlite3 '%s'\n", ast_config_AST_SBIN_DIR, ast_config_AST_DB);
+       if (0 <= res) {
+               res = ast_safe_system(cmd);
+               ast_free(cmd);
+       }
 
        return res;
 }

diff --git a/main/json.c b/main/json.c
index 9004978b467687ece8efd380bcf28a3867615695..56df7f4012c2b3481093f13182c1df1aebe527d0 100644 (file)
--- a/main/json.c
+++ b/main/json.c
@@ -460,7 +460,7 @@ struct ast_json *ast_json_vstringf(const char *format, va_list args)
 
        if (format) {
                int err = ast_vasprintf(&str, format, args);
-               if (err > 0) {
+               if (err >= 0) {
                        ret = json_string(str);
                        ast_free(str);
                }

diff --git a/main/stasis.c b/main/stasis.c
index a82e938f4fba6bcb5da3ecd0e1616a7868313d76..6dc5dbf288bb4e1cdc7ef50e06a03771177c4cfa 100644 (file)
--- a/main/stasis.c
+++ b/main/stasis.c
@@ -1342,7 +1342,7 @@ static struct ast_str *multi_object_blob_to_ami(void *obj)
 
        for (type = 0; type < STASIS_UMOS_MAX; ++type) {
                for (i = 0; i < AST_VECTOR_SIZE(&multi->snapshots[type]); ++i) {
-                       char *name = "";
+                       char *name = NULL;
                        void *snapshot = AST_VECTOR_GET(&multi->snapshots[type], i);
                        ami_snapshot = NULL;
 
@@ -1352,11 +1352,11 @@ static struct ast_str *multi_object_blob_to_ami(void *obj)
 
                        switch (type) {
                        case STASIS_UMOS_CHANNEL:
-                               ami_snapshot = ast_manager_build_channel_state_string_prefix(snapshot, name);
+                               ami_snapshot = ast_manager_build_channel_state_string_prefix(snapshot, name ?: "");
                                break;
 
                        case STASIS_UMOS_BRIDGE:
-                               ami_snapshot = ast_manager_build_bridge_state_string_prefix(snapshot, name);
+                               ami_snapshot = ast_manager_build_bridge_state_string_prefix(snapshot, name ?: "");
                                break;
 
                        case STASIS_UMOS_ENDPOINT:
@@ -1367,6 +1367,7 @@ static struct ast_str *multi_object_blob_to_ami(void *obj)
                                ast_str_append(&ami_str, 0, "%s", ast_str_buffer(ami_snapshot));
                                ast_free(ami_snapshot);
                        }
+                       ast_free(name);
                }
        }
 

diff --git a/main/utils.c b/main/utils.c
index f20ccd36e44f0b669214e0a1fd9748d25bab77d2..dd717629574c236501a70f594078ee82699b98d8 100644 (file)
--- a/main/utils.c
+++ b/main/utils.c
@@ -2349,7 +2349,13 @@ int __ast_asprintf(const char *file, int lineno, const char *func, char **ret, c
        va_list ap;
 
        va_start(ap, fmt);
-       if ((res = vasprintf(ret, fmt, ap)) == -1) {
+       res = vasprintf(ret, fmt, ap);
+       if (res < 0) {
+               /*
+                * *ret is undefined so set to NULL to ensure it is
+                * initialized to something useful.
+                */
+               *ret = NULL;
                MALLOC_FAILURE_MSG;
        }
        va_end(ap);

diff --git a/res/ari/resource_bridges.c b/res/ari/resource_bridges.c
index f243086c978709303ddc6f8c2ff8f01517515643..e08df7d8b10b8476f404f9c58c8bddd9ffb9a824 100644 (file)
--- a/res/ari/resource_bridges.c
+++ b/res/ari/resource_bridges.c
@@ -386,7 +386,6 @@ static int ari_bridges_play_helper(const char **args_media,
 
        if (ast_asprintf(playback_url, "/playbacks/%s",
                        stasis_app_playback_get_id(playback)) == -1) {
-               playback_url = NULL;
                ast_ari_response_alloc_failed(response);
                return -1;
        }

diff --git a/res/res_xmpp.c b/res/res_xmpp.c
index 8a06a6c35afbdea5e154271536ef1b2cad84fbf9..f683557a5a96c7684c839581cc50207f90fd637f 100644 (file)
--- a/res/res_xmpp.c
+++ b/res/res_xmpp.c
@@ -3909,8 +3909,11 @@ static int fetch_access_token(struct ast_xmpp_client_config *cfg)
        struct ast_json_error error;
        RAII_VAR(struct ast_json *, jobj, NULL, ast_json_unref);
 
-       ast_asprintf(&cmd, "CURL(%s,client_id=%s&client_secret=%s&refresh_token=%s&grant_type=refresh_token)",
-                    url, cfg->oauth_clientid, cfg->oauth_secret, cfg->refresh_token);
+       if (ast_asprintf(&cmd,
+               "CURL(%s,client_id=%s&client_secret=%s&refresh_token=%s&grant_type=refresh_token)",
+               url, cfg->oauth_clientid, cfg->oauth_secret, cfg->refresh_token) < 0) {
+               return -1;
+       }
 
        ast_debug(2, "Performing OAuth 2.0 authentication for client '%s' using command: %s\n",
                cfg->name, cmd);