Skip to content

Commit 003a1ff

Browse files
LibretroAdminLibretroAdmin
committed
input/bsv: bound key_event_count and input_event_count against array size
bsv_movie_handle_read_input_event's read path read attacker- controlled event counts straight from the .bsv replay file and used them as loop bounds, indexing into fixed-size arrays: uint8_t key_event_count; (.bsv format) uint16_t input_event_count; (.bsv format) bsv_key_data_t key_events[128]; (struct bsv_movie) bsv_input_data_t input_events[512]; (struct bsv_movie) Pre-this-patch the read loops at lines 822-836 (key) and 848-862 (input) iterated count-times into the fixed-size arrays with no upper-bound check. A malformed .bsv could supply: - key_event_count = 255 -> (255-128)*sizeof(bsv_key_data_t) = 1524 bytes of OOB heap-write past key_events[] - input_event_count = 65535 -> (65535-512)*sizeof(bsv_input_data_t) = 520184 bytes of OOB heap-write past input_events[] bsv_movie_t is heap-allocated via calloc in tasks/task_movie.c::bsv_movie_init_internal so this is a heap-buffer-overflow with attacker-chosen 8- or 12-byte payloads at attacker-chosen offsets, producing corruption of the rest of the bsv_movie_t struct (rewind state, statestream pointers, save buffers) and far beyond it. Reachability: the user opens a malformed .bsv file via Menu -> "Load Replay File" or the --bsvplay command-line argument. Same threat class as CDFS / CHD / BPS file-load bugs -- requires the user to deliberately load attacker- supplied content. Initial audit notes flagged this as netplay-reachable but verification of network/netplay/ netplay_frontend.c shows that netplay's "replay" terminology refers to internal frame-replay from the netplay ring buffer and does not invoke the .bsv file-read path; this is a file-loading bug only. Fix: in input/bsv/bsvmovie.c, reject the file as malformed and end playback if either count exceeds its array's ARRAY_SIZE. A legitimate writer cannot produce more events than the array holds because the writer's append path at lines 489-490 / 475-476 shares the same backing array. Adds samples/tasks/bsv_replay/bsv_replay_bounds_test as a regression test, registered in .github/workflows/Linux-samples-tasks.yml as an ASan-enabled step. Four cases covering legitimate counts up to and including the cap, and out-of-range counts being rejected without writes. Test follows the existing samples/tasks pattern (verbatim copy of the post-fix predicate with maintenance-contract comment, ASan as the discriminator). Verified to fire under ASan when the predicate always returns true: heap-buffer-overflow WRITE of size 1 at offset 1536 of a 1536-byte allocation in simulate_key_event_read. With the predicate the test passes clean. Note on writer side: the append site at line 489 movie->input_events[movie->input_event_count++] = data; has no bound check either, but it's not fed by attacker data and a single frame is unlikely to produce more than 512 events in practice. Not addressed by this commit; left for a separate audit pass.
1 parent cba3a9c commit 003a1ff

4 files changed

Lines changed: 356 additions & 0 deletions

File tree

‎.github/workflows/Linux-samples-tasks.yml‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,3 +150,24 @@ jobs:
150150
test -x input_remap_bounds_test
151151
timeout 60 ./input_remap_bounds_test
152152
echo "[pass] input_remap_bounds_test"
153+
154+
- name: Build and run bsv_replay_bounds_test (ASan)
155+
shell: bash
156+
working-directory: samples/tasks/bsv_replay
157+
run: |
158+
set -eu
159+
# Regression test for the .bsv replay-file event-count
160+
# bounds in input/bsv/bsvmovie.c. Pre-fix a malformed
161+
# replay file with key_event_count > 128 or
162+
# input_event_count > 512 caused a heap-buffer-overflow
163+
# write into bsv_movie_t (a calloc'd struct) with
164+
# attacker-chosen 8/12-byte payloads at attacker-chosen
165+
# offsets up to ~500KB. Build under AddressSanitizer
166+
# so any reintroduction of the unbounded count is caught
167+
# at the bounds level. If input/bsv/bsvmovie.c amends
168+
# the predicate, the verbatim copy in
169+
# bsv_replay_bounds_test.c must follow.
170+
make clean all SANITIZER=address
171+
test -x bsv_replay_bounds_test
172+
timeout 60 ./bsv_replay_bounds_test
173+
echo "[pass] bsv_replay_bounds_test"

‎input/bsv/bsvmovie.c‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -822,6 +822,25 @@ bool bsv_movie_read_next_events(bsv_movie_t *handle,
822822
if (intfstream_read(handle->file, &(handle->key_event_count), 1) == 1)
823823
{
824824
int i;
825+
/* key_events is a fixed-size bsv_key_data_t[128] (input_driver.h
826+
* line ~259); key_event_count is uint8_t and read straight from
827+
* the .bsv file with no inherent bound (max 255). Pre-this-patch
828+
* a malformed replay file could supply 129..255 here and the
829+
* intfstream_read into &handle->key_events[i] would OOB-write up
830+
* to (255-128)*12 = 1524 bytes past the end of the array, into
831+
* adjacent fields of bsv_movie_t (input_event_count,
832+
* input_events[]) and beyond. Reject as malformed -- a
833+
* legitimate writer cannot produce more than 128 key events per
834+
* frame because it shares the same backing array. */
835+
if (handle->key_event_count > ARRAY_SIZE(handle->key_events))
836+
{
837+
RARCH_ERR("[Replay] key_event_count %u exceeds storage capacity %u; rejecting malformed replay\n",
838+
(unsigned)handle->key_event_count,
839+
(unsigned)ARRAY_SIZE(handle->key_events));
840+
if (end_movie)
841+
input_st->bsv_movie_state.flags |= BSV_FLAG_MOVIE_END;
842+
return false;
843+
}
825844
for (i = 0; i < handle->key_event_count; i++)
826845
{
827846
if (intfstream_read(handle->file, &(handle->key_events[i]),
@@ -849,6 +868,27 @@ bool bsv_movie_read_next_events(bsv_movie_t *handle,
849868
{
850869
int i;
851870
handle->input_event_count = swap_if_big16(handle->input_event_count);
871+
/* Same shape as the key_event_count check above: input_events
872+
* is bsv_input_data_t[512] (input_driver.h line ~260) but
873+
* input_event_count is uint16_t read straight from the .bsv
874+
* (max 65535). Pre-this-patch a malformed replay file with
875+
* a count of 65535 would have driven the loop below into
876+
* (65535-512)*8 = 520184 bytes of OOB heap-write past
877+
* input_events[], corrupting the rest of the bsv_movie_t
878+
* struct (rewind state, statestream pointers, save buffers)
879+
* and far beyond. bsv_movie_t is heap-allocated via calloc
880+
* in tasks/task_movie.c::bsv_movie_init_internal so this is
881+
* a heap-buffer-overflow with attacker-chosen 8-byte values
882+
* at attacker-chosen offsets up to ~500KB. */
883+
if (handle->input_event_count > ARRAY_SIZE(handle->input_events))
884+
{
885+
RARCH_ERR("[Replay] input_event_count %u exceeds storage capacity %u; rejecting malformed replay\n",
886+
(unsigned)handle->input_event_count,
887+
(unsigned)ARRAY_SIZE(handle->input_events));
888+
if (end_movie)
889+
input_st->bsv_movie_state.flags |= BSV_FLAG_MOVIE_END;
890+
return false;
891+
}
852892
for (i = 0; i < handle->input_event_count; i++)
853893
{
854894
if (intfstream_read(handle->file, &(handle->input_events[i]),

‎samples/tasks/bsv_replay/Makefile‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
TARGET := bsv_replay_bounds_test
2+
3+
SOURCES := bsv_replay_bounds_test.c
4+
5+
OBJS := $(SOURCES:.c=.o)
6+
7+
CFLAGS += -Wall -pedantic -std=gnu99 -g -O0
8+
9+
ifneq ($(SANITIZER),)
10+
CFLAGS := -fsanitize=$(SANITIZER) -fno-omit-frame-pointer $(CFLAGS)
11+
LDFLAGS := -fsanitize=$(SANITIZER) $(LDFLAGS)
12+
endif
13+
14+
all: $(TARGET)
15+
16+
%.o: %.c
17+
$(CC) -c -o $@ $< $(CFLAGS)
18+
19+
$(TARGET): $(OBJS)
20+
$(CC) -o $@ $^ $(LDFLAGS)
21+
22+
clean:
23+
rm -f $(TARGET) $(OBJS)
24+
25+
.PHONY: clean

‎samples/tasks/bsv_replay/bsv_replay_bounds_test.c‎

Lines changed: 270 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,270 @@
1+
/* Copyright (C) 2010-2026 The RetroArch team
2+
*
3+
* ---------------------------------------------------------------------------------------
4+
* The following license statement only applies to this file (bsv_replay_bounds_test.c).
5+
* ---------------------------------------------------------------------------------------
6+
*
7+
* Permission is hereby granted, free of charge,
8+
* to any person obtaining a copy of this software and associated documentation files (the "Software"),
9+
* to deal in the Software without restriction, including without limitation the rights to
10+
* use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software,
11+
* and to permit persons to whom the Software is furnished to do so, subject to the following conditions:
12+
*
13+
* The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.
14+
*
15+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED,
16+
* INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
17+
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
18+
* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
19+
* WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
20+
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
21+
*/
22+
23+
/* Regression test for the .bsv replay-file event-count bounds in
24+
* input/bsv/bsvmovie.c.
25+
*
26+
* The .bsv replay format stores per-frame input events as:
27+
* uint8_t key_event_count;
28+
* bsv_key_data_t key_events[key_event_count];
29+
* uint16_t input_event_count; (versions > 0)
30+
* bsv_input_data_t input_events[input_event_count];
31+
*
32+
* The runtime stores those into fixed-size arrays:
33+
* bsv_key_data_t key_events[128]; (struct bsv_movie)
34+
* bsv_input_data_t input_events[512]; (struct bsv_movie)
35+
*
36+
* Pre-fix, the read loop iterated count-times into the
37+
* fixed-size array with no upper bound check. A malformed
38+
* .bsv with key_event_count = 255 wrote up to (255-128)*12 =
39+
* 1524 bytes past key_events[]; with input_event_count =
40+
* 65535, up to (65535-512)*8 = 520184 bytes past
41+
* input_events[]. bsv_movie_t is heap-allocated via calloc
42+
* in tasks/task_movie.c so this was a heap-buffer-overflow
43+
* with attacker-chosen 8- or 12-byte payloads at attacker-
44+
* chosen offsets.
45+
*
46+
* Reachability: user opens a malicious .bsv file (Menu ->
47+
* Load Replay File or --bsvplay). Same threat class as
48+
* other file-loading bugs (CDFS / CHD / BPS).
49+
*
50+
* Fix: in input/bsv/bsvmovie.c, reject the file as malformed
51+
* if either count exceeds its array size. A legitimate
52+
* writer cannot produce more events than the array holds
53+
* because the same array backs the writer's append path.
54+
*
55+
* IMPORTANT: this test keeps a verbatim copy of the post-fix
56+
* bound-check predicate. If input/bsv/bsvmovie.c amends the
57+
* predicate, the copy below must follow. Convention used by
58+
* archive_name_safety_test, http_method_match_test,
59+
* video_shader_wildcard_test, input_remap_bounds_test in
60+
* this directory tree.
61+
*/
62+
63+
#include <stdio.h>
64+
#include <stdint.h>
65+
#include <stdlib.h>
66+
#include <string.h>
67+
#include <assert.h>
68+
69+
/* Mirror constants from input/input_driver.h. */
70+
#define KEY_EVENTS_CAP 128
71+
#define INPUT_EVENTS_CAP 512
72+
73+
/* Mirror layout of bsv_key_data and bsv_input_data from
74+
* input/input_driver.h lines 221-240. */
75+
typedef struct {
76+
uint8_t down;
77+
uint8_t _padding;
78+
uint16_t mod;
79+
uint32_t code;
80+
uint32_t character;
81+
} mock_key_data_t;
82+
83+
typedef struct {
84+
uint8_t port;
85+
uint8_t device;
86+
uint8_t idx;
87+
uint8_t _padding;
88+
uint16_t id;
89+
int16_t value;
90+
} mock_input_data_t;
91+
92+
#ifndef ARRAY_SIZE
93+
#define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0]))
94+
#endif
95+
96+
static int failures = 0;
97+
98+
/* === verbatim copy of the post-fix bound-check predicate
99+
* from input/bsv/bsvmovie.c. Returns 1 if the count is
100+
* valid, 0 if it would have OOB-written. If
101+
* bsvmovie.c's predicate amends, this copy must follow.
102+
* === */
103+
static int key_event_count_in_bounds(unsigned key_event_count,
104+
unsigned key_events_cap)
105+
{
106+
if (key_event_count > key_events_cap)
107+
return 0;
108+
return 1;
109+
}
110+
111+
static int input_event_count_in_bounds(unsigned input_event_count,
112+
unsigned input_events_cap)
113+
{
114+
if (input_event_count > input_events_cap)
115+
return 0;
116+
return 1;
117+
}
118+
/* === end verbatim copy === */
119+
120+
/* Drive a write loop the way the production read path does.
121+
* Returns 1 if all writes succeeded, 0 if the bound rejected.
122+
* Used to confirm under ASan that out-of-range counts no
123+
* longer cause OOB writes when the bound fires.
124+
*
125+
* Buffer is sized exactly to the cap so any reintroduction of
126+
* the unbounded loop is flagged by ASan on the first OOB
127+
* index. */
128+
static int simulate_input_event_read(unsigned count)
129+
{
130+
/* Allocate exactly the production cap (no slack) so ASan
131+
* flags the first OOB index. */
132+
mock_input_data_t *events = (mock_input_data_t*)
133+
malloc(sizeof(mock_input_data_t) * INPUT_EVENTS_CAP);
134+
unsigned i;
135+
if (!events)
136+
return 0;
137+
138+
if (!input_event_count_in_bounds(count, INPUT_EVENTS_CAP))
139+
{
140+
free(events);
141+
return 0; /* correctly rejected */
142+
}
143+
144+
for (i = 0; i < count; i++)
145+
{
146+
events[i].port = 1;
147+
events[i].device = 2;
148+
events[i].idx = 3;
149+
events[i].id = 0xCAFE;
150+
events[i].value = 0x1234;
151+
}
152+
153+
free(events);
154+
return 1;
155+
}
156+
157+
static int simulate_key_event_read(unsigned count)
158+
{
159+
mock_key_data_t *events = (mock_key_data_t*)
160+
malloc(sizeof(mock_key_data_t) * KEY_EVENTS_CAP);
161+
unsigned i;
162+
if (!events)
163+
return 0;
164+
165+
if (!key_event_count_in_bounds(count, KEY_EVENTS_CAP))
166+
{
167+
free(events);
168+
return 0;
169+
}
170+
171+
for (i = 0; i < count; i++)
172+
{
173+
events[i].down = 1;
174+
events[i].mod = 0;
175+
events[i].code = 0x1234;
176+
events[i].character = 'A';
177+
}
178+
179+
free(events);
180+
return 1;
181+
}
182+
183+
/* ---- tests ------------------------------------------------ */
184+
185+
static void test_key_count_legitimate(void)
186+
{
187+
/* All counts up to and including the cap must succeed. */
188+
unsigned counts[] = { 0, 1, 64, 127, 128 };
189+
size_t i;
190+
for (i = 0; i < ARRAY_SIZE(counts); i++)
191+
{
192+
if (!simulate_key_event_read(counts[i]))
193+
{
194+
printf("[ERROR] legitimate key_event_count %u rejected\n",
195+
counts[i]);
196+
failures++;
197+
return;
198+
}
199+
}
200+
printf("[SUCCESS] legitimate key_event_counts (0..128) accepted\n");
201+
}
202+
203+
static void test_key_count_oob_rejected(void)
204+
{
205+
/* Any uint8 value above 128 must reject before any write. */
206+
unsigned counts[] = { 129, 200, 254, 255 };
207+
size_t i;
208+
for (i = 0; i < ARRAY_SIZE(counts); i++)
209+
{
210+
if (simulate_key_event_read(counts[i]))
211+
{
212+
printf("[ERROR] OOB key_event_count %u was not rejected\n",
213+
counts[i]);
214+
failures++;
215+
return;
216+
}
217+
}
218+
printf("[SUCCESS] OOB key_event_counts (129..255) rejected without writes\n");
219+
}
220+
221+
static void test_input_count_legitimate(void)
222+
{
223+
unsigned counts[] = { 0, 1, 256, 511, 512 };
224+
size_t i;
225+
for (i = 0; i < ARRAY_SIZE(counts); i++)
226+
{
227+
if (!simulate_input_event_read(counts[i]))
228+
{
229+
printf("[ERROR] legitimate input_event_count %u rejected\n",
230+
counts[i]);
231+
failures++;
232+
return;
233+
}
234+
}
235+
printf("[SUCCESS] legitimate input_event_counts (0..512) accepted\n");
236+
}
237+
238+
static void test_input_count_oob_rejected(void)
239+
{
240+
/* The pathological values: just-over-cap and uint16 max. */
241+
unsigned counts[] = { 513, 1024, 32768, 65534, 65535 };
242+
size_t i;
243+
for (i = 0; i < ARRAY_SIZE(counts); i++)
244+
{
245+
if (simulate_input_event_read(counts[i]))
246+
{
247+
printf("[ERROR] OOB input_event_count %u was not rejected\n",
248+
counts[i]);
249+
failures++;
250+
return;
251+
}
252+
}
253+
printf("[SUCCESS] OOB input_event_counts (513..65535) rejected without writes\n");
254+
}
255+
256+
int main(void)
257+
{
258+
test_key_count_legitimate();
259+
test_key_count_oob_rejected();
260+
test_input_count_legitimate();
261+
test_input_count_oob_rejected();
262+
263+
if (failures)
264+
{
265+
printf("\n%d bsv_replay_bounds test(s) failed\n", failures);
266+
return 1;
267+
}
268+
printf("\nAll bsv_replay_bounds regression tests passed.\n");
269+
return 0;
270+
}

0 commit comments

Comments
 (0)