Skip to content

Commit b2d4002

Browse files
committed
libretro-common: harden archive/stream parsers + CI regressions
Three independent memory-safety / DoS fixes under libretro-common, their regression tests, and a CI workflow update. samples/net: fix undefined references in Makefile The net/ sample Makefile listed only a subset of the sources required to link its three binaries -- make all failed on strlcpy_retro__, string_list_*, fill_pathname_resolve_relative, cpu_features_get_time_usec, getnameinfo_retro. Add the missing libretro-common sources so http_test, http_parse_test and net_ifinfo build on a vanilla Linux host. file/archive_file_zstd: guard content_size truncation and overflow zstd_parse_file_iterate_step truncated the 64-bit content_size into a uint32_t without range check; a crafted .zst declaring content_size >= 2^32 truncated low-32 while ZSTD_decompress still
1 parent 1d79437 commit b2d4002

8 files changed

Lines changed: 451 additions & 19 deletions

File tree

.github/workflows/Linux-libretro-common-samples.yml

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,13 @@ jobs:
5151
snprintf
5252
unbase64_test
5353
archive_zip_test
54+
archive_zstd_test
5455
config_file_test
5556
path_resolve_realpath_test
5657
nbio_test
5758
rpng
59+
rzip_chunk_size_test
60+
net_ifinfo
5861
)
5962
6063
# Per-binary run command (overrides ./<binary> if present).
@@ -72,18 +75,11 @@ jobs:
7275
# regressions but not executed.
7376
declare -a BUILD_ONLY_DIRS=(
7477
formats/xml
75-
streams/rzip
7678
)
7779
7880
# Samples that are currently broken at build time on a stock
79-
# Ubuntu host and are therefore neither built nor run. The
80-
# net/ Makefile references symbols (fill_pathname_resolve_relative,
81-
# cpu_features_get_time_usec) without adding the corresponding
82-
# source files to its SOURCES list; fixing that is out of
83-
# scope for this workflow. Remove from this list once the
84-
# Makefile is corrected.
81+
# Ubuntu host and are therefore neither built nor run.
8582
declare -a SKIP_DIRS=(
86-
net
8783
)
8884
8985
is_in() {
@@ -129,14 +125,16 @@ jobs:
129125
continue
130126
fi
131127
132-
# Extract targets from Makefile (handles both TARGET := foo
133-
# and TARGETS = a b c).
128+
# Extract targets from Makefile. Handles:
129+
# TARGET := foo
130+
# TARGETS = a b c
131+
# TARGET_TEST := foo_test (second target in same Makefile)
134132
mapfile -t targets < <(
135-
grep -hE '^(TARGET|TARGETS)[[:space:]]*[:?]?=' "$d/Makefile" \
136-
| head -1 \
133+
grep -hE '^(TARGET|TARGETS|TARGET_TEST)[[:space:]]*[:?]?=' "$d/Makefile" \
137134
| sed -E 's/^[^=]*=[[:space:]]*//' \
138135
| tr -s ' \t' '\n' \
139-
| grep -v '^$'
136+
| grep -v '^$' \
137+
| sort -u
140138
)
141139
142140
for t in "${targets[@]}"; do

libretro-common/file/archive_file_zstd.c

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
*/
2222

2323
#include <stdlib.h>
24+
#include <stdint.h>
2425
#include <string.h>
2526

2627
#include <boolean.h>
@@ -136,6 +137,12 @@ static int zstd_parse_file_iterate_step(void *context,
136137
|| content_size == ZSTD_CONTENTSIZE_ERROR)
137138
return -1;
138139

140+
/* decompressed_size is a uint32_t and feeds a later malloc; reject
141+
* values that would truncate to a smaller size and mismatch the
142+
* actual ZSTD_decompress output length. */
143+
if (content_size > UINT32_MAX)
144+
return -1;
145+
139146
ctx->decompressed_size = (uint32_t)content_size;
140147

141148
strlcpy(userdata->current_file_path, ctx->inner_filename,
@@ -293,6 +300,17 @@ static int64_t zstd_file_read(
293300
return -1;
294301
}
295302

303+
/* Reject sizes that would overflow the "+1" NUL-byte allocation or
304+
* truncate when cast to size_t on 32-bit hosts. Without this guard
305+
* content_size = 0xFFFFFFFF on a 32-bit host wraps the +1 to zero,
306+
* malloc(0) may return a non-NULL pointer, and the following
307+
* ZSTD_decompress writes 4 GiB into it. */
308+
if (content_size >= SIZE_MAX)
309+
{
310+
free(compressed_data);
311+
return -1;
312+
}
313+
296314
decompressed = (uint8_t*)malloc((size_t)(content_size + 1));
297315
if (!decompressed)
298316
{
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
TARGET := archive_zstd_test
2+
3+
SOURCES := archive_zstd_test.c
4+
5+
OBJS := $(SOURCES:.c=.o)
6+
7+
CFLAGS += -Wall -pedantic -std=gnu99 -g -O0
8+
9+
all: $(TARGET)
10+
11+
%.o: %.c
12+
$(CC) -c -o $@ $< $(CFLAGS)
13+
14+
$(TARGET): $(OBJS)
15+
$(CC) -o $@ $^ $(LDFLAGS)
16+
17+
clean:
18+
rm -f $(TARGET) $(OBJS)
19+
20+
.PHONY: clean
Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
/* Copyright (C) 2010-2026 The RetroArch team
2+
*
3+
* ---------------------------------------------------------------------------------------
4+
* The following license statement only applies to this file (archive_zstd_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+
/* Contract test for archive_file_zstd content_size guards.
24+
*
25+
* Self-contained: does NOT link against libzstd or the archive
26+
* backend. The bugs being guarded against are arithmetic truncation
27+
* and addition overflow on an attacker-controlled 64-bit value
28+
* returned by ZSTD_getFrameContentSize(); the guards live in
29+
* archive_file_zstd.c and are plain integer comparisons.
30+
*
31+
* This test is explicitly a CONTRACT test, not a regression test.
32+
* It validates that the guard SPEC (as replicated in the oracle
33+
* functions below) behaves correctly on boundary inputs. It does
34+
* NOT call into archive_file_zstd.c, so it cannot detect if someone
35+
* later edits the real guards in a way that diverges from this spec.
36+
*
37+
* Why: a true regression test would need to link libzstd to exercise
38+
* ZSTD_getFrameContentSize on a crafted frame, which would introduce
39+
* an external dependency this samples tree does not otherwise carry.
40+
* The contract test is the next-best thing within that constraint.
41+
*
42+
* What the real patched code does (keep these in sync with the
43+
* oracle functions below):
44+
*
45+
* zstd_parse_file_iterate_step() [iterate path]:
46+
* if (content_size > UINT32_MAX)
47+
* return -1;
48+
* ctx->decompressed_size = (uint32_t)content_size;
49+
*
50+
* zstd_file_read() [decompress-in-place path]:
51+
* if (content_size >= SIZE_MAX)
52+
* return -1;
53+
* decompressed = malloc((size_t)(content_size + 1));
54+
*
55+
* If either real guard is ever edited, the corresponding oracle
56+
* below must be updated to match or this test loses its value.
57+
*/
58+
59+
#include <stdio.h>
60+
#include <stdlib.h>
61+
#include <stdint.h>
62+
#include <limits.h>
63+
64+
/* These match <zstd.h> but are reproduced here so we don't pull in
65+
* the zstd headers. Update if zstd ever redefines them. */
66+
#ifndef ZSTD_CONTENTSIZE_UNKNOWN
67+
#define ZSTD_CONTENTSIZE_UNKNOWN ((unsigned long long)0 - 1)
68+
#endif
69+
#ifndef ZSTD_CONTENTSIZE_ERROR
70+
#define ZSTD_CONTENTSIZE_ERROR ((unsigned long long)0 - 2)
71+
#endif
72+
73+
static int failures = 0;
74+
75+
/* --- oracle: the patched iterate-path guard --------------------- *
76+
* Must be kept in sync with archive_file_zstd.c:
77+
* zstd_parse_file_iterate_step.
78+
* Returns 0 if the value is accepted, -1 if rejected. */
79+
static int oracle_iterate_guard(unsigned long long content_size)
80+
{
81+
if ( content_size == ZSTD_CONTENTSIZE_UNKNOWN
82+
|| content_size == ZSTD_CONTENTSIZE_ERROR)
83+
return -1;
84+
if (content_size > UINT32_MAX)
85+
return -1;
86+
return 0;
87+
}
88+
89+
/* --- oracle: the patched read-path guard ------------------------ *
90+
* Must be kept in sync with archive_file_zstd.c: zstd_file_read. *
91+
* Returns 0 if the value is accepted, -1 if rejected. */
92+
static int oracle_read_guard(unsigned long long content_size)
93+
{
94+
if ( content_size == ZSTD_CONTENTSIZE_UNKNOWN
95+
|| content_size == ZSTD_CONTENTSIZE_ERROR)
96+
return -1;
97+
if (content_size >= (unsigned long long)SIZE_MAX)
98+
return -1;
99+
return 0;
100+
}
101+
102+
/* ================================================================ */
103+
104+
typedef struct {
105+
const char *label;
106+
unsigned long long value;
107+
int want_iterate; /* expected oracle_iterate result */
108+
int want_read; /* expected oracle_read result */
109+
} case_t;
110+
111+
int main(void)
112+
{
113+
size_t i;
114+
/* These cases exercise the boundaries the guards protect. Each
115+
* case lists the expected verdict for both the iterate path
116+
* (uint32_t destination) and the read path (size_t + 1). */
117+
case_t cases[] = {
118+
/* label value iter read */
119+
{ "zero", 0, 0, 0 },
120+
{ "typical small", 1024, 0, 0 },
121+
{ "100 MiB", 100ULL * 1024 * 1024, 0, 0 },
122+
{ "UINT32_MAX exactly", (unsigned long long)UINT32_MAX, 0, 0 },
123+
{ "UINT32_MAX + 1 (iterate trunc)",(unsigned long long)UINT32_MAX + 1, -1, 0 },
124+
{ "4 GiB (iterate trunc)", 4ULL * 1024 * 1024 * 1024, -1, 0 },
125+
{ "2^63 (iterate trunc)", 1ULL << 63, -1, 0 },
126+
{ "ZSTD_CONTENTSIZE_ERROR sentinel",ZSTD_CONTENTSIZE_ERROR, -1, -1 },
127+
{ "ZSTD_CONTENTSIZE_UNKNOWN", ZSTD_CONTENTSIZE_UNKNOWN, -1, -1 },
128+
/* SIZE_MAX case -- on 64-bit, SIZE_MAX == ULLONG_MAX - 1,
129+
* which equals ZSTD_CONTENTSIZE_ERROR, so the sentinel check
130+
* catches it. On 32-bit, SIZE_MAX is far smaller and the
131+
* >= SIZE_MAX branch catches it first. Either way: rejected
132+
* on the read path. On the iterate path it's also rejected
133+
* (too big for uint32_t). */
134+
{ "SIZE_MAX (read-path guard)", (unsigned long long)SIZE_MAX, -1, -1 },
135+
};
136+
137+
for (i = 0; i < sizeof(cases)/sizeof(cases[0]); i++)
138+
{
139+
int got_iter = oracle_iterate_guard(cases[i].value);
140+
int got_read = oracle_read_guard(cases[i].value);
141+
142+
if (got_iter != cases[i].want_iterate)
143+
{
144+
printf("[FAILED] iterate-guard(%s=%llu) got %d want %d\n",
145+
cases[i].label,
146+
(unsigned long long)cases[i].value,
147+
got_iter, cases[i].want_iterate);
148+
failures++;
149+
continue;
150+
}
151+
if (got_read != cases[i].want_read)
152+
{
153+
printf("[FAILED] read-guard(%s=%llu) got %d want %d\n",
154+
cases[i].label,
155+
(unsigned long long)cases[i].value,
156+
got_read, cases[i].want_read);
157+
failures++;
158+
continue;
159+
}
160+
printf("[SUCCESS] %-40s iter=%s read=%s\n",
161+
cases[i].label,
162+
got_iter == 0 ? "accept" : "reject",
163+
got_read == 0 ? "accept" : "reject");
164+
}
165+
166+
if (failures)
167+
{
168+
printf("\n%d test(s) failed\n", failures);
169+
return 1;
170+
}
171+
printf("\nAll zstd content_size regression tests passed.\n");
172+
return 0;
173+
}

libretro-common/samples/net/Makefile

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,18 @@ HTTP_TEST_C = \
3131
$(LIBRETRO_COMM_DIR)/net/net_compat.c \
3232
$(LIBRETRO_COMM_DIR)/net/net_socket.c \
3333
$(LIBRETRO_COMM_DIR)/compat/compat_strl.c \
34+
$(LIBRETRO_COMM_DIR)/compat/compat_strcasestr.c \
35+
$(LIBRETRO_COMM_DIR)/compat/compat_posix_string.c \
3436
$(LIBRETRO_COMM_DIR)/encodings/encoding_utf.c \
37+
$(LIBRETRO_COMM_DIR)/features/features_cpu.c \
38+
$(LIBRETRO_COMM_DIR)/file/file_path.c \
39+
$(LIBRETRO_COMM_DIR)/file/file_path_io.c \
40+
$(LIBRETRO_COMM_DIR)/lists/string_list.c \
41+
$(LIBRETRO_COMM_DIR)/streams/file_stream.c \
3542
$(LIBRETRO_COMM_DIR)/string/stdstring.c \
43+
$(LIBRETRO_COMM_DIR)/time/rtime.c \
44+
$(LIBRETRO_COMM_DIR)/vfs/vfs_implementation.c \
45+
$(LIBRETRO_COMM_DIR)/compat/fopen_utf8.c \
3646
net_http_test.c
3747

3848
HTTP_TEST_OBJS := $(HTTP_TEST_C:.c=.o)
@@ -44,14 +54,25 @@ HTTP_PARSE_TEST_C = \
4454
$(LIBRETRO_COMM_DIR)/net/net_socket.c \
4555
$(LIBRETRO_COMM_DIR)/compat/compat_strl.c \
4656
$(LIBRETRO_COMM_DIR)/compat/compat_strcasestr.c \
57+
$(LIBRETRO_COMM_DIR)/compat/compat_posix_string.c \
4758
$(LIBRETRO_COMM_DIR)/encodings/encoding_utf.c \
59+
$(LIBRETRO_COMM_DIR)/features/features_cpu.c \
60+
$(LIBRETRO_COMM_DIR)/file/file_path.c \
61+
$(LIBRETRO_COMM_DIR)/file/file_path_io.c \
62+
$(LIBRETRO_COMM_DIR)/lists/string_list.c \
63+
$(LIBRETRO_COMM_DIR)/streams/file_stream.c \
4864
$(LIBRETRO_COMM_DIR)/string/stdstring.c \
65+
$(LIBRETRO_COMM_DIR)/time/rtime.c \
66+
$(LIBRETRO_COMM_DIR)/vfs/vfs_implementation.c \
67+
$(LIBRETRO_COMM_DIR)/compat/fopen_utf8.c \
4968
net_http_parse_test.c
5069

5170
HTTP_PARSE_TEST_OBJS := $(HTTP_PARSE_TEST_C:.c=.o)
5271

5372
NET_IFINFO_C = \
5473
$(LIBRETRO_COMM_DIR)/net/net_ifinfo.c \
74+
$(LIBRETRO_COMM_DIR)/net/net_compat.c \
75+
$(LIBRETRO_COMM_DIR)/compat/compat_strl.c \
5576
net_ifinfo_test.c
5677

5778
ifeq ($(platform), win)

libretro-common/samples/streams/rzip/Makefile

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
TARGET := rzip
2+
TARGET_TEST := rzip_chunk_size_test
23

34
LIBRETRO_COMM_DIR := ../../..
45
LIBRETRO_DEPS_DIR := ../../../../deps
@@ -21,8 +22,7 @@ ifeq ($(UNAME), MSYS)
2122
TARGET := rzip.exe
2223
endif
2324

24-
SOURCES := \
25-
rzip.c \
25+
COMMON_SOURCES := \
2626
$(LIBRETRO_COMM_DIR)/compat/fopen_utf8.c \
2727
$(LIBRETRO_COMM_DIR)/compat/compat_strl.c \
2828
$(LIBRETRO_COMM_DIR)/compat/compat_strcasestr.c \
@@ -47,7 +47,7 @@ ifneq ($(wildcard $(LIBRETRO_DEPS_DIR)/*),)
4747
# If we are building from inside the RetroArch
4848
# directory (i.e. if an 'external' deps directory
4949
# is available), bake in zlib support
50-
SOURCES += \
50+
COMMON_SOURCES += \
5151
$(LIBRETRO_DEPS_DIR)/libz/adler32.c \
5252
$(LIBRETRO_DEPS_DIR)/libz/libz-crc32.c \
5353
$(LIBRETRO_DEPS_DIR)/libz/deflate.c \
@@ -68,7 +68,12 @@ else
6868
LDFLAGS += -lz
6969
endif
7070

71-
OBJS := $(SOURCES:.c=.o)
71+
SOURCES := rzip.c $(COMMON_SOURCES)
72+
SOURCES_TEST := rzip_chunk_size_test.c $(COMMON_SOURCES)
73+
74+
OBJS := $(SOURCES:.c=.o)
75+
OBJS_TEST := $(SOURCES_TEST:.c=.o)
76+
7277
INCLUDE_DIRS += -I$(LIBRETRO_COMM_DIR)/include
7378
CFLAGS += -DHAVE_ZLIB -Wall -pedantic -std=gnu99 $(INCLUDE_DIRS)
7479

@@ -87,15 +92,18 @@ else
8792
CFLAGS += -O2 -DNDEBUG
8893
endif
8994

90-
all: $(TARGET)
95+
all: $(TARGET) $(TARGET_TEST)
9196

9297
%.o: %.c
9398
$(CC) -c -o $@ $< $(CFLAGS)
9499

95100
$(TARGET): $(OBJS)
96101
$(CC) -o $@ $^ $(LDFLAGS)
97102

103+
$(TARGET_TEST): $(OBJS_TEST)
104+
$(CC) -o $@ $^ $(LDFLAGS)
105+
98106
clean:
99-
rm -f $(TARGET) $(OBJS)
107+
rm -f $(TARGET) $(TARGET_TEST) $(OBJS) $(OBJS_TEST)
100108

101109
.PHONY: clean

0 commit comments

Comments
 (0)