From RFC 6455 http://tools.ietf.org/html/rfc6455#section-10.4 Browser should have platform-specific limitations regarding the frame size. It MUST protect themselves against exceeding those limits. For example, a malicious endpoint can try to exhaust its peer's memory or mount a denial-of-service attack by sending a single big frame (e.g., of size 2**60)
Created attachment 134944 [details] Patch
A malicious endpoint can try to send a long stream of small frames that are a part of a fragmented message, it can exhaust the memory of browser too. I plan to split this case into another bug, do you think it is Okay?
Comment on attachment 134944 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=134944&action=review > Source/WebCore/ChangeLog:9 > + Protect browser against exhausting its memory, when it reecives a very big Frame(e.g., of size 2**60). Typo: reecives. > Source/WebCore/Modules/websockets/WebSocketChannel.cpp:87 > +// FIXME: frameSizeLimitation should be platform-specific > +const size_t frameSizeLimitation = 500 * 1024 * 1024; This needs an explanation of how each platform would choose the limit. It's not even clear why a hardcoded limit is appropriate. How was this value chosen, for example?
(In reply to comment #3) > > Source/WebCore/Modules/websockets/WebSocketChannel.cpp:87 > > +// FIXME: frameSizeLimitation should be platform-specific > > +const size_t frameSizeLimitation = 500 * 1024 * 1024; > > This needs an explanation of how each platform would choose the limit. It's not even clear why a hardcoded limit is appropriate. How was this value chosen, for example? There is not specific definition about the frameSizeLimitation value in the RFC6455. In fact, there will not a very exact limitation value, it should be related with current free memory. But it will be difficult algorithm if we compute the current free memory. In addition, taking the efficiency into consideration, the most and normal scenarios should not send the big frame, so setting the value just prevent the malicious attack. So I suggest the smaller limitation value will be better. But I have no idea how to check that it is appropriate or not. What is your opinion?
We should start with what we want to achieve, not how. If there is no practical issue to fix, we should not do anything. Otherwise, let's discuss the issue and its severity first.
(In reply to comment #5) > We should start with what we want to achieve, not how. > > If there is no practical issue to fix, we should not do anything. Otherwise, let's discuss the issue and its severity first. But from RFC 6455, browser MUST protect themselves against exceeding those memory limits. If browser can't allocate the more memory, maybe it will crash. Taking the security into consideration, I think this is still valuable to do it.
Some easier ways to protect against this would be: 1. Fail gracefully when allocation fails. 2. Allocate memory when data is actually received, not when frame header states that it will be huge (maybe we already do that?) What's complicated about the approach suggested here is that there is no guidance about how to choose the limit on each platform. And the limit should really be the same across platforms for compatibility. It's normal that huge content will cause out of memory situations in the engine. We have no protections against crashing when receiving a multi-gigabyte HTML file, for example. The kind of issues we generally protect against is when a single value somewhere can cause out of memory situations, making for an easy denial of service attack.
(In reply to comment #7) > Some easier ways to protect against this would be: > > 1. Fail gracefully when allocation fails. > 2. Allocate memory when data is actually received, not when frame header states that it will be huge (maybe we already do that?) Yeah, browser used tryFastMalloc function to allocate memory, when it received a single frame whose final bit was set to be 1, which indeed can protect browser against crash. But when browser received a long stream of small frames that are a part of a fragmented message, it used "append" function to add data into Vector<char> m_continuousFrameData, maybe it fail because of no memory, I think using "tryappend" to replace "append" function can protect the browser against crash. Maybe we will need a patch to do that, do you think so?
> But when browser received a long stream of small frames that are a part of a fragmented message I don't think that this is a case worth protecting against. There is no reason to have complicated out of memory protections in WebSocket code when it's just as easy to crash the browser with a huge HTML file.