1455557 - WebRTC: incorrect handling CRLF/LF in SDP may break NACK

Status: RESOLVED FIXED

(Core :: WebRTC: Signaling, defect, P2)

Description

[grigoriev.ivan.a]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/65.0.3325.181 Safari/537.36

Steps to reproduce:

1. go to https://jsfiddle.net/gjwtzsna/
  This is a simple webrtc demo that emulates a peer that sends \n line breaks instead of \r\n in SDP
2. allow access to camera
3. open about:webrtc in new tab and look at remote SDP



Actual results:

"a=rtcp-fb:<ptype> nack" line is present in local SDP but missing in remote.



Expected results:

According to rfc4566 
"The sequence  CRLF (0x0d0a) is used to end a record, although parsers SHOULD be tolerant and also accept records terminated with a single newline character"

The problem is in this code in sdp_attr.c:

/* Check for empty string */
if (*ptr == '\r') {

https://hg.mozilla.org/mozilla-central/file/tip/media/webrtc/signaling/src/sdp/sipcc/sdp_attr.c#l4682

Comment 1

[Nils Ohlmeier [:drno]]

Attachment: Bug 1455557: accept new line as RTCP FB NACK terminator.

Review commit: https://reviewboard.mozilla.org/r/238706/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/238706/

Comment 2

[Nils Ohlmeier [:drno]]

Thanks for the super detailed bug report!

Comment 3

[Byron Campen [:bwc]]

Comment on attachment 8969863 [details]
Bug 1455557: accept new line as RTCP FB NACK terminator.

https://reviewboard.mozilla.org/r/238706/#review244622

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/41ce4a9c69dd
accept new line as RTCP FB NACK terminator. r=bwc

Comment 5

[Andrei Ciure[:andrei_ciure]]

https://hg.mozilla.org/mozilla-central/rev/41ce4a9c69dd