mirror of
https://github.com/libretro/RetroArch.git
synced 2026-11-08 08:10:30 -10:00
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>.
This commit is contained in:
1 parent
39fcefebaf
commit
fd69102744
3 files changed
+184
-10
No files matched your search
@@ -54,6 +54,7 @@ jobs:
|
||||
net_ifinfo
|
||||
vfs_read_overflow_test
|
||||
cdrom_cuesheet_overflow_test
|
||||
http_parse_test
|
||||
)
|
||||
|
||||
# Per-binary run command (overrides ./<binary> if present).
|
||||
|
||||
@@ -58,13 +58,20 @@ int string_parse_html_anchor(const char *line, char *link, char *name,
|
||||
if (!*link)
|
||||
{
|
||||
const char *end = strstr(line, "\"");
|
||||
size_t _len;
|
||||
|
||||
if (!end)
|
||||
return 1;
|
||||
|
||||
memcpy(link, line, end - line);
|
||||
/* Bound the href length against the caller's buffer.
|
||||
* Pre-patch the memcpy was unbounded and a long href
|
||||
* would overflow link[]. Keep room for the NUL. */
|
||||
_len = (size_t)(end - line);
|
||||
if (_len >= link_size)
|
||||
_len = (link_size > 0) ? link_size - 1 : 0;
|
||||
|
||||
*(link + (end - line)) = '\0';
|
||||
memcpy(link, line, _len);
|
||||
link[_len] = '\0';
|
||||
line += end - line;
|
||||
}
|
||||
|
||||
@@ -72,13 +79,18 @@ int string_parse_html_anchor(const char *line, char *link, char *name,
|
||||
{
|
||||
const char *start = strstr(line, "\">");
|
||||
const char *end = start ? strstr(start, "</a>") : NULL;
|
||||
size_t _len;
|
||||
|
||||
if (!start || !end)
|
||||
return 1;
|
||||
|
||||
memcpy(name, start + 2, end - start - 2);
|
||||
/* Same bounding for the anchor text. */
|
||||
_len = (size_t)(end - start - 2);
|
||||
if (_len >= name_size)
|
||||
_len = (name_size > 0) ? name_size - 1 : 0;
|
||||
|
||||
*(name + (end - start - 2)) = '\0';
|
||||
memcpy(name, start + 2, _len);
|
||||
name[_len] = '\0';
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -21,19 +21,180 @@
|
||||
*/
|
||||
|
||||
#include <stdio.h>
|
||||
#include <compat/strcasestr.h>
|
||||
#include <string.h>
|
||||
#include <net/net_http_parse.h>
|
||||
|
||||
int main(int argc, char *argv[])
|
||||
static int failures = 0;
|
||||
|
||||
static void test_happy_path(void)
|
||||
{
|
||||
char link[1024];
|
||||
char name[1024];
|
||||
const char *line = "<a href=\"http://www.test.com/somefile.zip\">Test</a>\n";
|
||||
const char *line = "<a href=\"http://www.test.com/somefile.zip\">Test</a>\n";
|
||||
int rc;
|
||||
|
||||
link[0] = name[0] = '\0';
|
||||
rc = string_parse_html_anchor(line, link, name, sizeof(link), sizeof(name));
|
||||
|
||||
string_parse_html_anchor(line, link, name, sizeof(link), sizeof(name));
|
||||
if (rc != 0)
|
||||
{
|
||||
printf("[ERROR] happy path: rc=%d, want 0\n", rc);
|
||||
failures++;
|
||||
return;
|
||||
}
|
||||
if (strcmp(link, "http://www.test.com/somefile.zip") != 0)
|
||||
{
|
||||
printf("[ERROR] happy path: link='%s'\n", link);
|
||||
failures++;
|
||||
return;
|
||||
}
|
||||
if (strcmp(name, "Test") != 0)
|
||||
{
|
||||
printf("[ERROR] happy path: name='%s'\n", name);
|
||||
failures++;
|
||||
return;
|
||||
}
|
||||
printf("[SUCCESS] happy path: link='%s' name='%s'\n", link, name);
|
||||
}
|
||||
|
||||
printf("link: %s\nname: %s\n", link, name);
|
||||
static void test_no_anchor(void)
|
||||
{
|
||||
char link[64];
|
||||
char name[64];
|
||||
int rc;
|
||||
|
||||
return 1;
|
||||
rc = string_parse_html_anchor("no anchor here", link, name,
|
||||
sizeof(link), sizeof(name));
|
||||
|
||||
if (rc == 0)
|
||||
{
|
||||
printf("[ERROR] no-anchor: rc=0, want 1\n");
|
||||
failures++;
|
||||
return;
|
||||
}
|
||||
printf("[SUCCESS] no-anchor returned %d\n", rc);
|
||||
}
|
||||
|
||||
static void test_undersized_link_buffer(void)
|
||||
{
|
||||
/* The href is 41 characters. Supply a 10-byte link buffer.
|
||||
* Pre-patch: memcpy copies 41 bytes into link[10] -- stack
|
||||
* smashing / canary hit / ASan heap-buffer-overflow.
|
||||
* Post-patch: len clamps to 9, link[9] = NUL, safe truncation. */
|
||||
char link[10]; /* deliberately too small */
|
||||
char name[64];
|
||||
char canary[16]; /* stack canary to detect over-write */
|
||||
const char *line = "<a href=\"http://www.test.com/somefile.zip\">Test</a>";
|
||||
int rc;
|
||||
|
||||
/* Fill canary with a known pattern. A pre-patch run smashes
|
||||
* link[] and onto the adjacent stack slot; the canary gets
|
||||
* corrupted. Post-patch leaves it alone. */
|
||||
memset(canary, 0xAA, sizeof(canary));
|
||||
link[0] = name[0] = '\0';
|
||||
|
||||
rc = string_parse_html_anchor(line, link, name,
|
||||
sizeof(link), sizeof(name));
|
||||
(void)rc;
|
||||
|
||||
/* Check the canary is intact. (Result of rc is don't-care:
|
||||
* post-patch should still return 0 with a truncated link.) */
|
||||
{
|
||||
size_t i;
|
||||
for (i = 0; i < sizeof(canary); ++i)
|
||||
{
|
||||
if ((unsigned char)canary[i] != 0xAA)
|
||||
{
|
||||
printf("[ERROR] undersized-link: canary corrupted at offset %zu\n", i);
|
||||
failures++;
|
||||
return;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/* The link buffer must be NUL-terminated somewhere in [0, 9]. */
|
||||
{
|
||||
int found_nul = 0;
|
||||
size_t i;
|
||||
for (i = 0; i < sizeof(link); ++i)
|
||||
{
|
||||
if (link[i] == '\0')
|
||||
{
|
||||
found_nul = 1;
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (!found_nul)
|
||||
{
|
||||
printf("[ERROR] undersized-link: output not NUL-terminated\n");
|
||||
failures++;
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
printf("[SUCCESS] undersized-link: truncated link='%s' (buffer safe)\n", link);
|
||||
}
|
||||
|
||||
static void test_undersized_name_buffer(void)
|
||||
{
|
||||
char link[256];
|
||||
char name[3]; /* deliberately too small for "Test" */
|
||||
char canary[16];
|
||||
const char *line = "<a href=\"http://x.com/\">Test</a>";
|
||||
int rc;
|
||||
|
||||
memset(canary, 0xBB, sizeof(canary));
|
||||
link[0] = name[0] = '\0';
|
||||
|
||||
rc = string_parse_html_anchor(line, link, name,
|
||||
sizeof(link), sizeof(name));
|
||||
(void)rc;
|
||||
|
||||
{
|
||||
size_t i;
|
||||
for (i = 0; i < sizeof(canary); ++i)
|
||||
{
|
||||
if ((unsigned char)canary[i] != 0xBB)
|
||||
{
|
||||
printf("[ERROR] undersized-name: canary corrupted at offset %zu\n", i);
|
||||
failures++;
|
||||
return;
|
||||
}
|
||||
}
|
||||
}
|
||||
{
|
||||
int found_nul = 0;
|
||||
size_t i;
|
||||
for (i = 0; i < sizeof(name); ++i)
|
||||
{
|
||||
if (name[i] == '\0')
|
||||
{
|
||||
found_nul = 1;
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (!found_nul)
|
||||
{
|
||||
printf("[ERROR] undersized-name: output not NUL-terminated\n");
|
||||
failures++;
|
||||
return;
|
||||
}
|
||||
}
|
||||
printf("[SUCCESS] undersized-name: truncated name='%s' (buffer safe)\n", name);
|
||||
}
|
||||
|
||||
int main(void)
|
||||
{
|
||||
test_happy_path();
|
||||
test_no_anchor();
|
||||
test_undersized_link_buffer();
|
||||
test_undersized_name_buffer();
|
||||
|
||||
if (failures)
|
||||
{
|
||||
printf("\n%d net_http_parse test(s) failed\n", failures);
|
||||
return 1;
|
||||
}
|
||||
printf("\nAll net_http_parse regression tests passed.\n");
|
||||
return 0;
|
||||
}
|
||||
Reference in new issue
Block a user