Fix some potential misuses of ast_str in the code.

Passing an ast_str pointer by value that then calls
ast_str_set(), ast_str_set_va(), ast_str_append(), or
ast_str_append_va() can result in the pointer originally
passed by value being invalidated if the ast_str had
to be reallocated.

This fixes places in the code that do this. Only the
example in ccss.c could result in pointer invalidation
though since the other cases use a stack-allocated ast_str
and cannot be reallocated.

I've also updated the doxygen in strings.h to include
notes about potential misuse of the functions mentioned
previously.

Review: https://reviewboard.asterisk.org/r/2161
........

Merged revisions 375025 from http://svn.asterisk.org/svn/asterisk/branches/1.8

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

diff --git a/apps/app_dial.c b/apps/app_dial.c
index c3573f907da607508c791acf3f5000bcf636d332..247b4466a8518493babd14dbdc3fe874835afcaf 100644 (file)
--- a/apps/app_dial.c
+++ b/apps/app_dial.c
@@ -672,7 +672,7 @@ struct chanlist {
        struct ast_aoc_decoded *aoc_s_rate_list;
 };
 
-static int detect_disconnect(struct ast_channel *chan, char code, struct ast_str *featurecode);
+static int detect_disconnect(struct ast_channel *chan, char code, struct ast_str **featurecode);
 
 static void chanlist_free(struct chanlist *outgoing)
 {
@@ -1532,7 +1532,7 @@ static struct ast_channel *wait_for_answer(struct ast_channel *in,
                                }
 
                                if (ast_test_flag64(peerflags, OPT_CALLER_HANGUP) &&
-                                       detect_disconnect(in, f->subclass.integer, featurecode)) {
+                                       detect_disconnect(in, f->subclass.integer, &featurecode)) {
                                        ast_verb(3, "User requested call disconnect.\n");
                                        *to = 0;
                                        strcpy(pa->status, "CANCEL");
@@ -1645,18 +1645,18 @@ skip_frame:;
        return peer;
 }
 
-static int detect_disconnect(struct ast_channel *chan, char code, struct ast_str *featurecode)
+static int detect_disconnect(struct ast_channel *chan, char code, struct ast_str **featurecode)
 {
        struct ast_flags features = { AST_FEATURE_DISCONNECT }; /* only concerned with disconnect feature */
        struct ast_call_feature feature = { 0, };
        int res;
 
-       ast_str_append(&featurecode, 1, "%c", code);
+       ast_str_append(featurecode, 1, "%c", code);
 
-       res = ast_feature_detect(chan, &features, ast_str_buffer(featurecode), &feature);
+       res = ast_feature_detect(chan, &features, ast_str_buffer(*featurecode), &feature);
 
        if (res != AST_FEATURE_RETURN_STOREDIGITS) {
-               ast_str_reset(featurecode);
+               ast_str_reset(*featurecode);
        }
        if (feature.feature_mask & AST_FEATURE_DISCONNECT) {
                return 1;

diff --git a/channels/chan_iax2.c b/channels/chan_iax2.c
index a446d60f9089dd95cd691d785441e64e7b006eea..a0d32d68a1ce541b1f5ebfe67b24ac0978349f34 100644 (file)
--- a/channels/chan_iax2.c
+++ b/channels/chan_iax2.c
@@ -1537,19 +1537,19 @@ static int send_ping(const void *data)
        return 0;
 }
 
-static void encmethods_to_str(int e, struct ast_str *buf)
+static void encmethods_to_str(int e, struct ast_str **buf)
 {
-       ast_str_set(&buf, 0, "(");
+       ast_str_set(buf, 0, "(");
        if (e & IAX_ENCRYPT_AES128) {
-               ast_str_append(&buf, 0, "aes128");
+               ast_str_append(buf, 0, "aes128");
        }
        if (e & IAX_ENCRYPT_KEYROTATE) {
-               ast_str_append(&buf, 0, ",keyrotate");
+               ast_str_append(buf, 0, ",keyrotate");
        }
-       if (ast_str_strlen(buf) > 1) {
-               ast_str_append(&buf, 0, ")");
+       if (ast_str_strlen(*buf) > 1) {
+               ast_str_append(buf, 0, ")");
        } else {
-               ast_str_set(&buf, 0, "No");
+               ast_str_set(buf, 0, "No");
        }
 }
 
@@ -3855,7 +3855,7 @@ static char *handle_cli_iax2_show_peer(struct ast_cli_entry *e, int cmd, struct
 
                ast_sockaddr_to_sin(&peer->addr, &peer_addr);
 
-               encmethods_to_str(peer->encmethods, encmethods);
+               encmethods_to_str(peer->encmethods, &encmethods);
                ast_cli(a->fd, "\n\n");
                ast_cli(a->fd, "  * Name       : %s\n", peer->name);
                ast_cli(a->fd, "  Description  : %s\n", peer->description);
@@ -6822,7 +6822,7 @@ static int __iax2_show_peers(int fd, int *total, struct mansession *s, const int
                else
                        ast_copy_string(name, peer->name, sizeof(name));
 
-               encmethods_to_str(peer->encmethods, encmethods);
+               encmethods_to_str(peer->encmethods, &encmethods);
                retstatus = peer_status(peer, status, sizeof(status));
                if (retstatus > 0)
                        online_peers++;
@@ -7130,7 +7130,7 @@ static int manager_iax2_show_peer_list(struct mansession *s, const struct messag
 
        i = ao2_iterator_init(peers, 0);
        for (; (peer = ao2_iterator_next(&i)); peer_unref(peer)) {
-               encmethods_to_str(peer->encmethods, encmethods);
+               encmethods_to_str(peer->encmethods, &encmethods);
                astman_append(s, "Event: PeerEntry\r\n%sChanneltype: IAX\r\n", idtext);
                if (!ast_strlen_zero(peer->username)) {
                        astman_append(s, "ObjectName: %s\r\nObjectUsername: %s\r\n", peer->name, peer->username);
@@ -14716,7 +14716,7 @@ static int peers_data_provider_get(const struct ast_data_search *search,
 
                ast_data_add_bool(data_peer, "dynamic", ast_test_flag64(peer, IAX_DYNAMIC));
 
-               encmethods_to_str(peer->encmethods, encmethods);
+               encmethods_to_str(peer->encmethods, &encmethods);
                ast_data_add_str(data_peer, "encryption", peer->encmethods ? ast_str_buffer(encmethods) : "no");
 
                peer_unref(peer);

diff --git a/include/asterisk/strings.h b/include/asterisk/strings.h
index 40477da7f0aeebb067d4fd4eecfa5d63a4b04f75..cb61788da87a2e9268fd749e5a0000561e764437 100644 (file)
--- a/include/asterisk/strings.h
+++ b/include/asterisk/strings.h
@@ -778,6 +778,12 @@ char *__ast_str_helper2(struct ast_str **buf, ssize_t max_len,
  *      ...
  * }
  * \endcode
+ *
+ * \note Care should be taken when using this function. The function can
+ * result in reallocating the ast_str. If a pointer to the ast_str is passed
+ * by value to a function that calls ast_str_set_va(), then the original ast_str
+ * pointer may be invalidated due to a reallocation.
+ *
  */
 AST_INLINE_API(int __attribute__((format(printf, 3, 0))) ast_str_set_va(struct ast_str **buf, ssize_t max_len, const char *fmt, va_list ap),
 {
@@ -789,6 +795,12 @@ AST_INLINE_API(int __attribute__((format(printf, 3, 0))) ast_str_set_va(struct a
  * \brief Append to a dynamic string using a va_list
  *
  * Same as ast_str_set_va(), but append to the current content.
+ *
+ * \note Care should be taken when using this function. The function can
+ * result in reallocating the ast_str. If a pointer to the ast_str is passed
+ * by value to a function that calls ast_str_append_va(), then the original ast_str
+ * pointer may be invalidated due to a reallocation.
+ *
  */
 AST_INLINE_API(int __attribute__((format(printf, 3, 0))) ast_str_append_va(struct ast_str **buf, ssize_t max_len, const char *fmt, va_list ap),
 {
@@ -827,6 +839,11 @@ AST_INLINE_API(char *ast_str_append_escapecommas(struct ast_str **buf, ssize_t m
 /*!
  * \brief Set a dynamic string using variable arguments
  *
+ * \note Care should be taken when using this function. The function can
+ * result in reallocating the ast_str. If a pointer to the ast_str is passed
+ * by value to a function that calls ast_str_set(), then the original ast_str
+ * pointer may be invalidated due to a reallocation.
+ *
  * \param buf This is the address of a pointer to a struct ast_str which should
  *      have been retrieved using ast_str_thread_get.  It will need to
  *      be updated in the case that the buffer has to be reallocated to
@@ -859,6 +876,11 @@ int __attribute__((format(printf, 3, 4))) ast_str_set(
 /*!
  * \brief Append to a thread local dynamic string
  *
+ * \note Care should be taken when using this function. The function can
+ * result in reallocating the ast_str. If a pointer to the ast_str is passed
+ * by value to a function that calls ast_str_append(), then the original ast_str
+ * pointer may be invalidated due to a reallocation.
+ *
  * The arguments, return values, and usage of this function are the same as
  * ast_str_set(), but the new data is appended to the current value.
  */

diff --git a/main/ccss.c b/main/ccss.c
index 5e8639d6d1c18ea27af39f2c7069b4abf899df7e..c6aeae132f655a45e6e50fbbac710675593c814f 100644 (file)
--- a/main/ccss.c
+++ b/main/ccss.c
@@ -3410,7 +3410,7 @@ struct ast_cc_monitor *ast_cc_get_monitor_by_recall_core_id(const int core_id, c
  * \param dialstring A new dialstring to add
  * \retval void
  */
-static void cc_unique_append(struct ast_str *str, const char *dialstring)
+static void cc_unique_append(struct ast_str **str, const char *dialstring)
 {
        char dialstring_search[AST_CHANNEL_NAME];
 
@@ -3419,10 +3419,10 @@ static void cc_unique_append(struct ast_str *str, const char *dialstring)
                return;
        }
        snprintf(dialstring_search, sizeof(dialstring_search), "%s%c", dialstring, '&');
-       if (strstr(ast_str_buffer(str), dialstring_search)) {
+       if (strstr(ast_str_buffer(*str), dialstring_search)) {
                return;
        }
-       ast_str_append(&str, 0, "%s", dialstring_search);
+       ast_str_append(str, 0, "%s", dialstring_search);
 }
 
 /*!
@@ -3440,7 +3440,7 @@ static void cc_unique_append(struct ast_str *str, const char *dialstring)
  * \param str Where we will store CC_INTERFACES
  * \retval void
  */
-static void build_cc_interfaces_chanvar(struct ast_cc_monitor *starting_point, struct ast_str *str)
+static void build_cc_interfaces_chanvar(struct ast_cc_monitor *starting_point, struct ast_str **str)
 {
        struct extension_monitor_pvt *extension_pvt;
        struct extension_child_dialstring *child_dialstring;
@@ -3449,7 +3449,7 @@ static void build_cc_interfaces_chanvar(struct ast_cc_monitor *starting_point, s
        size_t length;
 
        /* Init to an empty string. */
-       ast_str_truncate(str, 0);
+       ast_str_truncate(*str, 0);
 
        /* First we need to take all of the is_valid child_dialstrings from
         * the extension monitor we found and add them to the CC_INTERFACES
@@ -3472,9 +3472,9 @@ static void build_cc_interfaces_chanvar(struct ast_cc_monitor *starting_point, s
        /* str will have an extra '&' tacked onto the end of it, so we need
         * to get rid of that.
         */
-       length = ast_str_strlen(str);
+       length = ast_str_strlen(*str);
        if (length) {
-               ast_str_truncate(str, length - 1);
+               ast_str_truncate(*str, length - 1);
        }
        if (length <= 1) {
                /* Nothing to recall?  This should not happen. */
@@ -3509,7 +3509,7 @@ int ast_cc_agent_set_interfaces_chanvar(struct ast_channel *chan)
 
        AST_LIST_LOCK(interface_tree);
        monitor = AST_LIST_FIRST(interface_tree);
-       build_cc_interfaces_chanvar(monitor, str);
+       build_cc_interfaces_chanvar(monitor, &str);
        AST_LIST_UNLOCK(interface_tree);
 
        pbx_builtin_setvar_helper(chan, "CC_INTERFACES", ast_str_buffer(str));
@@ -3561,7 +3561,7 @@ int ast_set_cc_interfaces_chanvar(struct ast_channel *chan, const char * const e
                return -1;
        }
 
-       build_cc_interfaces_chanvar(monitor_iter, str);
+       build_cc_interfaces_chanvar(monitor_iter, &str);
        AST_LIST_UNLOCK(interface_tree);
 
        pbx_builtin_setvar_helper(chan, "CC_INTERFACES", ast_str_buffer(str));