Skip to content

Commit fd69102

Browse files
committed
libretro-common/net: fix buffer overflow in string_parse_html_anchor + test
The parser underlying task_http's buildbot directory-listing scrape had two unbounded memcpy calls in the same function. Found while fixing an adjacent bit-rot warning in the regression test. net_http_parse: bound memcpy against caller buffer sizes string_parse_html_anchor takes link_size and name_size for the caller's output buffers but did not check either. Both memcpy calls copied end-start bytes straight in: memcpy(link, line, end - line); *(link + (end - line)) = '\0'; memcpy(name, start + 2, end - start - 2); *(name + (end - start - 2)) = '\0'; A long href or anchor text trivially overflows the caller's buffer. Reachable through tasks/task_http.c's HTML directory scraping path -- any server response with a long link URL or long title trips it. Reproduced under ASan: "stack-buffer-overflow" in memcpy at net_http_parse.c:65 when link[] is smaller than the URL. Fix: clamp the copy length to link_size-1 / name_size-1 before the memcpy and write the NUL at the clamped position. Truncates cleanly rather than overflowing. Callers that need full URLs must pass adequately sized buffers, which is already the contract (the size parameters exist for a reason). net_http_parse_test: fix bit-rot + turn into true regression test The sample triggered "implicit declaration of string_parse_html_anchor" in every CI build -- it was including compat/strcasestr.h instead of the correct net/net_http_parse.h. Rewrote the sample as four subtests: 1. happy path (verifies output) 2. no anchor in input (verifies error return) 3. undersized link buffer (stack canary + NUL-term check) 4. undersized name buffer (same) Subtests 3 and 4 are true discriminators for the parser fix above. On unpatched parser: -O0 under ASan: stack-buffer-overflow in memcpy -O2 plain run: 2 of 4 subtests report "output not NUL- terminated" (the unclamped memcpy + NUL at offset N writes the terminator past the end of the buffer, leaving the buffer contents without any NUL inside it) Post-patch: all 4 subtests pass, ASan clean. Also cleaned up: - int main(int argc, char *argv[]) -> int main(void) (the original sample used neither) CI: http_parse_test added to the libretro-common samples RUN_TARGETS allowlist. Full local dry-run under the GHA shell contract: Built: 14 Ran: 14 Failed: 0 VC6 compatibility: net_http_parse.c is compiled on all platforms. The fix uses only size_t and standard comparisons; no new headers. Sample test uses stdint.h (already available via the VC6 compat shim) and basic <string.h>.
1 parent 39fcefe commit fd69102

3 files changed

Lines changed: 184 additions & 10 deletions

File tree

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ jobs:
5454
net_ifinfo
5555
vfs_read_overflow_test
5656
cdrom_cuesheet_overflow_test
57+
http_parse_test
5758
)
5859
5960
# Per-binary run command (overrides ./<binary> if present).

libretro-common/net/net_http_parse.c

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,27 +58,39 @@ int string_parse_html_anchor(const char *line, char *link, char *name,
5858
if (!*link)
5959
{
6060
const char *end = strstr(line, "\"");
61+
size_t _len;
6162

6263
if (!end)
6364
return 1;
6465

65-
memcpy(link, line, end - line);
66+
/* Bound the href length against the caller's buffer.
67+
* Pre-patch the memcpy was unbounded and a long href
68+
* would overflow link[]. Keep room for the NUL. */
69+
_len = (size_t)(end - line);
70+
if (_len >= link_size)
71+
_len = (link_size > 0) ? link_size - 1 : 0;
6672

67-
*(link + (end - line)) = '\0';
73+
memcpy(link, line, _len);
74+
link[_len] = '\0';
6875
line += end - line;
6976
}
7077

7178
if (!*name)
7279
{
7380
const char *start = strstr(line, "\">");
7481
const char *end = start ? strstr(start, "</a>") : NULL;
82+
size_t _len;
7583

7684
if (!start || !end)
7785
return 1;
7886

79-
memcpy(name, start + 2, end - start - 2);
87+
/* Same bounding for the anchor text. */
88+
_len = (size_t)(end - start - 2);
89+
if (_len >= name_size)
90+
_len = (name_size > 0) ? name_size - 1 : 0;
8091

81-
*(name + (end - start - 2)) = '\0';
92+
memcpy(name, start + 2, _len);
93+
name[_len] = '\0';
8294
}
8395
}
8496

libretro-common/samples/net/net_http_parse_test.c

Lines changed: 167 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,19 +21,180 @@
2121
*/
2222

2323
#include <stdio.h>
24-
#include <compat/strcasestr.h>
24+
#include <string.h>
25+
#include <net/net_http_parse.h>
2526

26-
int main(int argc, char *argv[])
27+
static int failures = 0;
28+
29+
static void test_happy_path(void)
2730
{
2831
char link[1024];
2932
char name[1024];
30-
const char *line = "<a href=\"http://www.test.com/somefile.zip\">Test</a>\n";
33+
const char *line = "<a href=\"http://www.test.com/somefile.zip\">Test</a>\n";
34+
int rc;
35+
36+
link[0] = name[0] = '\0';
37+
rc = string_parse_html_anchor(line, link, name, sizeof(link), sizeof(name));
38+
39+
if (rc != 0)
40+
{
41+
printf("[ERROR] happy path: rc=%d, want 0\n", rc);
42+
failures++;
43+
return;
44+
}
45+
if (strcmp(link, "http://www.test.com/somefile.zip") != 0)
46+
{
47+
printf("[ERROR] happy path: link='%s'\n", link);
48+
failures++;
49+
return;
50+
}
51+
if (strcmp(name, "Test") != 0)
52+
{
53+
printf("[ERROR] happy path: name='%s'\n", name);
54+
failures++;
55+
return;
56+
}
57+
printf("[SUCCESS] happy path: link='%s' name='%s'\n", link, name);
58+
}
59+
60+
static void test_no_anchor(void)
61+
{
62+
char link[64];
63+
char name[64];
64+
int rc;
65+
66+
rc = string_parse_html_anchor("no anchor here", link, name,
67+
sizeof(link), sizeof(name));
68+
69+
if (rc == 0)
70+
{
71+
printf("[ERROR] no-anchor: rc=0, want 1\n");
72+
failures++;
73+
return;
74+
}
75+
printf("[SUCCESS] no-anchor returned %d\n", rc);
76+
}
77+
78+
static void test_undersized_link_buffer(void)
79+
{
80+
/* The href is 41 characters. Supply a 10-byte link buffer.
81+
* Pre-patch: memcpy copies 41 bytes into link[10] -- stack
82+
* smashing / canary hit / ASan heap-buffer-overflow.
83+
* Post-patch: len clamps to 9, link[9] = NUL, safe truncation. */
84+
char link[10]; /* deliberately too small */
85+
char name[64];
86+
char canary[16]; /* stack canary to detect over-write */
87+
const char *line = "<a href=\"http://www.test.com/somefile.zip\">Test</a>";
88+
int rc;
3189

90+
/* Fill canary with a known pattern. A pre-patch run smashes
91+
* link[] and onto the adjacent stack slot; the canary gets
92+
* corrupted. Post-patch leaves it alone. */
93+
memset(canary, 0xAA, sizeof(canary));
3294
link[0] = name[0] = '\0';
3395

34-
string_parse_html_anchor(line, link, name, sizeof(link), sizeof(name));
96+
rc = string_parse_html_anchor(line, link, name,
97+
sizeof(link), sizeof(name));
98+
(void)rc;
3599

36-
printf("link: %s\nname: %s\n", link, name);
100+
/* Check the canary is intact. (Result of rc is don't-care:
101+
* post-patch should still return 0 with a truncated link.) */
102+
{
103+
size_t i;
104+
for (i = 0; i < sizeof(canary); ++i)
105+
{
106+
if ((unsigned char)canary[i] != 0xAA)
107+
{
108+
printf("[ERROR] undersized-link: canary corrupted at offset %zu\n", i);
109+
failures++;
110+
return;
111+
}
112+
}
113+
}
114+
115+
/* The link buffer must be NUL-terminated somewhere in [0, 9]. */
116+
{
117+
int found_nul = 0;
118+
size_t i;
119+
for (i = 0; i < sizeof(link); ++i)
120+
{
121+
if (link[i] == '\0')
122+
{
123+
found_nul = 1;
124+
break;
125+
}
126+
}
127+
if (!found_nul)
128+
{
129+
printf("[ERROR] undersized-link: output not NUL-terminated\n");
130+
failures++;
131+
return;
132+
}
133+
}
134+
135+
printf("[SUCCESS] undersized-link: truncated link='%s' (buffer safe)\n", link);
136+
}
137+
138+
static void test_undersized_name_buffer(void)
139+
{
140+
char link[256];
141+
char name[3]; /* deliberately too small for "Test" */
142+
char canary[16];
143+
const char *line = "<a href=\"http://x.com/\">Test</a>";
144+
int rc;
145+
146+
memset(canary, 0xBB, sizeof(canary));
147+
link[0] = name[0] = '\0';
148+
149+
rc = string_parse_html_anchor(line, link, name,
150+
sizeof(link), sizeof(name));
151+
(void)rc;
152+
153+
{
154+
size_t i;
155+
for (i = 0; i < sizeof(canary); ++i)
156+
{
157+
if ((unsigned char)canary[i] != 0xBB)
158+
{
159+
printf("[ERROR] undersized-name: canary corrupted at offset %zu\n", i);
160+
failures++;
161+
return;
162+
}
163+
}
164+
}
165+
{
166+
int found_nul = 0;
167+
size_t i;
168+
for (i = 0; i < sizeof(name); ++i)
169+
{
170+
if (name[i] == '\0')
171+
{
172+
found_nul = 1;
173+
break;
174+
}
175+
}
176+
if (!found_nul)
177+
{
178+
printf("[ERROR] undersized-name: output not NUL-terminated\n");
179+
failures++;
180+
return;
181+
}
182+
}
183+
printf("[SUCCESS] undersized-name: truncated name='%s' (buffer safe)\n", name);
184+
}
185+
186+
int main(void)
187+
{
188+
test_happy_path();
189+
test_no_anchor();
190+
test_undersized_link_buffer();
191+
test_undersized_name_buffer();
37192

38-
return 1;
193+
if (failures)
194+
{
195+
printf("\n%d net_http_parse test(s) failed\n", failures);
196+
return 1;
197+
}
198+
printf("\nAll net_http_parse regression tests passed.\n");
199+
return 0;
39200
}

0 commit comments

Comments
 (0)