One-byte stack overflow in gpredict rotctld client (arm64 stack protector abort)

Kiểm tra rotctld_socket_rw() trong src/gtk-rot-ctrl.c, đặc biệt là các thao tác ghi của recv() và ký tự kết thúc, rồi so sánh với kiểm tra giới hạn hiện có trong src/gtk-rig-ctrl.c. Issue chỉ ra các caller bị ảnh hưởng và một driver cục bộ trong lab/rot_oob.c; hãy tái hiện trường hợp phản hồi 128 byte trước khi sửa helper. Hoàn thành nghĩa là helper không bao giờ ghi vượt qua buffer được cung cấp, xử lý an toàn phản hồi đầy đủ và giữ nguyên hành vi trả về đã được tài liệu hóa.

Summary

get_pos(), set_pos(), and rot_locked_cb() each pass a gchar buffback[128] and a length of 128 into rotctld_socket_rw(). That function does:

size = recv(sock, buffout, sizeout, 0);
buffout[size] = '\0';

recv is allowed to fill all 128 bytes. When it returns 128, buffout[128] is one past the array, and the stored byte is 0x00.

A reply of 127 bytes or fewer writes the NUL inside the array. I measured that. A reply of 128 or more makes recv return 128 and writes the NUL into the next byte. On this machine’s arm64 ABI, with Apple clang at -O0 and at -O2 -fstack-protector-all, that byte is the stack canary. The process dies in __stack_chk_fail (exit status 134). The written byte is always NUL. I did not get a controlled pointer write or instruction-pointer control.

The radio helper in src/gtk-rig-ctrl.c already reserves the extra byte. The rotator helper does not.

gpredict does not open a rotator socket until the user fills in a host and port and presses Engage. The new-rotator dialog defaults the host to localhost and the port to 4533. The exchange is cleartext TCP. This client does not check the peer.

Code

src/gtk-rot-ctrl.c, rotctld_socket_rw(), lines 180 and 189:

size = recv(sock, buffout, sizeout, 0);
...
buffout[size] = '\0';

Callers, all gchar buffback[128] and size 128:

Caller	Line	Command written to the socket
get_pos()	311	p\n (g_strdup_printf("p\x0a"))
set_pos()	371	P <az> <el>\n
rot_locked_cb()	1209	S\n (stop, when Engage is released)

Radio path, src/gtk-rig-ctrl.c line 1492, already correct:

size = recv(sock, buffout, sizeout - 1, 0);
buffout[size] = '\0';

A normal rotctld position reply is two short decimal lines and a RPRT line, far below 128 bytes. The overflow needs a peer that returns at least 128 bytes in one recv. Any process that accepts the TCP connection can do that. The client does not authenticate it.

Local reproduction

GTK and hamlib were not linked. I did not connect to a rotator on the public internet. The send, the recv, and the buffout[size] = '\0' in the driver are the body of rotctld_socket_rw(), with the sat_log_log calls compiled out so the file links. The peer is a socketpair child in the same process.

Build and run

clang -arch arm64 -O2 -g -fstack-protector-all lab/rot_oob.c -o /tmp/rot_oob_arm2
arch -arm64 /tmp/rot_oob_arm2

-O0 arm64 was built and run the same way. x86_64 builds: /tmp/rot_oob and /tmp/rot_oob_o2.

What the client sent

Every shot uses the get_pos() command, 2 bytes, sent with send(sock, buff, strlen(buff), 0):

0000  70 0a                                             p.

ASCII p\n. set_pos / stop use the same recv and the same 128-byte buffer; only the outbound command string differs.

What the peer sent back

The child fills a buffer with Z (0x5a) and sends a chosen length, then shutdown(SHUT_WR).

Reply A, 127 bytes (NUL stays inside the array):

0000  5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a  ZZZZZZZZZZZZZZZZ
...
48 bytes of 0x5a per line, 7 full lines (112 bytes), then:
0070  5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a     ZZZZZZZZZZZZZZZ

Reply B, 128 bytes (the case that writes off the end):

0000  5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a  ZZZZZZZZZZZZZZZZ
...
0070  5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a  ZZZZZZZZZZZZZZZZ

128 × 0x5a. No trailing NUL in the reply. The client adds one, at index 128.

Reply C, 200 bytes of 0x5a. recv is capped by sizeout, so the client still accepts 128 and writes buffout[128].

The neighbor byte next to the 128-byte array was set to 0x42 before each call so a missed write is visible.

Program output (arm64, -O2 -fstack-protector-all)

recv returned 127, sizeout 128
nsend=127 ok=1 neighbor=0x42 buf0=0x5a
recv returned 128, sizeout 128
nsend=128 ok=1 neighbor=0x00 buf0=0x5a
recv returned 128, sizeout 128
nsend=200 ok=1 neighbor=0x00 buf0=0x5a
summary 127=0x42 128=0x00 200=0x00
recv returned 128, sizeout 128

Then the process exits with status 134.

Reply length	recv return	buffout[0]	byte after the array	result
127 × 0x5a	127	0x5a	stays 0x42	write is inside the array
128 × 0x5a	128	0x5a	0x42 → 0x00	one byte past the array
200 × 0x5a	128	0x5a	0x42 → 0x00	same write, extra bytes stay in the socket

buf0=0x5a is the first byte of the reply actually stored, so this is the buffer recv filled, not a stale 0x41 from the memset I did before the call.

Stack protector, arm64

Fourth call uses a frame shaped like get_pos(): a gchar *, a gchar **, a gchar buffback[128], and a gboolean, and it passes 128 as the length. The peer sends reply B (128 × 0x5a).

lldb -- /tmp/rot_oob_arm2
(lldb) breakpoint set -n __stack_chk_fail
(lldb) run

Stopped in libsystem_c.dylib__stack_chk_fail. The caller frame is get_pos_likeinrot_oob.c. The abort happens on return from that function, after the line recv returned 128, sizeout 128`.

arm64 -O0 is the same exit status 134, and it stops at the same point: the 127-byte reply leaves the neighbor at 0x42, the 128-byte reply changes it to 0x00, and the following get_pos_like return is where the protector fires.

Frame layout I read back from this clang:

Build	buffback	canary	buffout[128]
arm64 -O2	sp+24	x29-40, which is sp+152	sp+152
arm64 -O0	sp+40	sp+168	sp+168

In both arm64 builds the NUL lands on the canary.

x86_64, same source, no abort

/tmp/rot_oob and /tmp/rot_oob_o2 still change the neighbor from 0x42 to 0x00 for the 128-byte and 200-byte replies. Apple clang inserts 8 bytes of padding between this 128-byte array and the canary, so the NUL hits the padding. get_pos_like prints get_pos_like returned and does not enter __stack_chk_fail.

The overflow is in the source either way. Whether that one NUL hits the canary depends on how the compiler lays out the frame. The native architecture here is arm64, and that build aborts.

Driver

lab/rot_oob.c. The socket function matches gtk-rot-ctrl.c lines 157–197.

size = strlen(buff);
written = send(sock, buff, size, 0);
size = recv(sock, buffout, sizeout, 0);
fprintf(stderr, "recv returned %d, sizeout %d\n", size, sizeout);
buffout[size] = '\0';          /* size == sizeout is legal for recv */

/* probe: array and the following byte are adjacent */
struct probe { char buf[128]; unsigned char next; };
memset(p.buf, 'A', 128);
p.next = 0x42;
rotctld_socket_rw(sv[0], "p\n", p.buf, 128);

/* canary: locals match get_pos, peer sends 128 bytes of 'Z' */
static void get_pos_like(int sock) {
    gchar *buff, **vbuff;
    gchar buffback[128];
    gboolean retcode;
    buff = "p\n";
    retcode = rotctld_socket_rw(sock, buff, buffback, 128);
}

Suggested fix

Do what _send_rigctld_command() already does:

if (sizeout < 1)
    return FALSE;
size = recv(sock, buffout, sizeout - 1, 0);
if (size < 0)
    return FALSE;
buffout[size] = '\0';

Treat a full buffer as a hard error and drop that reply. Do not assume the peer only returns a few digits.

Credit

Found and reproduced locally against commit da7243fe7aaa14b20782d127b0fa5bd3c21e2dfe. The send bytes, reply bytes, recv returns, neighbor values, and the __stack_chk_fail stop are from that run.