Skip to content

Commit 1b150f1

Browse files
Recover undercounted discovery_message signals (#25)
* Recover undercounted discovery_message signals A Minecraft client sending a non-trickle answer with all ICE candidates inline undercounts the length prefixes it writes, so protodef truncates the parsed signal and setRemoteDescription fails. The full payload is present in the checksum-validated datagram, so read the discovery_message data field to the end of the buffer. Scoped to discovery_message and assumes one encapsulated message per datagram; adds synthetic regression fixtures for outer-only, inner-only, valid, and bad-checksum cases. * Scope LAN answer recovery and cover framing boundaries --------- Co-authored-by: extremeheat <extreme@protonmail.ch>
1 parent 17ecb71 commit 1b150f1

2 files changed

Lines changed: 134 additions & 1 deletion

File tree

‎src/util.js‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
const { encrypt, calculateChecksum, decrypt } = require('./crypto')
22

3+
// Outer length + type + sender + reserved + recipient + string length.
4+
const DISCOVERY_MESSAGE_DATA_OFFSET = 2 + 2 + 8 + 8 + 8 + 4
5+
36
const getRandomUint64 = () => {
47
const high = Math.floor(Math.random() * 0xFFFFFFFF)
58
const low = Math.floor(Math.random() * 0xFFFFFFFF)
@@ -62,8 +65,18 @@ const processSecurePacket = (buffer, deserializer) => {
6265
}
6366

6467
const packet = deserializer.parsePacketBuffer(decryptedData)
68+
const { name, params } = packet.data
69+
70+
// Retail LAN hosts can undercount the length of an inline-candidate SDP answer
71+
// (#23). Recover its tail from this checksum-validated datagram only.
72+
if (name === 'discovery_message' && params.data.startsWith('CONNECTRESPONSE ') && packet.metadata.size < decryptedData.length) {
73+
const data = decryptedData.subarray(DISCOVERY_MESSAGE_DATA_OFFSET)
74+
// SDP has no NUL bytes. An appended discovery frame does (its uint16 type),
75+
// so do not absorb binary framing into the answer. This is not batch parsing.
76+
if (!data.includes(0)) params.data = data.toString('utf8')
77+
}
6578

66-
return { name: packet.data.name, params: packet.data.params }
79+
return { name, params }
6780
}
6881

6982
module.exports = {

‎test/truncation.test.js‎

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
/* eslint-env mocha */
2+
// Regression for the discovery_message length-prefix compatibility fix. A Minecraft client that sends a non-trickle
3+
// answer with all ICE candidates inline undercounts the length prefixes it writes, so protodef truncates the parsed
4+
// signal even though the whole payload is present in the checksum-validated datagram. These fixtures are synthetic
5+
// (no real identity assertions or ICE credentials): a valid packet is built with our own serializer, then its length
6+
// prefixes are deliberately shortened to reproduce the undercount without any fragmentation machinery.
7+
const assert = require('assert')
8+
const { createSerializer, createDeserializer } = require('../src/serializer')
9+
const { encrypt, calculateChecksum } = require('../src/crypto')
10+
const { createPacketData, processSecurePacket } = require('../src/util')
11+
12+
// Byte layout of a discovery_message plaintext (little-endian): outer encapsulated lu16 (2) + type lu16 (2) +
13+
// sender_id lu64 (8) + reserved (8) + recipient_id lu64 (8) + data pstring lu32 (4) + data.
14+
const OUTER_LEN_OFFSET = 0
15+
const DATA_LEN_OFFSET = 28
16+
const DATA_OFFSET = 32
17+
18+
// A large answer with many inline candidates and a full SDP trailer, comfortably over the ~244-byte shortfall.
19+
function buildLargeSignal () {
20+
const cands = []
21+
for (let i = 0; i < 10; i++) cands.push(`a=candidate:${1000000000 + i} 1 udp ${2122000000 + i} 10.0.0.${i} ${50000 + i} typ host generation 0 network-id ${i}`)
22+
const sdp = ['v=0', 'o=- 1 2 IN IP4 127.0.0.1', 's=-', 't=0 0', 'a=group:BUNDLE 0', 'a=extmap-allow-mixed', 'a=msid-semantic: WMS', 'm=application 50000 UDP/DTLS/SCTP webrtc-datachannel', 'c=IN IP4 10.0.0.1', ...cands, 'a=ice-ufrag:abcd', 'a=ice-pwd:0123456789abcdef0123456789abcdef', 'a=ice-options:trickle', 'a=fingerprint:sha-256 00:11:22:33:44:55:66:77:88:99:AA:BB:CC:DD:EE:FF:00:11:22:33:44:55:66:77:88:99:AA:BB:CC:DD:EE:FF', 'a=setup:active', 'a=mid:0', 'a=sctp-port:5000', 'a=max-message-size:262144'].join('\r\n')
23+
return `CONNECTRESPONSE 12066214442905225968 ${sdp}`
24+
}
25+
26+
function plaintextFor (signal) {
27+
const serializer = createSerializer()
28+
return serializer.createPacketBuffer(createPacketData('discovery_message', 2, 111n, { recipient_id: 222n, data: signal }))
29+
}
30+
31+
function secureFrom (plaintext) {
32+
return Buffer.concat([calculateChecksum(plaintext), encrypt(plaintext)])
33+
}
34+
35+
describe('discovery_message length-prefix truncation', () => {
36+
const deserializer = createDeserializer()
37+
const signal = buildLargeSignal()
38+
const plaintext = plaintextFor(signal)
39+
40+
it('lays the data field out at the documented offsets', () => {
41+
// Confirms the byte accounting the fix relies on. The outer lu16 wraps the 30-byte inner header plus the data.
42+
const dataBytes = Buffer.byteLength(signal, 'utf8')
43+
assert.strictEqual(plaintext.readUInt32LE(DATA_LEN_OFFSET), dataBytes, 'data lu32 counts the signal bytes')
44+
assert.strictEqual(plaintext.readUInt16LE(OUTER_LEN_OFFSET), 30 + dataBytes, 'outer lu16 = 30-byte inner header + data')
45+
assert.strictEqual(plaintext.toString('utf8', DATA_OFFSET), signal, 'data begins at offset 32')
46+
assert.ok(dataBytes > 1200, 'fixture is large enough to exercise the undercount')
47+
})
48+
49+
it('returns a correctly framed packet unchanged', () => {
50+
const out = processSecurePacket(secureFrom(plaintext), deserializer)
51+
assert.strictEqual(out.name, 'discovery_message')
52+
assert.strictEqual(out.params.data, signal)
53+
})
54+
55+
it('recovers the full signal when only the inner data lu32 is undercounted', () => {
56+
const short = Buffer.from(plaintext)
57+
short.writeUInt32LE(1140, DATA_LEN_OFFSET) // client-style undercount, cutting the SDP trailer
58+
const out = processSecurePacket(secureFrom(short), deserializer)
59+
assert.strictEqual(Buffer.byteLength(out.params.data, 'utf8'), Buffer.byteLength(signal, 'utf8'))
60+
assert.strictEqual(out.params.data, signal)
61+
})
62+
63+
it('recovers the full signal when both the outer lu16 and inner lu32 are undercounted', () => {
64+
const short = Buffer.from(plaintext)
65+
short.writeUInt16LE(1172, OUTER_LEN_OFFSET)
66+
short.writeUInt32LE(1140, DATA_LEN_OFFSET)
67+
const out = processSecurePacket(secureFrom(short), deserializer)
68+
assert.strictEqual(out.params.data, signal)
69+
})
70+
71+
it('does not extend a small, correctly counted discovery_message', () => {
72+
const small = 'CANDIDATEADD 12066214442905225968 candidate:1 1 udp 2122000000 10.0.0.1 50000 typ host'
73+
const out = processSecurePacket(secureFrom(plaintextFor(small)), deserializer)
74+
assert.strictEqual(out.params.data, small)
75+
})
76+
77+
it('ignores an undercounted outer length when the inner length is correct', () => {
78+
const short = Buffer.from(plaintext)
79+
short.writeUInt16LE(1172, OUTER_LEN_OFFSET)
80+
assert.strictEqual(processSecurePacket(secureFrom(short), deserializer).params.data, signal)
81+
})
82+
83+
it('uses byte boundaries when the prefix splits the final UTF-8 character', () => {
84+
const text = 'CONNECTRESPONSE 42 v=0\r\ns=é'
85+
const short = plaintextFor(text)
86+
short.writeUInt32LE(Buffer.byteLength(text) - 1, DATA_LEN_OFFSET)
87+
assert.strictEqual(processSecurePacket(secureFrom(short), deserializer).params.data, text)
88+
})
89+
90+
it('does not extend other signalling messages', () => {
91+
const text = signal.replace('CONNECTRESPONSE', 'CONNECTREQUEST')
92+
const short = plaintextFor(text)
93+
short.writeUInt32LE(100, DATA_LEN_OFFSET)
94+
assert.strictEqual(processSecurePacket(secureFrom(short), deserializer).params.data, text.slice(0, 100))
95+
})
96+
97+
it('does not extend discovery advertisements', () => {
98+
const advertisement = createSerializer().createPacketBuffer(createPacketData('discovery_response', 1, 111n, { data: signal }))
99+
advertisement.writeUInt32LE(100, 20)
100+
assert.strictEqual(processSecurePacket(secureFrom(advertisement), deserializer).params.data, signal.slice(0, 100))
101+
})
102+
103+
it('does not absorb a second encapsulated discovery frame', () => {
104+
const batched = Buffer.concat([plaintext, plaintextFor('Ping')])
105+
assert.strictEqual(processSecurePacket(secureFrom(batched), deserializer).params.data, signal)
106+
})
107+
108+
it('does not recover a short answer across an appended binary frame', () => {
109+
const short = Buffer.from(plaintext)
110+
short.writeUInt32LE(100, DATA_LEN_OFFSET)
111+
const batched = Buffer.concat([short, plaintextFor('Ping')])
112+
assert.strictEqual(processSecurePacket(secureFrom(batched), deserializer).params.data, signal.slice(0, 100))
113+
})
114+
115+
it('still rejects an invalid checksum', () => {
116+
const bad = secureFrom(plaintext)
117+
bad[0] ^= 0xff
118+
assert.throws(() => processSecurePacket(bad, deserializer), /Checksum mismatch/)
119+
})
120+
})

0 commit comments

Comments
 (0)