[Asterisk-code-review] res rtp asterisk: Drop RTCP packets less than 2 words in le... (asterisk[13])

[Richard Mudgett]

Richard Mudgett has posted comments on this change. ( https://gerrit.asterisk.org/6443 )

Change subject: res_rtp_asterisk:  Drop RTCP packets less than 2 words in length
......................................................................


Patch Set 3: Code-Review-1

(2 comments)

https://gerrit.asterisk.org/#/c/6443/3/res/res_rtp_asterisk.c
File res/res_rtp_asterisk.c:

https://gerrit.asterisk.org/#/c/6443/3/res/res_rtp_asterisk.c@4612
PS3, Line 4612: 	if (packetwords < 2) {
              : 		ast_debug(1, "RTCP frame size (%u words) is shorter than 2 words\n", packetwords);
              : 		return &ast_null_frame;
              : 	}
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.

However, I don't think we know enough to enforce the padding bit validation check.


https://gerrit.asterisk.org/#/c/6443/3/res/res_rtp_asterisk.c@4633
PS3, Line 4633: 		ssrc = ntohl(rtcpheader[i + 1]);
You cannot read ssrc until after you have checked that the length of this RTCP block has the space for it.



-- 
To view, visit https://gerrit.asterisk.org/6443
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: comment
Gerrit-Change-Id: I67d89e3c27db83efa0e6b52734f73c88ac2939e2
Gerrit-Change-Number: 6443
Gerrit-PatchSet: 3
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Sean Bright <sean.bright at gmail.com>
Gerrit-Comment-Date: Thu, 07 Sep 2017 19:44:11 +0000
Gerrit-HasComments: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20170907/3a927468/attachment.html>