fix(runtime): validate every screen, depth and visual of the setup reply

parseSetup stopped after the first SCREEN header, so a reply truncated
inside that screen's depths or visuals, or missing a later screen, was
accepted. It now walks all declared screens, each screen's DEPTH headers
and each depth's VISUALTYPEs, checking every extent against the
remaining reply before advancing, and returns ErrMalformed when one
overflows.

Add malformed cases derived from the recorded Xvfb reply by truncation
and length rewrite: missing depths, missing visuals, partial visuals and
a declared second screen that is absent.

commit: c1fb73f69f02fae3908ec81a13272f2520aea4df
author: brice · 11h ago · 2026-10-22 08:19 UTC
committer: brice · 10h ago · 2026-10-22 08:47 UTC
parent: 106b42b
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
checks: 1/2

2 files · +54 -3

M conn_test.go +28 -0


@@ -161,6 +161,22 @@ func TestNewConnMalformed(t *testing.T) {
161	161		binary.LittleEndian.PutUint16(screenPast[6:], 8)
162	162		binary.LittleEndian.PutUint16(screenPast[24:], 0) // vendor_len
163	163		screenPast[29] = 0 // pixmap_formats_len
	164	+	// Walk the recorded reply to find the first screen and its first depth
	165	+	// that carries visuals; every extent is a multiple of 4 bytes.
	166	+	screen := setupFixedSize + pad4(int(binary.LittleEndian.Uint16(setup[24:]))) + formatSize*int(setup[29])
	167	+	visualDepth := screen + screenFixedSize
	168	+	for binary.LittleEndian.Uint16(setup[visualDepth+2:]) == 0 {
	169	+	visualDepth += depthFixedSize
	170	+	}
	171	+	// Screen header present, its allowed depths cut off.
	172	+	missingDepths := truncateSetup(setup, screen+screenFixedSize)
	173	+	// A depth header announcing visuals, the visuals cut off.
	174	+	missingVisuals := truncateSetup(setup, visualDepth+depthFixedSize)
	175	+	// One visual of the depth present, the rest cut off.
	176	+	partialVisuals := truncateSetup(setup, visualDepth+depthFixedSize+visualSize)
	177	+	// Complete first screen, a second screen declared but absent.
	178	+	missingScreen := bytes.Clone(setup)
	179	+	missingScreen[28] = 2
164	180		// Header claims 8 bytes of body, failure reason claims 20.
165	181		reasonPast := []byte{0, 20, 11, 0, 0, 0, 2, 0, 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h'}
166	182		// Success header whose length is shorter than the fixed Setup.

@@ -177,6 +193,10 @@ func TestNewConnMalformed(t *testing.T) {
177	193		{"protocol major", badVersion},
178	194		{"unknown status", badStatus},
179	195		{"screen past end", screenPast},
	196	+	{"missing depths", missingDepths},
	197	+	{"missing visuals", missingVisuals},
	198	+	{"partial visuals", partialVisuals},
	199	+	{"missing subsequent screen", missingScreen},
180	200		{"reason past end", reasonPast},
181	201		{"short success", shortSuccess},
182	202		} {

@@ -190,6 +210,14 @@ func TestNewConnMalformed(t *testing.T) {
190	210		}
191	211		}
192	212
	213	+	// truncateSetup keeps the first n bytes (n ≡ 0 mod 4) of a setup reply and
	214	+	// rewrites its additional-data length to match.
	215	+	func truncateSetup(setup []byte, n int) []byte {
	216	+	p := bytes.Clone(setup[:n])
	217	+	binary.LittleEndian.PutUint16(p[6:], uint16((n-setupHeaderSize)/4))
	218	+	return p
	219	+	}
	220	+
193	221		func TestConnFlushClose(t *testing.T) {
194	222		fds, err := unix.Socketpair(unix.AF_UNIX, unix.SOCK_STREAM\|unix.SOCK_CLOEXEC, 0)
195	223		if err != nil {


@@ -161,6 +161,22 @@ func TestNewConnMalformed(t *testing.T) {
161	binary.LittleEndian.PutUint16(screenPast[6:], 8)	161		binary.LittleEndian.PutUint16(screenPast[6:], 8)
162	binary.LittleEndian.PutUint16(screenPast[24:], 0) // vendor_len	162		binary.LittleEndian.PutUint16(screenPast[24:], 0) // vendor_len
163	screenPast[29] = 0 // pixmap_formats_len	163		screenPast[29] = 0 // pixmap_formats_len
		164	+	// Walk the recorded reply to find the first screen and its first depth
		165	+	// that carries visuals; every extent is a multiple of 4 bytes.
		166	+	screen := setupFixedSize + pad4(int(binary.LittleEndian.Uint16(setup[24:]))) + formatSize*int(setup[29])
		167	+	visualDepth := screen + screenFixedSize
		168	+	for binary.LittleEndian.Uint16(setup[visualDepth+2:]) == 0 {
		169	+	visualDepth += depthFixedSize
		170	+	}
		171	+	// Screen header present, its allowed depths cut off.
		172	+	missingDepths := truncateSetup(setup, screen+screenFixedSize)
		173	+	// A depth header announcing visuals, the visuals cut off.
		174	+	missingVisuals := truncateSetup(setup, visualDepth+depthFixedSize)
		175	+	// One visual of the depth present, the rest cut off.
		176	+	partialVisuals := truncateSetup(setup, visualDepth+depthFixedSize+visualSize)
		177	+	// Complete first screen, a second screen declared but absent.
		178	+	missingScreen := bytes.Clone(setup)
		179	+	missingScreen[28] = 2
164	// Header claims 8 bytes of body, failure reason claims 20.	180		// Header claims 8 bytes of body, failure reason claims 20.
165	reasonPast := []byte{0, 20, 11, 0, 0, 0, 2, 0, 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h'}	181		reasonPast := []byte{0, 20, 11, 0, 0, 0, 2, 0, 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h'}
166	// Success header whose length is shorter than the fixed Setup.	182		// Success header whose length is shorter than the fixed Setup.

@@ -177,6 +193,10 @@ func TestNewConnMalformed(t *testing.T) {
177	{"protocol major", badVersion},	193		{"protocol major", badVersion},
178	{"unknown status", badStatus},	194		{"unknown status", badStatus},
179	{"screen past end", screenPast},	195		{"screen past end", screenPast},
		196	+	{"missing depths", missingDepths},
		197	+	{"missing visuals", missingVisuals},
		198	+	{"partial visuals", partialVisuals},
		199	+	{"missing subsequent screen", missingScreen},
180	{"reason past end", reasonPast},	200		{"reason past end", reasonPast},
181	{"short success", shortSuccess},	201		{"short success", shortSuccess},
182	} {	202		} {

@@ -190,6 +210,14 @@ func TestNewConnMalformed(t *testing.T) {
190	}	210		}
191	}	211		}
192		212
		213	+	// truncateSetup keeps the first n bytes (n ≡ 0 mod 4) of a setup reply and
		214	+	// rewrites its additional-data length to match.
		215	+	func truncateSetup(setup []byte, n int) []byte {
		216	+	p := bytes.Clone(setup[:n])
		217	+	binary.LittleEndian.PutUint16(p[6:], uint16((n-setupHeaderSize)/4))
		218	+	return p
		219	+	}
		220	+
193	func TestConnFlushClose(t *testing.T) {	221		func TestConnFlushClose(t *testing.T) {
194	fds, err := unix.Socketpair(unix.AF_UNIX, unix.SOCK_STREAM\|unix.SOCK_CLOEXEC, 0)	222		fds, err := unix.Socketpair(unix.AF_UNIX, unix.SOCK_STREAM\|unix.SOCK_CLOEXEC, 0)
195	if err != nil {	223		if err != nil {

M setup.go +26 -3


@@ -15,6 +15,8 @@ const (
15	15		setupFixedSize = 40 // fixed part of a successful Setup
16	16		screenFixedSize = 40 // fixed part of a SCREEN
17	17		formatSize = 8 // FORMAT
	18	+	depthFixedSize = 8 // fixed part of a DEPTH
	19	+	visualSize = 24 // VISUALTYPE
18	20		protocolMajor = 11
19	21		protocolMinor = 0
20	22		)

@@ -111,10 +113,31 @@ func parseSetup(p []byte) (setupInfo, error) {
111	113		return setupInfo{}, malformed("setup has no screens")
112	114		}
113	115		off := setupFixedSize + pad4(int(binary.LittleEndian.Uint16(p[24:]))) + formatSize*int(p[29])
114		-	if off+screenFixedSize > len(p) {
115		-	return setupInfo{}, malformed("first screen overflows setup reply")
	116	+	if off > len(p) {
	117	+	return setupInfo{}, malformed("vendor or pixmap formats overflow setup reply")
	118	+	}
	119	+	roots := int(p[28])
	120	+	for i := range roots {
	121	+	if len(p)-off < screenFixedSize {
	122	+	return setupInfo{}, malformed(fmt.Sprintf("screen %d overflows setup reply", i))
	123	+	}
	124	+	if i == 0 {
	125	+	info.root = binary.LittleEndian.Uint32(p[off:])
	126	+	}
	127	+	depths := int(p[off+39])
	128	+	off += screenFixedSize
	129	+	for range depths {
	130	+	if len(p)-off < depthFixedSize {
	131	+	return setupInfo{}, malformed(fmt.Sprintf("screen %d depth overflows setup reply", i))
	132	+	}
	133	+	visuals := visualSize * int(binary.LittleEndian.Uint16(p[off+2:]))
	134	+	off += depthFixedSize
	135	+	if len(p)-off < visuals {
	136	+	return setupInfo{}, malformed(fmt.Sprintf("screen %d visuals overflow setup reply", i))
	137	+	}
	138	+	off += visuals
	139	+	}
116	140		}
117		-	info.root = binary.LittleEndian.Uint32(p[off:])
118	141		return info, nil
119	142		}
120	143


@@ -15,6 +15,8 @@ const (
15		setupFixedSize = 40 // fixed part of a successful Setup	15		setupFixedSize = 40 // fixed part of a successful Setup
16		screenFixedSize = 40 // fixed part of a SCREEN	16		screenFixedSize = 40 // fixed part of a SCREEN
17		formatSize = 8 // FORMAT	17		formatSize = 8 // FORMAT
			18	+	depthFixedSize = 8 // fixed part of a DEPTH
			19	+	visualSize = 24 // VISUALTYPE
18		protocolMajor = 11	20		protocolMajor = 11
19		protocolMinor = 0	21		protocolMinor = 0
20		)	22		)

@@ -111,10 +113,31 @@ func parseSetup(p []byte) (setupInfo, error) {
111		return setupInfo{}, malformed("setup has no screens")	113		return setupInfo{}, malformed("setup has no screens")
112		}	114		}
113		off := setupFixedSize + pad4(int(binary.LittleEndian.Uint16(p[24:]))) + formatSize*int(p[29])	115		off := setupFixedSize + pad4(int(binary.LittleEndian.Uint16(p[24:]))) + formatSize*int(p[29])
114	-	if off+screenFixedSize > len(p) {	116	+	if off > len(p) {
115	-	return setupInfo{}, malformed("first screen overflows setup reply")	117	+	return setupInfo{}, malformed("vendor or pixmap formats overflow setup reply")
			118	+	}
			119	+	roots := int(p[28])
			120	+	for i := range roots {
			121	+	if len(p)-off < screenFixedSize {
			122	+	return setupInfo{}, malformed(fmt.Sprintf("screen %d overflows setup reply", i))
			123	+	}
			124	+	if i == 0 {
			125	+	info.root = binary.LittleEndian.Uint32(p[off:])
			126	+	}
			127	+	depths := int(p[off+39])
			128	+	off += screenFixedSize
			129	+	for range depths {
			130	+	if len(p)-off < depthFixedSize {
			131	+	return setupInfo{}, malformed(fmt.Sprintf("screen %d depth overflows setup reply", i))
			132	+	}
			133	+	visuals := visualSize * int(binary.LittleEndian.Uint16(p[off+2:]))
			134	+	off += depthFixedSize
			135	+	if len(p)-off < visuals {
			136	+	return setupInfo{}, malformed(fmt.Sprintf("screen %d visuals overflow setup reply", i))
			137	+	}
			138	+	off += visuals
			139	+	}
116		}	140		}
117	-	info.root = binary.LittleEndian.Uint32(p[off:])
118		return info, nil	141		return info, nil
119		}	142		}
120			143