<p>Richard Mudgett has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.asterisk.org/5850">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">chan_pjsip: Fix PJSIP_MEDIA_OFFER dialplan function read.<br><br>The construction of the returned string assumed incorrectly that the<br>supplied buffer would always be initialized as an empty string.  If it is<br>not an empty string we could overrun the supplied buffer by the length of<br>the non-empty buffer string plus one.  It is also theoreticaly possible<br>for the supplied buffer to be overrun by a string terminator during a read<br>operation even if the supplied buffer is an empty string.<br><br>* Fix the assumption that the supplied buffer would already be an empty<br>string.  The buffer is not guaranteed to contain an empty string by all<br>possible callers.<br><br>* Fix string terminator buffer overrun potential.<br><br>Change-Id: If6a0806806527678c8554b1dcb34fd7808aa95c9<br>---<br>M channels/pjsip/dialplan_functions.c<br>1 file changed, 21 insertions(+), 14 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/50/5850/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/channels/pjsip/dialplan_functions.c b/channels/pjsip/dialplan_functions.c<br>index 7b3434d..e2c78cd 100644<br>--- a/channels/pjsip/dialplan_functions.c<br>+++ b/channels/pjsip/dialplan_functions.c<br>@@ -927,36 +927,40 @@<br> static int media_offer_read_av(struct ast_sip_session *session, char *buf,<br>                               size_t len, enum ast_media_type media_type)<br> {<br>-       int i, size = 0;<br>+     int idx;<br>+     size_t accum = 0;<br> <br>- for (i = 0; i < ast_format_cap_count(session->req_caps); i++) {<br>-                struct ast_format *fmt = ast_format_cap_get_format(session->req_caps, i);<br>+ /* Note: buf is not terminated while the string is being built. */<br>+   for (idx = 0; idx < ast_format_cap_count(session->req_caps); ++idx) {<br>+          struct ast_format *fmt;<br>+              size_t size;<br> <br>+              fmt = ast_format_cap_get_format(session->req_caps, idx);<br>           if (ast_format_get_type(fmt) != media_type) {<br>                         ao2_ref(fmt, -1);<br>                     continue;<br>             }<br> <br>-         /* add one since we'll include a comma */<br>+                /* Add one for a comma or terminator */<br>               size = strlen(ast_format_get_name(fmt)) + 1;<br>          if (len < size) {<br>                  ao2_ref(fmt, -1);<br>                     break;<br>                }<br>+<br>+         /* Append the format name */<br>+         strcpy(buf + accum, ast_format_get_name(fmt));/* Safe */<br>+             ao2_ref(fmt, -1);<br>+<br>+         accum += size;<br>                len -= size;<br> <br>-              /* no reason to use strncat here since we have already ensured buf has<br>-                   enough space, so strcat can be safely used */<br>-            strcat(buf, ast_format_get_name(fmt));<br>-               strcat(buf, ",");<br>-<br>-               ao2_ref(fmt, -1);<br>+            /* The last comma on the built string will be set to the terminator. */<br>+              buf[accum - 1] = ',';<br>         }<br> <br>- if (size) {<br>-          /* remove the extra comma */<br>-         buf[strlen(buf) - 1] = '\0';<br>- }<br>+    /* Remove the trailing comma or terminate an empty buffer. */<br>+        buf[accum ? accum - 1 : 0] = '\0';<br>    return 0;<br> }<br> <br>@@ -996,6 +1000,9 @@<br>                return media_offer_read_av(channel->session, buf, len, AST_MEDIA_TYPE_AUDIO);<br>      } else if (!strcmp(data, "video")) {<br>                return media_offer_read_av(channel->session, buf, len, AST_MEDIA_TYPE_VIDEO);<br>+     } else {<br>+             /* Ensure that the buffer is empty */<br>+                buf[0] = '\0';<br>        }<br> <br>  return 0;<br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/5850">change 5850</a>. To unsubscribe, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/5850"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>
<div style="display:none"> Gerrit-Change-Id: If6a0806806527678c8554b1dcb34fd7808aa95c9 </div>
<div style="display:none"> Gerrit-Change-Number: 5850 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Richard Mudgett <rmudgett@digium.com> </div>