fix(bot-screen): bound RFB clipboard buffering
Reject oversized positive and extended clipboard lengths at the header. Process coalesced WebSocket data in bounded slices without rejecting valid multi-message frames. Includes fragmented-header and boundary regressions. Addresses the clipboard finding reported by carlotestor and corroborated by helix4u and other reviewers on #108914. (cherry picked from commit b1fbe363266e6d6fa3d73d2605fe1ca383607859)
This commit is contained in:
40
tests/tools/test_bot_desktop_rfb_limits.py
Normal file
40
tests/tools/test_bot_desktop_rfb_limits.py
Normal file
@@ -0,0 +1,40 @@
|
||||
"""Untrusted RFB clipboard lengths are bounded without breaking stream framing."""
|
||||
|
||||
import pytest
|
||||
|
||||
from tools.bot_desktop.rfb_filter import RfbClientFilter
|
||||
|
||||
_HANDSHAKE = b"RFB 003.008\n\x01\x01"
|
||||
_CLIPBOARD_LIMIT = 256 * 1024
|
||||
|
||||
|
||||
def clipboard_header(length):
|
||||
return b"\x06\x00\x00\x00" + length.to_bytes(4, "big", signed=True)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("length", [_CLIPBOARD_LIMIT + 1, -_CLIPBOARD_LIMIT - 1, 2**31 - 1, -(2**31)])
|
||||
@pytest.mark.parametrize("holder", [False, True])
|
||||
def test_oversized_clipboard_is_rejected_at_header_without_waiting_for_payload(length, holder):
|
||||
parser = RfbClientFilter(lambda: holder)
|
||||
parser.feed(_HANDSHAKE)
|
||||
header = clipboard_header(length)
|
||||
for byte in header[:-1]:
|
||||
assert parser.feed(bytes([byte])) == b""
|
||||
with pytest.raises(ValueError, match="clipboard"):
|
||||
parser.feed(header[-1:])
|
||||
|
||||
|
||||
@pytest.mark.parametrize("extended", [False, True])
|
||||
@pytest.mark.parametrize("holder", [False, True])
|
||||
def test_bounded_clipboards_and_watch_requests_survive_fragmentation_and_large_coalesced_chunks(extended, holder):
|
||||
parser = RfbClientFilter(lambda: holder)
|
||||
assert parser.feed(_HANDSHAKE) == _HANDSHAKE
|
||||
payload = b"x" * _CLIPBOARD_LIMIT
|
||||
message = clipboard_header(-len(payload) if extended else len(payload)) + payload
|
||||
refresh = b"\x03\x00" + b"\x00" * 8
|
||||
# A WebSocket frame can contain several valid messages, not just one.
|
||||
stream = message + refresh + message + refresh
|
||||
received = parser.feed(stream[:7]) + parser.feed(stream[7:])
|
||||
expected = (message + refresh) * 2 if holder else refresh * 2
|
||||
assert received == expected
|
||||
assert parser.feed(refresh) == refresh
|
||||
@@ -30,6 +30,11 @@ _SET_ENCODINGS = 2
|
||||
_CLIENT_CUT_TEXT = 6
|
||||
_FENCE = 248
|
||||
|
||||
# Match TigerVNC's default MaxCutText. SetEncodings has a 16-bit count; every
|
||||
# accepted message must fit in this buffer even when the WebSocket coalesces many.
|
||||
_MAX_CUT_TEXT = 256 * 1024
|
||||
_MAX_BUFFER = max(8 + _MAX_CUT_TEXT, 4 + 4 * 0xFFFF)
|
||||
|
||||
|
||||
class RfbClientFilter:
|
||||
"""Feed client bytes with :meth:`feed`; get back the bytes allowed to reach Xvnc.
|
||||
@@ -44,6 +49,18 @@ class RfbClientFilter:
|
||||
self._handshake_left = 12 + 1 + 1 # version + security type + ClientInit(shared flag)
|
||||
|
||||
def feed(self, chunk: bytes) -> bytes:
|
||||
out = bytearray()
|
||||
offset = 0
|
||||
while offset < len(chunk):
|
||||
room = _MAX_BUFFER - len(self._buf)
|
||||
if room <= 0:
|
||||
raise ValueError("RFB client message exceeds buffer limit")
|
||||
end = min(len(chunk), offset + room)
|
||||
out += self._feed(chunk[offset:end])
|
||||
offset = end
|
||||
return bytes(out)
|
||||
|
||||
def _feed(self, chunk: bytes) -> bytes:
|
||||
self._buf += chunk
|
||||
out = bytearray()
|
||||
if self._handshake_left:
|
||||
@@ -84,6 +101,8 @@ class RfbClientFilter:
|
||||
return None
|
||||
n = int.from_bytes(self._buf[4:8], "big", signed=True)
|
||||
# Extended clipboard (RFB 3.8 + TigerVNC): negative length, |n| bytes follow.
|
||||
if abs(n) > _MAX_CUT_TEXT:
|
||||
raise ValueError("RFB clipboard exceeds 256 KiB limit")
|
||||
return 8 + abs(n)
|
||||
if t == _FENCE:
|
||||
if len(self._buf) < 9:
|
||||
|
||||
Reference in New Issue
Block a user