Fix many issues from the NULL_RETURNS Coverity report

Most of the changes here are trivial NULL checks.  There are a couple
optimizations to remove the need to check for NULL and outboundproxy parsing
in chan_sip.c was rewritten to avoid use of strtok.  Additionally, a bug was
found and fixed with the parsing of outboundproxy when "outboundproxy=," was
set.

(Closes issue ASTERISK-19654)

git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/1.8@365398 65c4cc65-6c06-0410-ace0-fbb531ad65f3

diff --git a/apps/app_chanspy.c b/apps/app_chanspy.c
index d2c10bcdfca30a0091059bca76cbd20dec0e11c5..c8f08af3ced3431292482266c90075303fd4e9af 100644 (file)
--- a/apps/app_chanspy.c
+++ b/apps/app_chanspy.c
@@ -765,13 +765,10 @@ static int common_exec(struct ast_channel *chan, struct ast_flags *flags,
        const char *context, const char *mailbox, const char *name_context)
 {
        char nameprefix[AST_NAME_STRLEN];
-       char peer_name[AST_NAME_STRLEN + 5];
        char exitcontext[AST_MAX_CONTEXT] = "";
        signed char zero_volume = 0;
        int waitms;
        int res;
-       char *ptr;
-       int num;
        int num_spyed_upon = 1;
        struct ast_channel_iterator *iter = NULL;
 
@@ -861,7 +858,6 @@ static int common_exec(struct ast_channel *chan, struct ast_flags *flags,
                                next_channel(iter, autochan, chan), next_autochan = NULL) {
                        int igrp = !mygroup;
                        int ienf = !myenforced;
-                       char *s;
 
                        if (autochan->chan == prev) {
                                ast_autochan_destroy(autochan);
@@ -946,22 +942,34 @@ static int common_exec(struct ast_channel *chan, struct ast_flags *flags,
                                continue;
                        }
 
-                       strcpy(peer_name, "spy-");
-                       strncat(peer_name, autochan->chan->name, AST_NAME_STRLEN - 4 - 1);
-                       ptr = strchr(peer_name, '/');
-                       *ptr++ = '\0';
-                       ptr = strsep(&ptr, "-");
-
-                       for (s = peer_name; s < ptr; s++)
-                               *s = tolower(*s);
 
                        if (!ast_test_flag(flags, OPTION_QUIET)) {
+                               char peer_name[AST_NAME_STRLEN + 5];
+                               char *ptr, *s;
+
+                               strcpy(peer_name, "spy-");
+                               strncat(peer_name, autochan->chan->name, AST_NAME_STRLEN - 4 - 1);
+                               if ((ptr = strchr(peer_name, '/'))) {
+                                       *ptr++ = '\0';
+                                       for (s = peer_name; s < ptr; s++) {
+                                               *s = tolower(*s);
+                                       }
+                                       if ((s = strchr(ptr, '-'))) {
+                                               *s = '\0';
+                                       }
+                               }
+
                                if (ast_test_flag(flags, OPTION_NAME)) {
                                        const char *local_context = S_OR(name_context, "default");
                                        const char *local_mailbox = S_OR(mailbox, ptr);
-                                       res = ast_app_sayname(chan, local_mailbox, local_context);
+                                       if (local_mailbox) {
+                                               res = ast_app_sayname(chan, local_mailbox, local_context);
+                                       } else {
+                                               res = -1;
+                                       }
                                }
                                if (!ast_test_flag(flags, OPTION_NAME) || res < 0) {
+                                       int num;
                                        if (!ast_test_flag(flags, OPTION_NOTECH)) {
                                                if (ast_fileexists(peer_name, NULL, NULL) > 0) {
                                                        res = ast_streamfile(chan, peer_name, chan->language);
@@ -976,8 +984,9 @@ static int common_exec(struct ast_channel *chan, struct ast_flags *flags,
                                                        res = ast_say_character_str(chan, peer_name, "", chan->language);
                                                }
                                        }
-                                       if ((num = atoi(ptr)))
-                                               ast_say_digits(chan, atoi(ptr), "", chan->language);
+                                       if (ptr && (num = atoi(ptr))) {
+                                               ast_say_digits(chan, num, "", chan->language);
+                                       }
                                }
                        }
 

diff --git a/apps/app_followme.c b/apps/app_followme.c
index 9e173807c9363c24cbe77f5d04db7a8e17434317..cd1262f1fb8accb0b1b8848e95142411acd2ca2f 100644 (file)
--- a/apps/app_followme.c
+++ b/apps/app_followme.c
@@ -884,6 +884,10 @@ static void findmeexec(struct fm_args *tpargs)
        struct findme_user_listptr *findme_user_list;
 
        findme_user_list = ast_calloc(1, sizeof(*findme_user_list));
+       if (!findme_user_list) {
+               ast_log(LOG_WARNING, "Failed to allocate memory for findme_user_list\n");
+               return;
+       }
        AST_LIST_HEAD_INIT_NOLOCK(findme_user_list);
 
        /* We're going to figure out what the longest possible string of digits to collect is */

diff --git a/apps/app_stack.c b/apps/app_stack.c
index b6c9045cec79b09749e66c5bee15e39150c60fb7..72a3367eda2541fd49785dea0a5df4ce3010d47b 100644 (file)
--- a/apps/app_stack.c
+++ b/apps/app_stack.c
@@ -764,8 +764,15 @@ static int handle_gosub(struct ast_channel *chan, AGI *agi, int argc, const char
                        struct ast_pbx *pbx = chan->pbx;
                        struct ast_pbx_args args;
                        struct ast_datastore *stack_store = ast_channel_datastore_find(chan, &stack_info, NULL);
-                       AST_LIST_HEAD(, gosub_stack_frame) *oldlist = stack_store->data;
-                       struct gosub_stack_frame *cur = AST_LIST_FIRST(oldlist);
+                       AST_LIST_HEAD(, gosub_stack_frame) *oldlist;
+                       struct gosub_stack_frame *cur;
+                       if (!stack_store) {
+                               ast_log(LOG_WARNING, "No GoSub stack remaining after AGI GoSub execution.\n");
+                               ast_free(gosub_args);
+                               return RESULT_FAILURE;
+                       }
+                       oldlist = stack_store->data;
+                       cur = AST_LIST_FIRST(oldlist);
                        cur->is_agi = 1;
 
                        memset(&args, 0, sizeof(args));

diff --git a/apps/app_voicemail.c b/apps/app_voicemail.c
index b461e1d5d3102b163c6f9a3c7c0f80cf5136cbc1..ff28df4ae0d4bb2605526b1eae3a59e2c1eeed5c 100644 (file)
--- a/apps/app_voicemail.c
+++ b/apps/app_voicemail.c
@@ -11523,6 +11523,10 @@ static void mwi_unsub_event_cb(const struct ast_event *event, void *userdata)
        if (ast_event_get_ie_uint(event, AST_EVENT_IE_EVENTTYPE) != AST_EVENT_MWI)
                return;
 
+       if (!uniqueid) {
+               ast_log(LOG_ERROR, "Unable to allocate memory for uniqueid\n");
+               return;
+       }
        u = ast_event_get_ie_uint(event, AST_EVENT_IE_UNIQUEID);
        *uniqueid = u;
        if (ast_taskprocessor_push(mwi_subscription_tps, handle_unsubscribe, uniqueid) < 0) {

diff --git a/channels/chan_iax2.c b/channels/chan_iax2.c
index 20b3b534e4f1fdaa0d91585255b07a8829786762..85a0dbb83cdff798c79c20b548fa5c9dac265a00 100644 (file)
--- a/channels/chan_iax2.c
+++ b/channels/chan_iax2.c
@@ -13046,6 +13046,11 @@ static int set_config(const char *config_file, int reload)
                        ast_config_destroy(ucfg);
                        return 0;
                }
+               if (!cfg) {
+                       /* should have been able to load the config here */
+                       ast_log(LOG_ERROR, "Unable to load config %s again\n", config_file);
+                       return -1;
+               }
        } else if (cfg == CONFIG_STATUS_FILEINVALID) {
                ast_log(LOG_ERROR, "Config file %s is in an invalid format.  Aborting.\n", config_file);
                return 0;
@@ -13923,8 +13928,9 @@ static int function_iaxpeer(struct ast_channel *chan, const char *cmd, char *dat
        } else  if (!strncasecmp(colname, "codec[", 6)) {
                char *codecnum, *ptr;
                int codec = 0;
-               
-               codecnum = strchr(colname, '[');
+
+               /* skip over "codec" to the '[' */
+               codecnum = colname + 5;
                *codecnum = '\0';
                codecnum++;
                if ((ptr = strchr(codecnum, ']'))) {

diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index 0f6d77cd49b022c4cba5f3aa188965a692353c5f..e660d2871c6cef4d18f205c4ccb92c3ea4c6c12e 100644 (file)
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -15265,7 +15265,9 @@ static int get_rdnis(struct sip_pvt *p, struct sip_request *oreq, char **name, c
                char *end_quote;
                rname = tmp + 1;
                end_quote = strchr(rname, '\"');
-               *end_quote = '\0';
+               if (end_quote) {
+                       *end_quote = '\0';
+               }
        }
 
        if (number) {
@@ -24210,6 +24212,7 @@ static int cc_esc_publish_handler(struct sip_pvt *pvt, struct sip_request *req,
         * document earlier. So there's no need to enclose this operation in an if statement.
         */
        tuple_children = ast_xml_node_get_children(tuple_node);
+       /* coverity[null_returns: FALSE] */
        status_node = ast_xml_find_element(tuple_children, "status", NULL, NULL);
 
        if (!(status_children = ast_xml_node_get_children(status_node))) {
@@ -24541,7 +24544,8 @@ static int handle_request_subscribe(struct sip_pvt *p, struct sip_request *req,
        struct sip_peer *authpeer = NULL;
        const char *eventheader = get_header(req, "Event");     /* Get Event package name */
        int resubscribe = (p->subscribed != NONE) && !req->ignore;
-       char *temp, *event;
+       char *event_end;
+       ptrdiff_t event_len = 0;
 
        if (p->initreq.headers) {       
                /* We already have a dialog */
@@ -24603,13 +24607,10 @@ static int handle_request_subscribe(struct sip_pvt *p, struct sip_request *req,
                return 0;
        }
 
-       if ( (strchr(eventheader, ';'))) {
-               event = ast_strdupa(eventheader);       /* Since eventheader is a const, we can't change it */
-               temp = strchr(event, ';');              
-               *temp = '\0';                           /* Remove any options for now */
-                                                       /* We might need to use them later :-) */
-       } else
-               event = (char *) eventheader;           /* XXX is this legal ? */
+       event_end = strchr(eventheader, ';');
+       if (event_end) {
+               event_len = event_end - eventheader;
+       }
 
        /* Handle authentication if we're new and not a retransmission. We can't just
         * use if !req->ignore, because then we'll end up sending
@@ -24648,7 +24649,7 @@ static int handle_request_subscribe(struct sip_pvt *p, struct sip_request *req,
                return 0;
        }
 
-       if (strcmp(event, "message-summary") && strcmp(event, "call-completion")) {
+       if (strncmp(eventheader, "message-summary", MAX(event_len, 15)) && strncmp(eventheader, "call-completion", MAX(event_len, 15))) {
                /* Get destination right away */
                gotdest = get_destination(p, NULL, NULL);
        }
@@ -24674,7 +24675,7 @@ static int handle_request_subscribe(struct sip_pvt *p, struct sip_request *req,
        if (ast_strlen_zero(p->tag))
                make_our_tag(p->tag, sizeof(p->tag));
 
-       if (!strcmp(event, "presence") || !strcmp(event, "dialog")) { /* Presence, RFC 3842 */
+       if (!strncmp(eventheader, "presence", MAX(event_len, 8)) || !strncmp(eventheader, "dialog", MAX(event_len, 6))) { /* Presence, RFC 3842 */
                unsigned int pidf_xml;
                const char *accept;
                int start = 0;
@@ -24752,7 +24753,7 @@ static int handle_request_subscribe(struct sip_pvt *p, struct sip_request *req,
                } else {
                        p->subscribed = subscribed;
                }
-       } else if (!strcmp(event, "message-summary")) {
+       } else if (!strncmp(eventheader, "message-summary", MAX(event_len, 15))) {
                int start = 0;
                int found_supported = 0;
                const char *acceptheader;
@@ -24815,11 +24816,11 @@ static int handle_request_subscribe(struct sip_pvt *p, struct sip_request *req,
                        p->relatedpeer = ref_peer(authpeer, "setting dialog's relatedpeer pointer");
                }
                /* Do not release authpeer here */
-       } else if (!strcmp(event, "call-completion")) {
+       } else if (!strncmp(eventheader, "call-completion", MAX(event_len, 15))) {
                handle_cc_subscribe(p, req);
        } else { /* At this point, Asterisk does not understand the specified event */
                transmit_response(p, "489 Bad Event", req);
-               ast_debug(2, "Received SIP subscribe for unknown event package: %s\n", event);
+               ast_debug(2, "Received SIP subscribe for unknown event package: %s\n", eventheader);
                pvt_set_needdestroy(p, "unknown event package");
                if (authpeer) {
                        unref_peer(authpeer, "unref_peer, from handle_request_subscribe (authpeer 5)");
@@ -27389,32 +27390,42 @@ static struct sip_peer *build_peer(const char *name, struct ast_variable *v, str
                        } else if (!strcasecmp(v->name, "fromuser")) {
                                ast_string_field_set(peer, fromuser, v->value);
                        } else if (!strcasecmp(v->name, "outboundproxy")) {
-                               char *tok, *proxyname;
+                               char *host, *proxyname, *sep;
 
                                if (ast_strlen_zero(v->value)) {
-                                       ast_log(LOG_WARNING, "no value given for outbound proxy on line %d of sip.conf.", v->lineno);
+                                       ast_log(LOG_WARNING, "no value given for outbound proxy on line %d of sip.conf\n", v->lineno);
                                        continue;
                                }
 
-                               peer->outboundproxy =
-                                   ao2_alloc(sizeof(*peer->outboundproxy), NULL);
-
-                               tok = ast_skip_blanks(strtok(ast_strdupa(v->value), ","));
-
-                               sip_parse_host(tok, v->lineno, &proxyname,
-                                              &peer->outboundproxy->port,
-                                              &peer->outboundproxy->transport);
+                               if (!peer->outboundproxy) {
+                                       peer->outboundproxy = ao2_alloc(sizeof(*peer->outboundproxy), NULL);
+                                       if (!peer->outboundproxy) {
+                                               ast_log(LOG_WARNING, "Unable to allocate config storage for outboundproxy\n");
+                                               continue;
+                                       }
+                               }
 
-                               tok = ast_skip_blanks(strtok(ast_strdupa(v->value), ","));
+                               host = ast_strdupa(v->value);
+                               if (!host) {
+                                       ast_log(LOG_WARNING, "Unable to allocate stack space for parsing outboundproxy\n");
+                                       continue;
+                               }
 
-                               if ((tok = strtok(NULL, ","))) {
-                                       peer->outboundproxy->force = !strncasecmp(ast_skip_blanks(tok), "force", 5);
+                               host = ast_skip_blanks(host);
+                               sep = strchr(host, ',');
+                               if (sep) {
+                                       *sep++ = '\0';
+                                       peer->outboundproxy->force = !strncasecmp(ast_skip_blanks(sep), "force", 5);
                                } else {
                                        peer->outboundproxy->force = FALSE;
                                }
 
+                               sip_parse_host(host, v->lineno, &proxyname,
+                                              &peer->outboundproxy->port,
+                                              &peer->outboundproxy->transport);
+
                                if (ast_strlen_zero(proxyname)) {
-                                       ast_log(LOG_WARNING, "you must specify a name for the outboundproxy on line %d of sip.conf.", v->lineno);
+                                       ast_log(LOG_WARNING, "you must specify a name for the outboundproxy on line %d of sip.conf\n", v->lineno);
                                        sip_cfg.outboundproxy.name[0] = '\0';
                                        continue;
                                }
@@ -27968,6 +27979,11 @@ static int reload_config(enum channelreloadreason reason)
                        ast_config_destroy(ucfg);
                        return 1;
                }
+               if (!cfg) {
+                       /* should have been able to reload here */
+                       ast_log(LOG_NOTICE, "Unable to load config %s\n", config);
+                       return -1;
+               }
        } else if (cfg == CONFIG_STATUS_FILEINVALID) {
                ast_log(LOG_ERROR, "Contents of %s are invalid and cannot be parsed\n", config);
                return 1;
@@ -28360,27 +28376,34 @@ static int reload_config(enum channelreloadreason reason)
                                default_fromdomainport = STANDARD_SIP_PORT;
                        }
                } else if (!strcasecmp(v->name, "outboundproxy")) {
-                       char *tok, *proxyname;
+                       char *host, *proxyname, *sep;
 
                        if (ast_strlen_zero(v->value)) {
-                               ast_log(LOG_WARNING, "no value given for outbound proxy on line %d of sip.conf.", v->lineno);
+                               ast_log(LOG_WARNING, "no value given for outbound proxy on line %d of sip.conf\n", v->lineno);
                                continue;
                        }
 
-                       tok = ast_skip_blanks(strtok(ast_strdupa(v->value), ","));
-
-                       sip_parse_host(tok, v->lineno, &proxyname,
-                                      &sip_cfg.outboundproxy.port,
-                                      &sip_cfg.outboundproxy.transport);
+                       host = ast_strdupa(v->value);
+                       if (!host) {
+                               ast_log(LOG_WARNING, "Unable to allocate stack space for parsing outboundproxy\n");
+                               continue;
+                       }
 
-                       if ((tok = strtok(NULL, ","))) {
-                               sip_cfg.outboundproxy.force = !strncasecmp(ast_skip_blanks(tok), "force", 5);
+                       host = ast_skip_blanks(host);
+                       sep = strchr(host, ',');
+                       if (sep) {
+                               *sep++ = '\0';
+                               sip_cfg.outboundproxy.force = !strncasecmp(ast_skip_blanks(sep), "force", 5);
                        } else {
                                sip_cfg.outboundproxy.force = FALSE;
                        }
 
+                       sip_parse_host(host, v->lineno, &proxyname,
+                                      &sip_cfg.outboundproxy.port,
+                                      &sip_cfg.outboundproxy.transport);
+
                        if (ast_strlen_zero(proxyname)) {
-                               ast_log(LOG_WARNING, "you must specify a name for the outboundproxy on line %d of sip.conf.", v->lineno);
+                               ast_log(LOG_WARNING, "you must specify a name for the outboundproxy on line %d of sip.conf\n", v->lineno);
                                sip_cfg.outboundproxy.name[0] = '\0';
                                continue;
                        }

diff --git a/channels/sip/config_parser.c b/channels/sip/config_parser.c
index 72a7b2c7d077b9c4674283d68babd5f07351f2a5..89f1a988edd4c3378d38852c4512573d2dc03083 100644 (file)
--- a/channels/sip/config_parser.c
+++ b/channels/sip/config_parser.c
@@ -638,6 +638,7 @@ int sip_parse_host(char *line, int lineno, char **hostname, int *portnum, enum s
        char *port;
 
        if (ast_strlen_zero(line)) {
+               *hostname = NULL;
                return -1;
        }
        if ((*hostname = strstr(line, "://"))) {

diff --git a/funcs/func_aes.c b/funcs/func_aes.c
index 1854c3b11482e9a2464aaca86afd0787179c6a6e..e719cc80bfadad13a1207ebae9a9ac4d4f1d58f8 100644 (file)
--- a/funcs/func_aes.c
+++ b/funcs/func_aes.c
@@ -114,6 +114,10 @@ static int aes_helper(struct ast_channel *chan, const char *cmd, char *data,
        ast_aes_set_encrypt_key((unsigned char *) args.key, &ecx);   /* encryption:  plaintext -> encryptedtext -> base64 */
        ast_aes_set_decrypt_key((unsigned char *) args.key, &dcx);   /* decryption:  base64 -> encryptedtext -> plaintext */
        tmp = ast_calloc(1, len);                     /* requires a tmp buffer for the base64 decode */
+       if (!tmp) {
+               ast_log(LOG_ERROR, "Unable to allocate memory for data\n");
+               return -1;
+       }
        tmpP = tmp;
        encrypt = strcmp("AES_DECRYPT", cmd);           /* -1 if encrypting, 0 if decrypting */
 

diff --git a/main/config.c b/main/config.c
index c9ecd7c0f3df75066437bba9f25f300ecf62c433..a52dd815537182101ab2a0f8a13073ca2ca406a8 100644 (file)
--- a/main/config.c
+++ b/main/config.c
@@ -2118,6 +2118,10 @@ int read_config_maps(void)
        clear_config_maps();
 
        configtmp = ast_config_new();
+       if (!configtmp) {
+               ast_log(LOG_ERROR, "Unable to allocate memory for new config\n");
+               return -1;
+       }
        configtmp->max_include_level = 1;
        config = ast_config_internal_load(extconfig_conf, configtmp, flags, "", "extconfig");
        if (config == CONFIG_STATUS_FILEINVALID) {

diff --git a/main/features.c b/main/features.c
index 4e2126369d1aa91b9e6985516b9793b70045a3d8..81c6cd5f4e13e5fcb250b106487ecf1f72fb1e0f 100644 (file)
--- a/main/features.c
+++ b/main/features.c
@@ -5645,7 +5645,7 @@ static struct ast_parkinglot *build_parkinglot(const char *pl_name, struct ast_v
 static void process_applicationmap_line(struct ast_variable *var)
 {
        char *tmp_val = ast_strdupa(var->value);
-       char *activateon;
+       char *activateon, *new_syn;
        struct ast_call_feature *feature;
        AST_DECLARE_APP_ARGS(args,
                AST_APP_ARG(exten);
@@ -5656,10 +5656,10 @@ static void process_applicationmap_line(struct ast_variable *var)
        );
 
        AST_STANDARD_APP_ARGS(args, tmp_val);
-       if (strchr(args.app, '(')) {
+       if ((new_syn = strchr(args.app, '('))) {
                /* New syntax */
                args.moh_class = args.app_args;
-               args.app_args = strchr(args.app, '(');
+               args.app_args = new_syn;
                *args.app_args++ = '\0';
                if (args.app_args[strlen(args.app_args) - 1] == ')') {
                        args.app_args[strlen(args.app_args) - 1] = '\0';

diff --git a/pbx/pbx_config.c b/pbx/pbx_config.c
index b86e1a8cbf28e11fbd40f26efa0fced08f5a888a..46a605e2b9f80b2b77cbc1abd546a7e071ce89c1 100644 (file)
--- a/pbx/pbx_config.c
+++ b/pbx/pbx_config.c
@@ -733,6 +733,11 @@ static char *handle_cli_dialplan_save(struct ast_cli_entry *e, int cmd, struct a
        snprintf(filename, sizeof(filename), "%s%s%s", base, slash, config);
 
        cfg = ast_config_load("extensions.conf", config_flags);
+       if (!cfg) {
+               ast_cli(a->fd, "Failed to load extensions.conf\n");
+               ast_mutex_unlock(&save_dialplan_lock);
+               return CLI_FAILURE;
+       }
 
        /* try to lock contexts list */
        if (ast_rdlock_contexts()) {