665518 - Websocket Data Framing crash [@mozilla::net::nsWebSocketHandler::ApplyMask]

Status: RESOLVED FIXED

(Core :: Networking: WebSockets, defect)

Description

[Christoph Diehl [:posidron]]

Attachment: callstack

Received connection: ('127.0.0.1', 61673)
HTTP/1.1 101 Switching Protocols
Upgrade: websocket
Connection: Upgrade
Sec-WebSocket-Accept: x6X5fx2b1lAIM7QZxYAlCH7GW6c=


Sending: b'\t\xff\xfe\x81\x05Hello'

Comment 2

[Patrick McManus [:mcmanus]]

section 4.2 says that the most-significant-bit of a 64 bit length must be 0.. (0xfe is the MSB of the length field above) and we are missing a check to enforce that.. failing to do so led to a quantity going negative and then various length checks failing.

Comment 3

[Patrick McManus [:mcmanus]]

Attachment: 63 bit lengths v1

Comment 4

[Ian Melven :imelven]

is there a particular reason that PRInt64 payloadLength is signed ?

Comment 5

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/4b4280bad349

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

This should land to aurora too, right?

Comment 7

[Patrick McManus [:mcmanus]]

for the aurora review: this bug is found as part of websockets fuzz testing as part of the security review. yea for fuzz testing!

Comment 8

[LegNeato]

Comment on attachment 540471 [details] [diff] [review]
63 bit lengths v1

Approved for releases/mozilla-aurora

Comment 9

[Patrick McManus [:mcmanus]]

http://hg.mozilla.org/releases/mozilla-aurora/rev/8ee63a6bfefb

Comment 10

[Daniel Veditz [:dveditz]]

assuming worst-case sg:critical since it looks like we can overwrite heap data in ApplyMask() if we don't die reading out of bounds first.