Built-in WebSockets: unmasked client frames are accepted (RFC 6455 §5.1)

[UWCSZhou]

Summary

The broker's built-in WebSockets transport (WITH_WEBSOCKETS_BUILTIN, the default) does not enforce the RFC 6455 masking rule. §5.1 states:

A client MUST mask all frames that it sends to the server … A server MUST close the connection upon receiving a frame that is not masked.

lib/net_ws.c never checks the mask bit. It reads a masking key only when the bit is set, and unconditionally XORs the payload with mosq->wsd.maskingkey. Two observable consequences:

1. The conformance violation: an unmasked client frame is accepted. An unmasked CONNECT is answered with CONNACK 0x00 instead of the connection being failed. (Deterministic.)
2. A direct consequence: because the masking key is only written when the mask bit is set and is never cleared between frames, an unmasked frame that arrives after a masked one is XORed with the previous frame's stale key and silently corrupted, breaking the session. On a fresh connection the key is zero-initialised (calloc), so the first unmasked frame XORs with zero (identity) and passes through intact — which is why the violation is silent rather than an obvious failure.

This is a protocol-conformance / robustness issue, not a memory-safety bug. (Related in file to the empty-frame report #3704 — same reader — but a distinct root cause and fix.)

Version / platform

• mosquitto 2.1.2 / current master.
• WITH_WEBSOCKETS=ON, WITH_WEBSOCKETS_BUILTIN=ON (defaults). The libwebsockets backend is a separate code path and is not affected.
• Linux.

Steps to reproduce

Run a broker with a websockets listener:

listener 1884 127.0.0.1
protocol websockets
allow_anonymous true
persistence false

Minimal self-contained reproducer (Python stdlib only), also attached as mini_repro.py:

import base64, os, socket, struct, sys, time
host = sys.argv[1] if len(sys.argv) > 1 else "127.0.0.1"
port = int(sys.argv[2]) if len(sys.argv) > 2 else 1884

def ws(payload, mask=True, opcode=0x2):
    b = bytes([0x80 | opcode]); n = len(payload); mb = 0x80 if mask else 0x00
    b += bytes([mb | n]) if n < 126 else bytes([mb | 126]) + struct.pack("!H", n)
    if not mask: return b + payload
    k = os.urandom(4); return b + k + bytes(c ^ k[i % 4] for i, c in enumerate(payload))

def mqtt(cmd, body): return bytes([cmd, len(body)]) + body
def s(x): return struct.pack("!H", len(x)) + x.encode()
def connect(cid): return mqtt(0x10, s("MQTT")+bytes([5,2])+struct.pack("!H",60)+b"\x00"+s(cid))

def handshake(sock):
    key = base64.b64encode(os.urandom(16)).decode()
    sock.sendall((f"GET /mqtt HTTP/1.1\r\nHost: {host}\r\nUpgrade: websocket\r\n"
                  f"Connection: Upgrade\r\nSec-WebSocket-Key: {key}\r\n"
                  f"Sec-WebSocket-Version: 13\r\nSec-WebSocket-Protocol: mqtt\r\n\r\n").encode())
    time.sleep(0.2); sock.recv(4096)

def connack_ok(r):
    i = r.find(b"\x20"); return i != -1 and i+3 < len(r) and r[i+3] == 0x00

# unmasked CONNECT as the first frame -> should be rejected per RFC 6455 5.1
sock = socket.create_connection((host, port), 5); handshake(sock)
sock.sendall(ws(connect("u"), mask=False)); sock.settimeout(1.0)
try: r = sock.recv(4096)
except socket.timeout: r = b""
sock.close()
print("unmasked CONNECT ->", "ACCEPTED (CONNACK 0x00)" if connack_ok(r) else f"rejected/closed ({r!r})")

# gate: a properly masked CONNECT must be accepted
sock = socket.create_connection((host, port), 5); handshake(sock)
sock.sendall(ws(connect("g"), mask=True)); sock.settimeout(1.0)
try: r = sock.recv(4096)
except socket.timeout: r = b""
sock.close()
print("masked   CONNECT ->", "accepted" if connack_ok(r) else f"UNEXPECTED ({r!r})")

Expected output:

unmasked CONNECT -> ACCEPTED (CONNACK 0x00)
masked   CONNECT -> accepted

The masked control proves the sequence is otherwise valid, so the only difference between the (wrongly) accepted and the expected-rejected case is the mask bit.

Root cause

In lib/net_ws.c, read_ws_payloadlen_short() records the mask bit but never rejects mask == 0:

mosq->wsd.mask = (hbuf & 0x80) >> 7;
plen = hbuf & 0x7F;

read_ws_mask() is called only when mosq->wsd.mask == 1, so for an unmasked frame maskingkey retains whatever it held before. The payload de-mask loop in net__read_ws() always runs:

((uint8_t *)buf)[i] ^= mosq->wsd.maskingkey[(i+mosq->wsd.pos)%4];

so an unmasked frame is XORed with a stale key — zero on a fresh connection (hence accepted intact), the previous frame's key afterwards (hence silently corrupted).

Suggested fix

Reject an unmasked frame the moment the mask bit is found clear, matching the reader's existing error style:

mosq->wsd.mask = (hbuf & 0x80) >> 7;
if(mosq->wsd.mask == 0){
    /* RFC 6455 5.1: a server MUST close the connection on receipt of a frame
     * that is not masked. */
    mosq->wsd.disconnect_reason = 0xEA;
    errno = EPROTO;
    return -1;
}
plen = hbuf & 0x7F;

Verified locally: 10/10 unmasked CONNECTs are rejected after the patch, and the masked gate still connects. Conformant clients always mask, so this rejects nothing legitimate.