mirror of
https://github.com/libretro/RetroArch.git
synced 2026-11-09 03:29:43 -10:00
Fix remaining overlapping-buffer copies found in tree-wide audit
Follow-up to bd79aee (in-place word_wrap OSK crash): audited every
strlcpy/strlcat/strcpy/strncpy/memcpy/sprintf call in the tree for
overlapping source/destination, both directly and through helper
functions that copy from one parameter to another. Two defects found:
1) gfx_widget_leaderboard_display.c: removing a tracker/challenge
entry shifted the remainder of the array down with
memcpy(&info[i], &info[i + 1], (count - i) * sizeof(info[i]));
which copies between overlapping regions whenever more than one
element moves (dst == src - stride). This is undefined behaviour
and is reported by ASan as memcpy-param-overlap; converted both
sites to memmove(). All other element-shift loops in the tree
(netplay chat history, cheat list insert/delete) move a single
element per iteration in the correct direction and are fine.
2) menu_cbs_ok.c (HAVE_LAKKA): MENU_ENUM_LABEL_CB_LAKKA_LIST called
fill_pathname_join_special(url_path, url_path, ...);
violating the documented contract ("@dir and @s CANNOT be
identical" - the helper strlcpy()s @dir into @s unguarded), the
same fortified-strlcpy abort class as the OSK crash. Switched to
fill_pathname_join(), which explicitly supports @s == @dir and is
otherwise identical; verified byte-identical output and return
values for the aliased call across representative dir/path
combinations.
Audit notes for everything else flagged and cleared:
- fill_pathname_join()/_join_delim()/_parent_dir() carry alias guards;
their in-place callers (runloop save dirs, cheevos badge path,
platform_gx, task_database, parent-dir navigation) are safe.
- LZ-style self-copies in r7z_lzma, encoding_vcdiff, encoding_rlz4 and
tasks/task_patch (BPS TARGET_COPY) are deliberate overlapping runs
with explicit disjoint/periodic-chunk guards.
- Decoder-internal same-buffer copies (ropus SILK resampler and LPC
rewind, rmp3 L12 scf spread, raac SBR persistence, rvp8 reference
rotation, rhmap slot compaction, ntsc_crt/sdl_rs90 row duplication,
ctr_svchax cache flush) are disjoint by construction or mirror
upstream reference implementations.
- No direct strlcpy/strlcat/sprintf calls with aliased arguments
remain anywhere in the tree.
Sanitizer sweep (ASan+UBSan+LSan):
- old leaderboard shift pattern reproduces
"AddressSanitizer: memcpy-param-overlap"; memmove version verified
element-exact against expected post-removal contents
- fill_pathname_join(buf, buf, path) output verified byte-identical
(string and return value) to fill_pathname_join_special() with
distinct buffers over dir/path edge cases including trailing slash
and empty components
- gfx_widget_leaderboard_display.c and menu_cbs_ok.c compile clean;
HAVE_LAKKA branch additionally syntax-checked with gcc
TSan: not applicable (no concurrency change).
This commit is contained in:
1 parent
bd79aee4a7
commit
e942ee2a70
2 files changed
+10
-3
No files matched your search
@@ -472,7 +472,9 @@ void gfx_widgets_set_leaderboard_display(unsigned id, const char* value)
|
||||
--state->tracker_count;
|
||||
if (i < state->tracker_count)
|
||||
{
|
||||
memcpy(&state->tracker_info[i], &state->tracker_info[i + 1],
|
||||
/* Multi-element downward shift - regions overlap,
|
||||
* so memcpy is undefined here */
|
||||
memmove(&state->tracker_info[i], &state->tracker_info[i + 1],
|
||||
(state->tracker_count - i) * sizeof(state->tracker_info[i]));
|
||||
}
|
||||
}
|
||||
@@ -573,7 +575,9 @@ void gfx_widgets_set_challenge_display(unsigned id, const char* badge)
|
||||
--state->challenge_count;
|
||||
if (i < state->challenge_count)
|
||||
{
|
||||
memcpy(&state->challenge_info[i], &state->challenge_info[i + 1],
|
||||
/* Multi-element downward shift - regions overlap,
|
||||
* so memcpy is undefined here */
|
||||
memmove(&state->challenge_info[i], &state->challenge_info[i + 1],
|
||||
(state->challenge_count - i) * sizeof(state->challenge_info[i]));
|
||||
}
|
||||
|
||||
|
||||
@@ -5440,7 +5440,10 @@ static int generic_action_ok_network(const char *path,
|
||||
fill_pathname_join_special(url_path,
|
||||
FILE_PATH_LAKKA_URL,
|
||||
LAKKA_PROJECT, sizeof(url_path));
|
||||
fill_pathname_join_special(url_path, url_path,
|
||||
/* fill_pathname_join_special() requires @s != @dir
|
||||
* (unguarded strlcpy with overlapping buffers);
|
||||
* fill_pathname_join() handles the aliased case */
|
||||
fill_pathname_join(url_path, url_path,
|
||||
FILE_PATH_INDEX_URL,
|
||||
sizeof(url_path));
|
||||
url_label = msg_hash_to_str(enum_idx);
|
||||
|
||||
Reference in new issue
Block a user