[Asterisk-code-review] Fix ast (v)asprintf() malloc failure usage conditions. (asterisk[15])
Richard Mudgett
asteriskteam at digium.com
Mon Nov 6 11:47:16 CST 2017
Richard Mudgett has uploaded this change for review. ( https://gerrit.asterisk.org/7014
Change subject: Fix ast_(v)asprintf() malloc failure usage conditions.
......................................................................
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
---
M apps/app_mixmonitor.c
M bridges/bridge_softmix.c
M include/asterisk/utils.h
M main/db.c
M main/json.c
M main/stasis.c
M main/utils.c
M res/ari/resource_bridges.c
M res/res_xmpp.c
9 files changed, 31 insertions(+), 13 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/14/7014/1
diff --git a/apps/app_mixmonitor.c b/apps/app_mixmonitor.c
index 2922e0c..bb47cfc 100644
--- a/apps/app_mixmonitor.c
+++ b/apps/app_mixmonitor.c
@@ -813,6 +813,8 @@
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 9197df6..f490967 100644
--- a/bridges/bridge_softmix.c
+++ b/bridges/bridge_softmix.c
@@ -502,7 +502,6 @@
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 423d73b..0a12b1d 100644
--- a/include/asterisk/utils.h
+++ b/include/asterisk/utils.h
@@ -622,7 +622,13 @@
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 9432435..fa125d7 100644
--- a/main/db.c
+++ b/main/db.c
@@ -191,9 +191,11 @@
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 9004978..56df7f4 100644
--- a/main/json.c
+++ b/main/json.c
@@ -460,7 +460,7 @@
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 a82e938..6dc5dbf 100644
--- a/main/stasis.c
+++ b/main/stasis.c
@@ -1342,7 +1342,7 @@
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 @@
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 @@
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 f20ccd3..dd71762 100644
--- a/main/utils.c
+++ b/main/utils.c
@@ -2349,7 +2349,13 @@
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 f243086..e08df7d 100644
--- a/res/ari/resource_bridges.c
+++ b/res/ari/resource_bridges.c
@@ -386,7 +386,6 @@
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 8a06a6c..f683557 100644
--- a/res/res_xmpp.c
+++ b/res/res_xmpp.c
@@ -3909,8 +3909,11 @@
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);
--
To view, visit https://gerrit.asterisk.org/7014
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: 15
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib5252fb8850ecf0f78ed0ee2ca0796bda7e91c23
Gerrit-Change-Number: 7014
Gerrit-PatchSet: 1
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20171106/de927542/attachment-0001.html>
More information about the asterisk-code-review
mailing list