Skip to content

vnc: fix RFB 3.7/3.8 handshake broken by the short-read check - #1116

Merged
vanhauser-thc merged 3 commits into
vanhauser-thc:masterfrom
protonnegativo:fix/vnc-rfb37-handshake
Nov 7, 2026
Merged

vanhauser-thc merged 3 commits into
vanhauser-thc:masterfrom
protonnegativo:fix/vnc-rfb37-handshake

Conversation

@protonnegativo

@protonnegativo protonnegativo commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Since d78db4b ("security fixes") the vnc module fails against every RFB 3.7/3.8 server (i.e. most current VNC servers): each attempt ends with [ERROR] VNC server connection failed (short read) and no password is found.

Cause

start_vnc() now requires the first recv() to return 20 bytes. That is only true for RFB 3.3 with VNC auth (4-byte type + 16-byte challenge). With 3.7/3.8 the server sends just the security type list (1 + n bytes) and then waits for the client to choose a type, so the read is always short. The same commit also changed the type loop from i <= buf2[0] to i < buf2[0], which skips the last offered type (a server offering only type 2 never matches).

Fix

  • Store the recv() result and require only what the protocol version sends at that point: 1 byte for 3.7+, 4 for 3.3 (an RFB 3.3 server with no auth sends only those 4 bytes and was broken too).
  • Fix the type loop back to i <= buf2[0], and read the rest of the type list if it arrives in more than one segment (fail only if the connection closes). The buffer gets one more byte so the largest accepted count (20) fits.
  • In the RFB 3.3 VNC auth branch, read the rest of the 16-byte challenge if it arrives in a separate segment (fail only if the connection closes).

Testing

Mock RFB server (Python, pycryptodome for the DES check), password secret, wordlist a b secret c:

python3 vnc_server.py 47301 3.8 secret 2      # RFB 3.8, offers type 2
python3 vnc_server.py 47302 3.3 secret        # RFB 3.3
python3 vnc_server.py 47303 3.8 secret 16,2   # RFB 3.8, offers 16 and 2
./hydra -P pw.txt -t 1 -s <port> 127.0.0.1 vnc

Before (master):

[ERROR] VNC server connection failed (short read)
[ERROR] VNC server connection failed (short read)
[ERROR] VNC server connection failed (short read)
[ERROR] VNC server connection failed (short read)
1 of 1 target completed, 0 valid password found

(3.3 still worked; with hydra-vnc.c from d78db4b~1 all three were found.)

After:

[47301][vnc] host: 127.0.0.1   password: secret
1 of 1 target successfully completed, 1 valid password found

Same for 47302 and 47303. With a wordlist without the password: 0 valid password found on all three. A 3.8 server offering only type 1 still reports "VNC server does not require authentication."

With SPLIT=1 the mock sends the first field in its own write, 50 ms before the rest (RFB 3.8: the type count, then the types; RFB 3.3: the type, then the challenge). Both failed on master ("short read"); the password is found after, also for 3.8 with 16,2. A 3.8 server that announces 3 types, sends one and closes gives "short read" instead of hanging.

vnc_server.py
#!/usr/bin/env python3
"""Tiny RFB (VNC) server for testing hydra's vnc module.
usage: vnc_server.py PORT VERSION(3.3|3.8) PASSWORD [SECTYPES e.g. 2 or 16,2]
SPLIT=1 sends the first field (type count / security type) in its own segment."""
import os, socket, struct, sys, threading, time
from Crypto.Cipher import DES

port, ver, password = int(sys.argv[1]), sys.argv[2], sys.argv[3].encode()
sectypes = [int(x) for x in (sys.argv[4] if len(sys.argv) > 4 else "2").split(",")]
split = os.environ.get("SPLIT") == "1"


def send(c, head, rest):
    if split:
        c.sendall(head)
        time.sleep(0.05)
        c.sendall(rest)
    else:
        c.sendall(head + rest)


def vnc_key(pw):
    k = (pw + b"\0" * 8)[:8]
    return bytes(int(f"{b:08b}"[::-1], 2) for b in k)


def handle(c):
    try:
        c.sendall(b"RFB 003.008\n" if ver == "3.8" else b"RFB 003.003\n")
        c.recv(12)
        chal = os.urandom(16)
        if ver == "3.8":
            send(c, bytes([len(sectypes)]), bytes(sectypes))
            if c.recv(1) != b"\x02":
                return
            c.sendall(chal)
        else:
            send(c, struct.pack(">I", 2), chal)
        resp = b""
        while len(resp) < 16:
            d = c.recv(16 - len(resp))
            if not d:
                return
            resp += d
        ok = resp == DES.new(vnc_key(password), DES.MODE_ECB).encrypt(chal)
        if ok:
            c.sendall(struct.pack(">I", 0))
        else:
            reason = b"Authentication failed"
            c.sendall(struct.pack(">I", 1) + (struct.pack(">I", len(reason)) + reason if ver == "3.8" else b""))
    finally:
        c.close()


s = socket.socket()
s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
s.bind(("127.0.0.1", port))
s.listen(16)
while True:
    c, _ = s.accept()
    threading.Thread(target=handle, args=(c,), daemon=True).start()

d78db4b made start_vnc() require 20 bytes from the first recv(), but
an RFB 3.7/3.8 server only sends the security type list (1 + n bytes)
and then waits for the client to pick one, so every attempt against
such a server failed with "short read". The same change turned the
security type loop into i < buf2[0], which skips the last type.

Require only the bytes the protocol version actually sends, bound the
type loop by the bytes received, and in the RFB 3.3 VNC auth path read
the rest of the challenge if it arrives in a separate segment instead
of failing.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Fragmented RFB 3.7/3.8 security-type lists still cause valid handshakes to fail.

1 open issue
What changed in this PR

This PR addresses short-read regressions in the VNC module’s RFB security negotiation, restoring support for common RFB 3.7/3.8 handshakes and split RFB 3.3 challenges.

Changes:

  • Apply protocol-specific minimum lengths to the initial security read.
  • Include the last received security type while bounding the scan by received bytes.
  • Accumulate the remaining RFB 3.3 authentication challenge before encryption.
File Description
hydra-vnc.c Updates security negotiation checks, type scanning, and RFB 3.3 challenge reads.

Review effort: Balanced

Comment thread hydra-vnc.c Resolved
With RFB 3.7+ the server may send the type count and the types in
separate segments. Keep reading until all advertised types are in,
and size the buffer for the largest accepted count.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Blocking receives in both new handshake loops can leave workers waiting beyond the configured timeout when a server sends incomplete data and keeps the connection open.

1 open issue
1 resolved since last review

Review effort: Balanced

Comment thread hydra-vnc.c Resolved
The loops that read the rest of the security type list and the rest
of the RFB 3.3 challenge used a blocking recv(), so a server that sent
an incomplete list or challenge and kept the connection open left the
child waiting forever. Use hydra_recv_nb() so these reads honour the
-w wait time and fail the handshake on timeout.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

There are no active review issues for the VNC authentication handshake changes.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

@vanhauser-thc

Copy link
Copy Markdown
Owner

thanks

@vanhauser-thc
vanhauser-thc merged commit 2c18681 into vanhauser-thc:master Nov 7, 2026
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants