aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMatthew Wild <mwild1@gmail.com>2020-09-29 15:18:32 +0100
committerMatthew Wild <mwild1@gmail.com>2020-09-29 15:18:32 +0100
commita481251b29a745556bcb9a32786f3ccd02ad7102 (patch)
tree0b519a65df24e8d1f79d551189f15986994772ae
parenta0fb8d876f7b962866543042821db4f95011e931 (diff)
downloadprosody-a481251b29a745556bcb9a32786f3ccd02ad7102.tar.gz
prosody-a481251b29a745556bcb9a32786f3ccd02ad7102.zip
mod_websocket: Refactor frame validity checking, also check partially-received frames against constraints
-rw-r--r--plugins/mod_websocket.lua172
1 files changed, 95 insertions, 77 deletions
diff --git a/plugins/mod_websocket.lua b/plugins/mod_websocket.lua
index ecc12a45..a613672b 100644
--- a/plugins/mod_websocket.lua
+++ b/plugins/mod_websocket.lua
@@ -142,6 +142,65 @@ local function filter_open_close(data)
return data;
end
+
+local function validate_frame(frame, max_length)
+ local opcode, length = frame.opcode, frame.length;
+
+ if max_length and length > max_length then
+ return false, 1009, "Payload too large";
+ end
+
+ -- Error cases
+ if frame.RSV1 or frame.RSV2 or frame.RSV3 then -- Reserved bits non zero
+ return false, 1002, "Reserved bits not zero";
+ end
+
+ if opcode == 0x8 and frame.data then -- close frame
+ if length == 1 then
+ return false, 1002, "Close frame with payload, but too short for status code";
+ elseif length >= 2 then
+ local status_code = parse_close(frame.data)
+ if status_code < 1000 then
+ return false, 1002, "Closed with invalid status code";
+ elseif ((status_code > 1003 and status_code < 1007) or status_code > 1011) and status_code < 3000 then
+ return false, 1002, "Closed with reserved status code";
+ end
+ end
+ end
+
+ if opcode >= 0x8 then
+ if length > 125 then -- Control frame with too much payload
+ return false, 1002, "Payload too large";
+ end
+
+ if not frame.FIN then -- Fragmented control frame
+ return false, 1002, "Fragmented control frame";
+ end
+ end
+
+ if (opcode > 0x2 and opcode < 0x8) or (opcode > 0xA) then
+ return false, 1002, "Reserved opcode";
+ end
+
+ -- Check opcode
+ if opcode == 0x2 then -- Binary frame
+ return false, 1003, "Only text frames are supported, RFC 7395 3.2";
+ elseif opcode == 0x8 then -- Close request
+ return false, 1000, "Goodbye";
+ end
+
+ -- Other (XMPP-specific) validity checks
+ if not frame.FIN then
+ return false, 1003, "Continuation frames are not supported, RFC 7395 3.3.3";
+ end
+ if opcode == 0x01 and frame.data and frame.data:byte(1, 1) ~= 60 then
+ return false, 1007, "Invalid payload start character, RFC 7395 3.3.3";
+ end
+
+ return true;
+end
+
+
function handle_request(event)
local request, response = event.request, event.response;
local conn = response.conn;
@@ -172,90 +231,40 @@ function handle_request(event)
conn:close();
end
- local dataBuffer;
- local function handle_frame(frame)
- local opcode = frame.opcode;
- local length = frame.length;
- module:log("debug", "Websocket received frame: opcode=%0x, %i bytes", frame.opcode, #frame.data);
-
- -- Error cases
- if frame.RSV1 or frame.RSV2 or frame.RSV3 then -- Reserved bits non zero
- websocket_close(1002, "Reserved bits not zero");
- return false;
- end
-
- if opcode == 0x8 then -- close frame
- if length == 1 then
- websocket_close(1002, "Close frame with payload, but too short for status code");
- return false;
- elseif length >= 2 then
- local status_code = parse_close(frame.data)
- if status_code < 1000 then
- websocket_close(1002, "Closed with invalid status code");
- return false;
- elseif ((status_code > 1003 and status_code < 1007) or status_code > 1011) and status_code < 3000 then
- websocket_close(1002, "Closed with reserved status code");
- return false;
- end
- end
- end
-
- if opcode >= 0x8 then
- if length > 125 then -- Control frame with too much payload
- websocket_close(1002, "Payload too large");
- return false;
- end
-
- if not frame.FIN then -- Fragmented control frame
- websocket_close(1002, "Fragmented control frame");
- return false;
- end
- end
-
- if (opcode > 0x2 and opcode < 0x8) or (opcode > 0xA) then
- websocket_close(1002, "Reserved opcode");
- return false;
+ local function websocket_handle_error(session, code, message)
+ if code == 1009 then -- stanza size limit exceeded
+ -- we close the session, rather than the connection,
+ -- otherwise a resuming client will simply resend the
+ -- offending stanza
+ session:close({ condition = "policy-violation", text = "stanza too large" });
+ else
+ websocket_close(code, message);
end
+ end
- if opcode == 0x0 and not dataBuffer then
- websocket_close(1002, "Unexpected continuation frame");
- return false;
- end
+ local function handle_frame(frame)
+ module:log("debug", "Websocket received frame: opcode=%0x, %i bytes", frame.opcode, #frame.data);
- if (opcode == 0x1 or opcode == 0x2) and dataBuffer then
- websocket_close(1002, "Continuation frame expected");
- return false;
+ -- Check frame makes sense
+ local frame_ok, err_status, err_text = validate_frame(frame, stanza_size_limit);
+ if not frame_ok then
+ return frame_ok, err_status, err_text;
end
- -- Valid cases
- if opcode == 0x0 then -- Continuation frame
- dataBuffer[#dataBuffer+1] = frame.data;
- elseif opcode == 0x1 then -- Text frame
- dataBuffer = {frame.data};
- elseif opcode == 0x2 then -- Binary frame
- websocket_close(1003, "Only text frames are supported");
- return;
- elseif opcode == 0x8 then -- Close request
- websocket_close(1000, "Goodbye");
- return;
- elseif opcode == 0x9 then -- Ping frame
+ local opcode = frame.opcode;
+ if opcode == 0x9 then -- Ping frame
frame.opcode = 0xA;
frame.MASK = false; -- Clients send masked frames, servers don't, see #1484
conn:write(build_frame(frame));
return "";
elseif opcode == 0xA then -- Pong frame, MAY be sent unsolicited, eg as keepalive
return "";
- else
+ elseif opcode ~= 0x1 then -- Not text frame (which is all we support)
log("warn", "Received frame with unsupported opcode %i", opcode);
return "";
end
- if frame.FIN then
- local data = t_concat(dataBuffer, "");
- dataBuffer = nil;
- return data;
- end
- return "";
+ return frame.data;
end
conn:setlistener(c2s_listener);
@@ -282,19 +291,28 @@ function handle_request(event)
end
local cache = {};
- local frame, length = parse_frame(frameBuffer);
+ local frame, length, partial = parse_frame(frameBuffer);
while frame do
- if length > stanza_size_limit then
- session:close({ condition = "policy-violation", text = "stanza too large" });
- return;
- end
frameBuffer:discard(length);
- local result = handle_frame(frame);
- if not result then break; end
+ local result, err_status, err_text = handle_frame(frame);
+ if not result then
+ websocket_handle_error(session, err_status, err_text);
+ break;
+ end
cache[#cache+1] = filter_open_close(result);
- frame, length = parse_frame(frameBuffer);
+ frame, length, partial = parse_frame(frameBuffer);
end
+
+ if partial then
+ -- The header of the next frame is already in the buffer, run
+ -- some early validation here
+ local frame_ok, err_status, err_text = validate_frame(partial, stanza_size_limit);
+ if not frame_ok then
+ websocket_handle_error(session, err_status, err_text);
+ end
+ end
+
return t_concat(cache, "");
end);