<p>Richard Mudgett <strong>posted comments</strong> on this change.</p><p><a href="https://gerrit.asterisk.org/6443">View Change</a></p><p>Patch set 2:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #ffd4d4;">Code-Review -1</span></p><p>(2 comments)</p><ul style="list-style: none; padding-left: 20px;"><li><p><a href="https://gerrit.asterisk.org/#/c/6443/2/res/res_rtp_asterisk.c">File res/res_rtp_asterisk.c:</a></p><ul style="list-style: none; padding-left: 20px;"><li><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/6443/2/res/res_rtp_asterisk.c@4608">Patch Set #2, Line 4608:</a> <code style="font-family:monospace,monospace">       if (packetwords < 2) {</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">This test isn't necessary.  The length test below will take care of it.</p></li><li><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/6443/2/res/res_rtp_asterisk.c@4621">Patch Set #2, Line 4621:</a> <code style="font-family:monospace,monospace">                if ((i + (length & 0xffff)) > packetwords) {</code></p><ul><li>This test should be done after the length is calculated.</li></ul><ul><li>We need to add this check after testing the packet length to ensure that the encoded packet record length is at least large enough for the fields we will read from the record:</li></ul><p style="white-space: pre-wrap; word-wrap: break-word;">unsigned int min_len;</p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">min_len = 2; /* length and ssrc */<br>switch (pt) {<br>case RTCP_PT_SR:<br>   min_len += 5;<br>   /* fall through */<br>case RTCP_PT_RR:<br>   min_len += 5;<br>   break;<br>default:<br>   break;<br>}<br>if (length < min_len) {<br>   ast_debug(1, "Packet RTCP record too short\n");<br>   return &ast_null_frame;<br>}</pre><p style="white-space: pre-wrap; word-wrap: break-word;">The switch min_len values are determined by the later switch cases.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/6443">change 6443</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/6443"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 13 </div>
<div style="display:none"> Gerrit-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: I67d89e3c27db83efa0e6b52734f73c88ac2939e2 </div>
<div style="display:none"> Gerrit-Change-Number: 6443 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Richard Mudgett <rmudgett@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Sean Bright <sean.bright@gmail.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Wed, 06 Sep 2017 23:37:57 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>