Closed Bug 1313527 Opened 3 years ago Closed 3 years ago

red parser goes into endless loop on broken SDP

Categories

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

50 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox49 --- unaffected
firefox-esr45 --- unaffected
firefox50 --- fixed
firefox51 --- fixed
firefox52 --- fixed
Blocking Flags:

People

(Reporter: drno, Assigned: drno)

References

Details

(Keywords: csectype-dos, regression)

Attachments

(1 file)

No description provided.
backlog: --- → webrtc/webaudio+
Rank: 10
Blocks: 1309641
Comment on attachment 8805313 [details]
Bug 1313527: exit red parser loop if stroul failed.

https://reviewboard.mozilla.org/r/89058/#review88262

::: media/webrtc/signaling/test/sdp_unittests.cpp:444
(Diff revision 1)
>  
>  TEST_F(SdpTest, parseRtcpFbFooBarBaz) {
>    ParseSdp(kVideoSdp + "a=rtcp-fb:120 foo bar baz\r\n");
>  }
>  
> +static const std::string kVideoSdpWithUnknonwBrokenFtmp =

typo Unknonw should be Unknown
Comment on attachment 8805313 [details]
Bug 1313527: exit red parser loop if stroul failed.

https://reviewboard.mozilla.org/r/89058/#review88270

One nit, otherwise good.
Attachment #8805313 - Flags: review?(mfroman) → review+
Pushed by drno@ohlmeier.org:
https://hg.mozilla.org/integration/autoland/rev/aeb0df69182a
exit red parser loop if stroul failed. r=mjf
Comment on attachment 8805313 [details]
Bug 1313527: exit red parser loop if stroul failed.

Approval Request Comment
[Feature/regressing bug #]: Bug 1275360 added support for parsing red, which has a problem.

[User impact if declined]: Firefox users who use WebRTC services or connect to WebRTC devices which send broken SDP to Firefox can cause Firefox's main thread to enter an endless loop from which the user can only terminate Firefox via the OS.

[Describe test coverage new/current, TreeHerder]: The patch contains a new unit test which shows that their is no longer an endless loop.

[Risks and why]: The risk is pretty low as it only set an additional exit criteria from a loop which should only be entered in rare corner cases.

[String/UUID change made/needed]: N/A
Attachment #8805313 - Flags: approval-mozilla-beta?
Attachment #8805313 - Flags: approval-mozilla-aurora?
Note: this could be used to trivially DOS the browser (though I'm sure there are other ways to do so easily).
Keywords: csectype-dos
https://hg.mozilla.org/mozilla-central/rev/aeb0df69182a
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Comment on attachment 8805313 [details]
Bug 1313527: exit red parser loop if stroul failed.

Fixes a hang (and see comment 7) with invalid SDP, Aurora51+, Beta50+
Attachment #8805313 - Flags: approval-mozilla-beta?
Attachment #8805313 - Flags: approval-mozilla-beta+
Attachment #8805313 - Flags: approval-mozilla-aurora?
Attachment #8805313 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.