strings.c: Fix __ast_str_helper() to always return a terminated string.
authorRichard Mudgett <rmudgett@digium.com>
Mon, 19 Oct 2015 20:28:46 +0000 (15:28 -0500)
committerRichard Mudgett <rmudgett@digium.com>
Wed, 21 Oct 2015 21:49:13 +0000 (16:49 -0500)
Users of functions which call __ast_str_helper() such as the ones listed
below are likely to not check the return value for failure so ensuring
that the string is always nil terminated is a good safety measure.

ast_str_set_va()
ast_str_append_va()
ast_str_set()
ast_str_append()

Change-Id: I36ab2d14bb6015868b49329dda8639d70fbcae07

main/strings.c

index 7aaff79..495011e 100644 (file)
@@ -60,55 +60,78 @@ int __ast_str_helper(struct ast_str **buf, ssize_t max_len,
        int append, const char *fmt, va_list ap)
 #endif
 {
-       int res, need;
+       int res;
+       int added;
+       int need;
        int offset = (append && (*buf)->__AST_STR_LEN) ? (*buf)->__AST_STR_USED : 0;
        va_list aq;
 
+       if (max_len < 0) {
+               max_len = (*buf)->__AST_STR_LEN;        /* don't exceed the allocated space */
+       }
+
        do {
-               if (max_len < 0) {
-                       max_len = (*buf)->__AST_STR_LEN;        /* don't exceed the allocated space */
-               }
-               /*
-                * Ask vsnprintf how much space we need. Remember that vsnprintf
-                * does not count the final <code>'\\0'</code> so we must add 1.
-                */
                va_copy(aq, ap);
                res = vsnprintf((*buf)->__AST_STR_STR + offset, (*buf)->__AST_STR_LEN - offset, fmt, aq);
+               va_end(aq);
+
+               if (res < 0) {
+                       /*
+                        * vsnprintf write to string failed.
+                        * I don't think this is possible with a memory buffer.
+                        */
+                       res = AST_DYNSTR_BUILD_FAILED;
+                       added = 0;
+                       break;
+               }
 
-               need = res + offset + 1;
                /*
-                * If there is not enough space and we are below the max length,
-                * reallocate the buffer and return a message telling to retry.
+                * vsnprintf returns how much space we used or would need.
+                * Remember that vsnprintf does not count the nil terminator
+                * so we must add 1.
                 */
-               if (need > (*buf)->__AST_STR_LEN && (max_len == 0 || (*buf)->__AST_STR_LEN < max_len) ) {
-                       int len = (int)(*buf)->__AST_STR_LEN;
-                       if (max_len && max_len < need) {        /* truncate as needed */
-                               need = max_len;
-                       } else if (max_len == 0) {      /* if unbounded, give more room for next time */
-                               need += 16 + need / 4;
-                       }
-                       if (
+               added = res;
+               need = offset + added + 1;
+               if (need <= (*buf)->__AST_STR_LEN
+                       || (max_len && max_len <= (*buf)->__AST_STR_LEN)) {
+                       /*
+                        * There was enough room for the string or we are not
+                        * allowed to try growing the string buffer.
+                        */
+                       break;
+               }
+
+               /* Reallocate the buffer and try again. */
+               if (max_len == 0) {
+                       /* unbounded, give more room for next time */
+                       need += 16 + need / 4;
+               } else if (max_len < need) {
+                       /* truncate as needed */
+                       need = max_len;
+               }
+
+               if (
 #if (defined(MALLOC_DEBUG) && !defined(STANDALONE))
-                                       _ast_str_make_space(buf, need, file, lineno, function)
+                       _ast_str_make_space(buf, need, file, lineno, function)
 #else
-                                       ast_str_make_space(buf, need)
+                       ast_str_make_space(buf, need)
 #endif
-                               ) {
-                               ast_log_safe(LOG_VERBOSE, "failed to extend from %d to %d\n", len, need);
-                               va_end(aq);
-                               return AST_DYNSTR_BUILD_FAILED;
-                       }
-                       (*buf)->__AST_STR_STR[offset] = '\0';   /* Truncate the partial write. */
+                       ) {
+                       ast_log_safe(LOG_VERBOSE, "failed to extend from %d to %d\n",
+                               (int) (*buf)->__AST_STR_LEN, need);
 
-                       /* Restart va_copy before calling vsnprintf() again. */
-                       va_end(aq);
-                       continue;
+                       res = AST_DYNSTR_BUILD_FAILED;
+                       break;
                }
-               va_end(aq);
-               break;
        } while (1);
-       /* update space used, keep in mind the truncation */
-       (*buf)->__AST_STR_USED = (res + offset > (*buf)->__AST_STR_LEN) ? (*buf)->__AST_STR_LEN - 1: res + offset;
+
+       /* Update space used, keep in mind truncation may be necessary. */
+       (*buf)->__AST_STR_USED = ((*buf)->__AST_STR_LEN <= offset + added)
+               ? (*buf)->__AST_STR_LEN - 1
+               : offset + added;
+
+       /* Ensure that the string is terminated. */
+       (*buf)->__AST_STR_STR[(*buf)->__AST_STR_USED] = '\0';
 
        return res;
 }