<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 3:<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/3/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/3/res/res_rtp_asterisk.c@4612">Patch Set #3, Line 4612:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;">  if (packetwords < 2) {<br>             ast_debug(1, "RTCP frame size (%u words) is shorter than 2 words\n", packetwords);<br>          return &ast_null_frame;<br>   }<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">We should implement the RFC RTCP validation check here.  We aren't even checking the version or that SR/RR is the first block payload type.  We are trying to check the packet length while processing the packet below but we should be doing the overall length check before we consume any packet report data.</p><p style="white-space: pre-wrap; word-wrap: break-word;">However, I don't think we know enough to enforce the padding bit validation check.</p></li><li><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/6443/3/res/res_rtp_asterisk.c@4633">Patch Set #3, Line 4633:</a> <code style="font-family:monospace,monospace">         ssrc = ntohl(rtcpheader[i + 1]);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">You cannot read ssrc until after you have checked that the length of this RTCP block has the space for it.</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: 3 </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: Thu, 07 Sep 2017 19:44:11 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>