Skip to content

Commit

Permalink
various files - fix some alerts raised by lgtm code analysis
Browse files Browse the repository at this point in the history
This patch fixes several issues reported by the lgtm code analysis tool:

https://lgtm.com/projects/g/asterisk/asterisk

Not all reported issues were addressed in this patch. This patch mostly fixes
confirmed reported errors, potential problematic code points, and a few other
"low hanging" warnings or recommendations found in core supported modules.
These include, but are not limited to the following:

* innapropriate stack allocation in loops
* buffer overflows
* variable declaration "hiding" another variable declaration
* comparisons results that are always the same
* ambiguously signed bit-field members
* missing header guards

Change-Id: Id4a881686605d26c94ab5409bc70fcc21efacc25
  • Loading branch information
kharwell authored and gtjoseph committed Nov 18, 2019
1 parent 990a91b commit bdd785d
Show file tree
Hide file tree
Showing 49 changed files with 325 additions and 204 deletions.
8 changes: 4 additions & 4 deletions apps/app_cdr.c
Expand Up @@ -119,13 +119,13 @@ struct app_cdr_message_payload {
/*! The name of the channel to be manipulated */
const char *channel_name;
/*! Disable the CDR for this channel */
int disable:1;
unsigned int disable:1;
/*! Re-enable the CDR for this channel */
int reenable:1;
unsigned int reenable:1;
/*! Reset the CDR */
int reset:1;
unsigned int reset:1;
/*! If reseting the CDR, keep the variables */
int keep_variables:1;
unsigned int keep_variables:1;
};

static void appcdr_callback(void *data, struct stasis_subscription *sub, struct stasis_message *message)
Expand Down
4 changes: 3 additions & 1 deletion apps/app_dictate.c
Expand Up @@ -149,7 +149,8 @@ static int dictate_exec(struct ast_channel *chan, const char *data)
ast_mkdir(base, 0755);
len = strlen(base) + strlen(filein) + 2;
if (!path || len > maxlen) {
path = ast_alloca(len);
ast_free(path);
path = ast_malloc(len);
memset(path, 0, len);
maxlen = len;
} else {
Expand Down Expand Up @@ -334,6 +335,7 @@ static int dictate_exec(struct ast_channel *chan, const char *data)
ast_frfree(f);
}
}
ast_free(path);
if (oldr) {
ast_set_read_format(chan, oldr);
ao2_ref(oldr, -1);
Expand Down
10 changes: 4 additions & 6 deletions apps/app_followme.c
Expand Up @@ -408,7 +408,6 @@ static int reload_followme(int reload)
char *cat = NULL, *tmp;
struct ast_variable *var;
struct number *cur, *nm;
char *numberstr;
int timeout;
int numorder;
const char* enable_callee_prompt_str;
Expand Down Expand Up @@ -536,9 +535,12 @@ static int reload_followme(int reload)
while (var) {
if (!strcasecmp(var->name, "number")) {
int idx = 0;
char copy[strlen(var->value) + 1];
char *numberstr;

/* Add a new number */
numberstr = ast_strdupa(var->value);
strcpy(copy, var->value); /* safe */
numberstr = copy;
if ((tmp = strchr(numberstr, ','))) {
*tmp++ = '\0';
timeout = atoi(tmp);
Expand Down Expand Up @@ -762,10 +764,6 @@ static struct ast_channel *wait_for_winner(struct findme_user_listptr *findme_us
}

tmpto = to;
if (to < 0) {
to = 1000;
tmpto = 1000;
}
towas = to;
winner = ast_waitfor_n(watchers, pos, &to);
tmpto -= to;
Expand Down
3 changes: 2 additions & 1 deletion apps/app_minivm.c
Expand Up @@ -2652,9 +2652,10 @@ static int create_vmaccount(char *name, struct ast_variable *var, int realtime)
ast_copy_string(vmu->fullname, var->value, sizeof(vmu->fullname));
} else if (!strcasecmp(var->name, "setvar")) {
char *varval;
char *varname = ast_strdupa(var->value);
char varname[strlen(var->value) + 1];
struct ast_variable *tmpvar;

strcpy(varname, var->value); /* safe */
if ((varval = strchr(varname, '='))) {
*varval = '\0';
varval++;
Expand Down
9 changes: 7 additions & 2 deletions apps/app_playback.c
Expand Up @@ -173,7 +173,10 @@ static int s_streamwait3(const say_args_t *a, const char *fn)
static int do_say(say_args_t *a, const char *s, const char *options, int depth)
{
struct ast_variable *v;
char *lang, *x, *rule = NULL;
char *lang;
char *x;
char *rule = NULL;
char *rule_head = NULL;
int ret = 0;
struct varshead head = { .first = NULL, .last = NULL };
struct ast_var_t *n;
Expand All @@ -195,7 +198,7 @@ static int do_say(say_args_t *a, const char *s, const char *options, int depth)
for (;;) {
for (v = ast_variable_browse(say_cfg, lang); v ; v = v->next) {
if (ast_extension_match(v->name, s)) {
rule = ast_strdupa(v->value);
rule_head = rule = ast_strdup(v->value);
break;
}
}
Expand All @@ -220,6 +223,7 @@ static int do_say(say_args_t *a, const char *s, const char *options, int depth)
n = ast_var_assign("SAY", s);
if (!n) {
ast_log(LOG_ERROR, "Memory allocation error in do_say\n");
ast_free(rule_head);
return -1;
}
AST_LIST_INSERT_HEAD(&head, n, entries);
Expand Down Expand Up @@ -281,6 +285,7 @@ static int do_say(say_args_t *a, const char *s, const char *options, int depth)
}
}
ast_var_delete(n);
ast_free(rule_head);
return ret;
}

Expand Down
3 changes: 1 addition & 2 deletions apps/app_readexten.c
Expand Up @@ -170,8 +170,7 @@ static int readexten_exec(struct ast_channel *chan, const char *data)
if (timeout <= 0)
timeout = ast_channel_pbx(chan) ? ast_channel_pbx(chan)->rtimeoutms : 10000;

if (digit_timeout <= 0)
digit_timeout = ast_channel_pbx(chan) ? ast_channel_pbx(chan)->dtimeoutms : 5000;
digit_timeout = ast_channel_pbx(chan) ? ast_channel_pbx(chan)->dtimeoutms : 5000;

if (ast_test_flag(&flags, OPT_INDICATION) && !ast_strlen_zero(arglist.filename)) {
ts = ast_get_indication_tone(ast_channel_zone(chan), arglist.filename);
Expand Down
111 changes: 65 additions & 46 deletions apps/app_voicemail.c
Expand Up @@ -1797,7 +1797,7 @@ static void vm_change_password(struct ast_vm_user *vmu, const char *newpassword)
struct ast_config *cfg = NULL;
struct ast_variable *var = NULL;
struct ast_category *cat = NULL;
char *category = NULL, *value = NULL, *new = NULL;
char *category = NULL;
const char *tmp = NULL;
struct ast_flags config_flags = { CONFIG_FLAG_WITHCOMMENTS };
char secretfn[PATH_MAX] = "";
Expand All @@ -1824,24 +1824,28 @@ static void vm_change_password(struct ast_vm_user *vmu, const char *newpassword)
if ((cfg = ast_config_load(VOICEMAIL_CONFIG, config_flags)) && valid_config(cfg)) {
while ((category = ast_category_browse(cfg, category))) {
if (!strcasecmp(category, vmu->context)) {
char *value = NULL;
char *new = NULL;
if (!(tmp = ast_variable_retrieve(cfg, category, vmu->mailbox))) {
ast_log(AST_LOG_WARNING, "We could not find the mailbox.\n");
break;
}
value = strstr(tmp, ",");
if (!value) {
new = ast_alloca(strlen(newpassword)+1);
new = ast_malloc(strlen(newpassword) + 1);
sprintf(new, "%s", newpassword);
} else {
new = ast_alloca((strlen(value) + strlen(newpassword) + 1));
new = ast_malloc((strlen(value) + strlen(newpassword) + 1));
sprintf(new, "%s%s", newpassword, value);
}
if (!(cat = ast_category_get(cfg, category, NULL))) {
ast_log(AST_LOG_WARNING, "Failed to get category structure.\n");
ast_free(new);
break;
}
ast_variable_update(cat, vmu->mailbox, new, NULL, 0);
found = 1;
ast_free(new);
}
}
/* save the results */
Expand All @@ -1865,13 +1869,14 @@ static void vm_change_password(struct ast_vm_user *vmu, const char *newpassword)
for (category = ast_category_browse(cfg, NULL); category; category = ast_category_browse(cfg, category)) {
ast_debug(4, "users.conf: %s\n", category);
if (!strcasecmp(category, vmu->mailbox)) {
char new[strlen(newpassword) + 1];
if (!ast_variable_retrieve(cfg, category, "vmsecret")) {
ast_debug(3, "looks like we need to make vmsecret!\n");
var = ast_variable_new("vmsecret", newpassword, "");
} else {
var = NULL;
}
new = ast_alloca(strlen(newpassword) + 1);

sprintf(new, "%s", newpassword);
if (!(cat = ast_category_get(cfg, category, NULL))) {
ast_debug(4, "failed to get category!\n");
Expand Down Expand Up @@ -2194,7 +2199,6 @@ static int imap_retrieve_greeting(const char *dir, const int msgnum, struct ast_
struct vm_state *vms_p;
char *file, *filename;
char dest[PATH_MAX];
char *attachment;
int i;
BODY *body;
int ret = 0;
Expand Down Expand Up @@ -2247,21 +2251,26 @@ static int imap_retrieve_greeting(const char *dir, const int msgnum, struct ast_
mail_fetchstructure(vms_p->mailstream, i + 1, &body);
/* We have the body, now we extract the file name of the first attachment. */
if (body->nested.part && body->nested.part->next && body->nested.part->next->body.parameter->value) {
attachment = ast_strdupa(body->nested.part->next->body.parameter->value);
char *attachment = body->nested.part->next->body.parameter->value;
char copy[strlen(attachment) + 1];

strcpy(copy, attachment); /* safe */
attachment = copy;

filename = strsep(&attachment, ".");
if (!strcmp(filename, file)) {
ast_copy_string(vms_p->fn, dir, sizeof(vms_p->fn));
vms_p->msgArray[vms_p->curmsg] = i + 1;
create_dirpath(dest, sizeof(dest), vmu->context, vms_p->username, "");
save_body(body, vms_p, "2", attachment, 0);
ret = 0;
break;
}
} else {
ast_log(AST_LOG_ERROR, "There is no file attached to this IMAP message.\n");
ret = -1;
break;
}
filename = strsep(&attachment, ".");
if (!strcmp(filename, file)) {
ast_copy_string(vms_p->fn, dir, sizeof(vms_p->fn));
vms_p->msgArray[vms_p->curmsg] = i + 1;
create_dirpath(dest, sizeof(dest), vmu->context, vms_p->username, "");
save_body(body, vms_p, "2", attachment, 0);
ret = 0;
break;
}
}

if (curr_mbox != -1) {
Expand Down Expand Up @@ -8141,10 +8150,12 @@ static void queue_mwi_event(const char *channel_id, const char *box, int urgent,

aliases = ao2_find(mailbox_alias_mappings, box, OBJ_SEARCH_KEY | OBJ_MULTIPLE);
while ((mapping = ao2_iterator_next(aliases))) {
char alias[strlen(mapping->alias) + 1];
strcpy(alias, mapping->alias); /* safe */
mailbox = NULL;
context = NULL;
ast_debug(3, "Found alias mapping: %s -> %s\n", mapping->alias, box);
separate_mailbox(ast_strdupa(mapping->alias), &mailbox, &context);
separate_mailbox(alias, &mailbox, &context);
ast_publish_mwi_state_channel(mailbox, context, new + urgent, old, channel_id);
ao2_ref(mapping, -1);
}
Expand Down Expand Up @@ -8364,12 +8375,12 @@ static int forward_message(struct ast_channel *chan, char *context, struct vm_st
directory_app = pbx_findapp("Directory");
if (directory_app) {
char vmcontext[256];
char *old_context;
char *old_exten;
char old_context[strlen(ast_channel_context(chan)) + 1];
char old_exten[strlen(ast_channel_exten(chan)) + 1];
int old_priority;
/* make backup copies */
old_context = ast_strdupa(ast_channel_context(chan));
old_exten = ast_strdupa(ast_channel_exten(chan));
strcpy(old_context, ast_channel_context(chan)); /* safe */
strcpy(old_exten, ast_channel_exten(chan)); /* safe */
old_priority = ast_channel_priority(chan);

/* call the Directory, changes the channel */
Expand Down Expand Up @@ -9050,7 +9061,6 @@ static int imap_remove_file(char *dir, int msgnum)
static int imap_delete_old_greeting (char *dir, struct vm_state *vms)
{
char *file, *filename;
char *attachment;
char arg[11];
int i;
BODY* body;
Expand Down Expand Up @@ -9079,17 +9089,22 @@ static int imap_delete_old_greeting (char *dir, struct vm_state *vms)
mail_fetchstructure(vms->mailstream, i + 1, &body);
/* We have the body, now we extract the file name of the first attachment. */
if (body->nested.part->next && body->nested.part->next->body.parameter->value) {
attachment = ast_strdupa(body->nested.part->next->body.parameter->value);
char *attachment = body->nested.part->next->body.parameter->value;
char copy[strlen(attachment) + 1];

strcpy(copy, attachment); /* safe */
attachment = copy;

filename = strsep(&attachment, ".");
if (!strcmp(filename, file)) {
snprintf(arg, sizeof(arg), "%d", i + 1);
mail_setflag(vms->mailstream, arg, "\\DELETED");
}
} else {
ast_log(AST_LOG_ERROR, "There is no file attached to this IMAP message.\n");
ast_mutex_unlock(&vms->lock);
return -1;
}
filename = strsep(&attachment, ".");
if (!strcmp(filename, file)) {
snprintf(arg, sizeof(arg), "%d", i + 1);
mail_setflag(vms->mailstream, arg, "\\DELETED");
}
}
mail_expunge(vms->mailstream);

Expand Down Expand Up @@ -13705,26 +13720,30 @@ static void load_zonemessages(struct ast_config *cfg)

var = ast_variable_browse(cfg, "zonemessages");
while (var) {
struct vm_zone *z;
char *msg_format, *tzone;

z = ast_malloc(sizeof(*z));
if (!z) {
return;
}
if (var->value) {
struct vm_zone *z;
char *msg_format, *tzone;
char storage[strlen(var->value) + 1];

z = ast_malloc(sizeof(*z));
if (!z) {
return;
}

msg_format = ast_strdupa(var->value);
tzone = strsep(&msg_format, "|,");
if (msg_format) {
ast_copy_string(z->name, var->name, sizeof(z->name));
ast_copy_string(z->timezone, tzone, sizeof(z->timezone));
ast_copy_string(z->msg_format, msg_format, sizeof(z->msg_format));
AST_LIST_LOCK(&zones);
AST_LIST_INSERT_HEAD(&zones, z, list);
AST_LIST_UNLOCK(&zones);
} else {
ast_log(AST_LOG_WARNING, "Invalid timezone definition at line %d\n", var->lineno);
ast_free(z);
strcpy(storage, var->value); /* safe */
msg_format = storage;
tzone = strsep(&msg_format, "|,");
if (msg_format) {
ast_copy_string(z->name, var->name, sizeof(z->name));
ast_copy_string(z->timezone, tzone, sizeof(z->timezone));
ast_copy_string(z->msg_format, msg_format, sizeof(z->msg_format));
AST_LIST_LOCK(&zones);
AST_LIST_INSERT_HEAD(&zones, z, list);
AST_LIST_UNLOCK(&zones);
} else {
ast_log(AST_LOG_WARNING, "Invalid timezone definition at line %d\n", var->lineno);
ast_free(z);
}
}
var = var->next;
}
Expand Down

0 comments on commit bdd785d

Please sign in to comment.