[Asterisk-code-review] rtp: Add support for RTP extension negotiation and abs-send-... (asterisk[15])

[Kevin Harwell]

Kevin Harwell has posted comments on this change. ( https://gerrit.asterisk.org/8883 )

Change subject: rtp: Add support for RTP extension negotiation and abs-send-time.
......................................................................


Patch Set 2: Code-Review-1

(2 comments)

https://gerrit.asterisk.org/#/c/8883/2/main/rtp_engine.c
File main/rtp_engine.c:

https://gerrit.asterisk.org/#/c/8883/2/main/rtp_engine.c@797
PS2, Line 797: int ast_rtp_instance_extmap_negotiate(struct ast_rtp_instance *instance, int id, enum ast_rtp_extension_direction direction,
the vector replace below will crash if id is <=0. Prob need to check it to make sure.


https://gerrit.asterisk.org/#/c/8883/2/res/res_pjsip_sdp_rtp.c
File res/res_pjsip_sdp_rtp.c:

https://gerrit.asterisk.org/#/c/8883/2/res/res_pjsip_sdp_rtp.c@1250
PS2, Line 1250: 		ast_rtp_instance_extmap_negotiate(session_media->rtp, id, direction, uri, attributes);
The parsing of 'id' above (sscanf) could result in id potentially being equal to -1 (integer overflow from a crafted extension).

If this is the case I'm fairly certain crashes may result upon passing in -1 here.

Is the id/value limited to 1-14? I wonder if we should put some range checking in or something to be safe.



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

Gerrit-Project: asterisk
Gerrit-Branch: 15
Gerrit-MessageType: comment
Gerrit-Change-Id: I508deac557867b1e27fc7339be890c8018171588
Gerrit-Change-Number: 8883
Gerrit-PatchSet: 2
Gerrit-Owner: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Comment-Date: Thu, 03 May 2018 15:23:39 +0000
Gerrit-HasComments: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180503/2c590aae/attachment.html>