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 b1579a70ac6329ce0d48e59d9b4421a334dbe082..9f147768dd160eb607a06965fdbe486e5cd84b61 100644 (file)
--- a/apps/app_mixmonitor.c
+++ b/apps/app_mixmonitor.c
@@ -818,6 +818,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/include/asterisk/utils.h b/include/asterisk/utils.h
index 4a5dbf282c5b2ce4a808fd74a909b0bf7978c02d..03705b321ac58bd523c1aa5dedcffa357e626fb2 100644 (file)
--- a/include/asterisk/utils.h
+++ b/include/asterisk/utils.h
@@ -721,7 +721,13 @@ int _ast_vasprintf(char **ret, const char *file, int lineno, const char *func, c
 
        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 4bb9355c66e6d045f03fa763b6811ac852d695bc..ab5f7a07c2375ad54bb636f0f783edd1272d1d59 100644 (file)
--- a/main/db.c
+++ b/main/db.c
@@ -193,9 +193,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 f45d585c1fac24f14d356f8e5014c319cfdd4a6d..057a26212515ab66e416b7646715cd6627eb65f9 100644 (file)
--- a/main/json.c
+++ b/main/json.c
@@ -462,7 +462,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 48e4eb5a554d17bce57b9a2b2318a7c6b8d6fcd9..2d998051707e7e404538831571c08f4294dc6951 100644 (file)
--- a/main/stasis.c
+++ b/main/stasis.c
@@ -1344,7 +1344,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;
 
@@ -1354,11 +1354,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:
@@ -1369,6 +1369,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 0824a373a59713d5f667f498417556724884f3e7..03420304600e9f77a2a9d3cb13cacc6869fad9b5 100644 (file)
--- a/main/utils.c
+++ b/main/utils.c
@@ -2389,7 +2389,13 @@ int _ast_asprintf(char **ret, const char *file, int lineno, const char *func, co
        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 8c058c4a7b1199ef95074d400cac63217d642cb3..d6a0400c86e03faf6ccbee57234844f9186bbbe0 100644 (file)
--- a/res/ari/resource_bridges.c
+++ b/res/ari/resource_bridges.c
@@ -387,7 +387,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 6ba4014c7059908da7073305f58d27fd684cb9c6..1b0ae427bc7abd3c3eb70a2d82eb9db2bdd5690c 100644 (file)
--- a/res/res_xmpp.c
+++ b/res/res_xmpp.c
@@ -3911,8 +3911,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);