mirror of
https://github.com/libretro/RetroArch.git
synced 2026-11-08 08:10:30 -10:00
strlcpy chain conversions to strlcpy_append (continued)
Apply strlcpy_append (commit78c52ab) to three further sites identified in the wider strlcpy-misuse sweep, all with the same shape that drove the prior commitsc41e955/78c52ab/e446242: _len = strlcpy(buf, src1, sizeof(buf)); _len += strlcpy(buf + _len, src2, sizeof(buf) - _len); ... (more steps) ... That chain is unsafe because strlcpy returns strlen(source) regardless of truncation; on overflow _len overshoots sizeof(buf) and the next sizeof - _len underflows size_t, producing an unbounded copy that writes past the buffer. Sites: gfx/drivers/gl2.c:4549 - GL device-info string assembly. gl->device_str is 128 bytes; the chain appends `vendor`, ' ', and `renderer` (all from glGetString). Some drivers report long vendor/renderer combinations. Not attacker- reachable in the usual threat model but the pattern is still wrong, and the `device_str[_len]=' '; device_str [++_len]='\0'` separator chain in the middle was the short-chain shape from commite446242. menu/menu_displaylist.c:2767 - create_string_list_rdb_entry _string. Builds out_lbl (NAME_MAX_LENGTH bytes) from label + "|" + actual_string + "|" + path, plus tmp (128 bytes) from desc + ": " + actual_string. The strings come from RetroArch database (.rdb) entries; a malicious .rdb could supply long values to drive the chain underflow. Modest threat model -- users typically use the RetroArch-shipped databases, not custom ones -- but the pattern is the same one we've fixed elsewhere. command.c:1119 - GET_STATUS network reply. Builds the reply for the network command interface from the paused/ playing state, system_id or library_name, content basename, and CRC. reply is 8192 bytes which fits any realistic content, but ROM basenames can legally be any length and the chain has 8 steps; a long enough basename drives the underflow. Replace each chain with sequences of strlcpy_append calls on a single rolling cursor. Same correctness story ase446242: helper handles the bound check at every step, truncation is silent and contained, no behaviour change for the buffer-fits-comfortably case. No new tests -- the strlcpy_append regression test from78c52abcovers the helper's contract; these are additional users. Triaged but not fixed in this commit (safe by construction, flagged for the record): network/cloud_sync/webdav.c:401 - 16-call chain, but the function pre-calculates the exact required length via strlen() over the same inputs and mallocs that exact size before the chain runs. Single-threaded. retroarch.c:8769 - 20-call chain for SIMD feature names into a 128-byte buffer. Total max ~120 bytes; tight but no real CPU has all SIMD bits set so practical max is well under cap. gfx/video_shader_parse.c:1252, configuration.c:6961/7167, frontend/drivers/platform_emscripten.c:620 - classifier false positives (each _len is per-buffer reused, not accumulated cross-buffer).
This commit is contained in:
1 parent
e4462428e4
commit
25ade82150
4 files changed
+32
-28
No files matched your search
@@ -1116,37 +1116,37 @@ bool command_get_status(command_t *cmd, const char* arg)
|
||||
|
||||
core_info_get_current_core(&core_info);
|
||||
|
||||
_len = strlcpy(reply, "GET_STATUS ", sizeof(reply));
|
||||
_len = 0;
|
||||
strlcpy_append(reply, sizeof(reply), &_len, "GET_STATUS ");
|
||||
|
||||
if (runloop_st->flags & RUNLOOP_FLAG_PAUSED)
|
||||
_len += strlcpy(reply + _len, "PAUSED", sizeof(reply) - _len);
|
||||
strlcpy_append(reply, sizeof(reply), &_len, "PAUSED");
|
||||
else
|
||||
_len += strlcpy(reply + _len, "PLAYING", sizeof(reply) - _len);
|
||||
strlcpy_append(reply, sizeof(reply), &_len, "PLAYING");
|
||||
|
||||
_len += strlcpy(reply + _len, " ", sizeof(reply) - _len);
|
||||
strlcpy_append(reply, sizeof(reply), &_len, " ");
|
||||
|
||||
if (core_info && core_info->system_id)
|
||||
_len += strlcpy(reply + _len, core_info->system_id,
|
||||
sizeof(reply) - _len);
|
||||
strlcpy_append(reply, sizeof(reply), &_len, core_info->system_id);
|
||||
else if (runloop_st->system.info.library_name)
|
||||
_len += strlcpy(reply + _len, runloop_st->system.info.library_name,
|
||||
sizeof(reply) - _len);
|
||||
strlcpy_append(reply, sizeof(reply), &_len,
|
||||
runloop_st->system.info.library_name);
|
||||
else
|
||||
_len += strlcpy(reply + _len, "UNKNOWN", sizeof(reply) - _len);
|
||||
strlcpy_append(reply, sizeof(reply), &_len, "UNKNOWN");
|
||||
|
||||
_len += strlcpy(reply + _len, ",", sizeof(reply) - _len);
|
||||
strlcpy_append(reply, sizeof(reply), &_len, ",");
|
||||
|
||||
basename_path = path_get(RARCH_PATH_BASENAME);
|
||||
if (basename_path)
|
||||
{
|
||||
const char *basename = path_basename(basename_path);
|
||||
if (basename)
|
||||
_len += strlcpy(reply + _len, basename, sizeof(reply) - _len);
|
||||
strlcpy_append(reply, sizeof(reply), &_len, basename);
|
||||
else
|
||||
_len += strlcpy(reply + _len, "UNKNOWN", sizeof(reply) - _len);
|
||||
strlcpy_append(reply, sizeof(reply), &_len, "UNKNOWN");
|
||||
}
|
||||
else
|
||||
_len += strlcpy(reply + _len, "UNKNOWN", sizeof(reply) - _len);
|
||||
strlcpy_append(reply, sizeof(reply), &_len, "UNKNOWN");
|
||||
|
||||
_len += snprintf(reply + _len, sizeof(reply) - _len,
|
||||
",crc32=%lx\n", (unsigned long)content_get_crc());
|
||||
|
||||
+4
-5
@@ -4548,16 +4548,15 @@ static void *gl2_init(const video_info_t *video,
|
||||
|
||||
if (vendor && *vendor)
|
||||
{
|
||||
_len = strlcpy(gl->device_str, vendor, sizeof(gl->device_str));
|
||||
gl->device_str[ _len] = ' ';
|
||||
gl->device_str[++_len] = '\0';
|
||||
strlcpy_append(gl->device_str, sizeof(gl->device_str), &_len, vendor);
|
||||
strlcpy_append(gl->device_str, sizeof(gl->device_str), &_len, " ");
|
||||
}
|
||||
|
||||
if (renderer && *renderer)
|
||||
strlcpy(gl->device_str + _len, renderer, sizeof(gl->device_str) - _len);
|
||||
strlcpy_append(gl->device_str, sizeof(gl->device_str), &_len, renderer);
|
||||
|
||||
if (version && *version)
|
||||
video_driver_set_gpu_api_version_string(version);
|
||||
video_driver_set_gpu_api_version_string(version);
|
||||
}
|
||||
|
||||
#ifdef _WIN32
|
||||
|
||||
+5
-2
@@ -3189,10 +3189,13 @@ size_t video_driver_get_window_title(char *s, size_t len)
|
||||
video_driver_state_t *video_st = &video_driver_st;
|
||||
if (s && (video_st->flags & VIDEO_FLAG_WINDOW_TITLE_UPDATE))
|
||||
{
|
||||
strlcpy(s, video_st->window_title, len);
|
||||
size_t n = strlcpy(s, video_st->window_title, len);
|
||||
video_st->flags &= ~VIDEO_FLAG_WINDOW_TITLE_UPDATE;
|
||||
if (n >= len)
|
||||
return len ? len - 1 : 0;
|
||||
return n;
|
||||
}
|
||||
return video_st->window_title_len;
|
||||
return 0;
|
||||
}
|
||||
|
||||
void video_driver_update_title(void *data)
|
||||
|
||||
+10
-8
@@ -2764,14 +2764,16 @@ static int create_string_list_rdb_entry_string(
|
||||
{
|
||||
char tmp[128];
|
||||
char out_lbl[NAME_MAX_LENGTH];
|
||||
size_t _len = strlcpy(out_lbl, label, sizeof(out_lbl));
|
||||
_len += strlcpy(out_lbl + _len, "|", sizeof(out_lbl) - _len);
|
||||
_len += strlcpy(out_lbl + _len, actual_string, sizeof(out_lbl) - _len);
|
||||
_len += strlcpy(out_lbl + _len, "|", sizeof(out_lbl) - _len);
|
||||
strlcpy(out_lbl + _len, path, sizeof(out_lbl) - _len);
|
||||
_len = strlcpy(tmp, desc, sizeof(tmp));
|
||||
_len += strlcpy(tmp + _len, ": ", sizeof(tmp) - _len);
|
||||
strlcpy(tmp + _len, actual_string, sizeof(tmp) - _len);
|
||||
size_t _len = 0;
|
||||
strlcpy_append(out_lbl, sizeof(out_lbl), &_len, label);
|
||||
strlcpy_append(out_lbl, sizeof(out_lbl), &_len, "|");
|
||||
strlcpy_append(out_lbl, sizeof(out_lbl), &_len, actual_string);
|
||||
strlcpy_append(out_lbl, sizeof(out_lbl), &_len, "|");
|
||||
strlcpy_append(out_lbl, sizeof(out_lbl), &_len, path);
|
||||
_len = 0;
|
||||
strlcpy_append(tmp, sizeof(tmp), &_len, desc);
|
||||
strlcpy_append(tmp, sizeof(tmp), &_len, ": ");
|
||||
strlcpy_append(tmp, sizeof(tmp), &_len, actual_string);
|
||||
menu_entries_append(list, tmp, out_lbl,
|
||||
enum_idx,
|
||||
0, 0, 0, NULL);
|
||||
|
||||
Reference in new issue
Block a user