Skip to content

Commit 25ade82

Browse files
committed
strlcpy chain conversions to strlcpy_append (continued)
Apply strlcpy_append (commit 78c52ab) to three further sites identified in the wider strlcpy-misuse sweep, all with the same shape that drove the prior commits c41e955 / 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 commit e446242. 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 as e446242: 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 from 78c52ab covers 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).
1 parent e446242 commit 25ade82

4 files changed

Lines changed: 32 additions & 28 deletions

File tree

command.c

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1116,37 +1116,37 @@ bool command_get_status(command_t *cmd, const char* arg)
11161116

11171117
core_info_get_current_core(&core_info);
11181118

1119-
_len = strlcpy(reply, "GET_STATUS ", sizeof(reply));
1119+
_len = 0;
1120+
strlcpy_append(reply, sizeof(reply), &_len, "GET_STATUS ");
11201121

11211122
if (runloop_st->flags & RUNLOOP_FLAG_PAUSED)
1122-
_len += strlcpy(reply + _len, "PAUSED", sizeof(reply) - _len);
1123+
strlcpy_append(reply, sizeof(reply), &_len, "PAUSED");
11231124
else
1124-
_len += strlcpy(reply + _len, "PLAYING", sizeof(reply) - _len);
1125+
strlcpy_append(reply, sizeof(reply), &_len, "PLAYING");
11251126

1126-
_len += strlcpy(reply + _len, " ", sizeof(reply) - _len);
1127+
strlcpy_append(reply, sizeof(reply), &_len, " ");
11271128

11281129
if (core_info && core_info->system_id)
1129-
_len += strlcpy(reply + _len, core_info->system_id,
1130-
sizeof(reply) - _len);
1130+
strlcpy_append(reply, sizeof(reply), &_len, core_info->system_id);
11311131
else if (runloop_st->system.info.library_name)
1132-
_len += strlcpy(reply + _len, runloop_st->system.info.library_name,
1133-
sizeof(reply) - _len);
1132+
strlcpy_append(reply, sizeof(reply), &_len,
1133+
runloop_st->system.info.library_name);
11341134
else
1135-
_len += strlcpy(reply + _len, "UNKNOWN", sizeof(reply) - _len);
1135+
strlcpy_append(reply, sizeof(reply), &_len, "UNKNOWN");
11361136

1137-
_len += strlcpy(reply + _len, ",", sizeof(reply) - _len);
1137+
strlcpy_append(reply, sizeof(reply), &_len, ",");
11381138

11391139
basename_path = path_get(RARCH_PATH_BASENAME);
11401140
if (basename_path)
11411141
{
11421142
const char *basename = path_basename(basename_path);
11431143
if (basename)
1144-
_len += strlcpy(reply + _len, basename, sizeof(reply) - _len);
1144+
strlcpy_append(reply, sizeof(reply), &_len, basename);
11451145
else
1146-
_len += strlcpy(reply + _len, "UNKNOWN", sizeof(reply) - _len);
1146+
strlcpy_append(reply, sizeof(reply), &_len, "UNKNOWN");
11471147
}
11481148
else
1149-
_len += strlcpy(reply + _len, "UNKNOWN", sizeof(reply) - _len);
1149+
strlcpy_append(reply, sizeof(reply), &_len, "UNKNOWN");
11501150

11511151
_len += snprintf(reply + _len, sizeof(reply) - _len,
11521152
",crc32=%lx\n", (unsigned long)content_get_crc());

gfx/drivers/gl2.c

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4548,16 +4548,15 @@ static void *gl2_init(const video_info_t *video,
45484548

45494549
if (vendor && *vendor)
45504550
{
4551-
_len = strlcpy(gl->device_str, vendor, sizeof(gl->device_str));
4552-
gl->device_str[ _len] = ' ';
4553-
gl->device_str[++_len] = '\0';
4551+
strlcpy_append(gl->device_str, sizeof(gl->device_str), &_len, vendor);
4552+
strlcpy_append(gl->device_str, sizeof(gl->device_str), &_len, " ");
45544553
}
45554554

45564555
if (renderer && *renderer)
4557-
strlcpy(gl->device_str + _len, renderer, sizeof(gl->device_str) - _len);
4556+
strlcpy_append(gl->device_str, sizeof(gl->device_str), &_len, renderer);
45584557

45594558
if (version && *version)
4560-
video_driver_set_gpu_api_version_string(version);
4559+
video_driver_set_gpu_api_version_string(version);
45614560
}
45624561

45634562
#ifdef _WIN32

gfx/video_driver.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3189,10 +3189,13 @@ size_t video_driver_get_window_title(char *s, size_t len)
31893189
video_driver_state_t *video_st = &video_driver_st;
31903190
if (s && (video_st->flags & VIDEO_FLAG_WINDOW_TITLE_UPDATE))
31913191
{
3192-
strlcpy(s, video_st->window_title, len);
3192+
size_t n = strlcpy(s, video_st->window_title, len);
31933193
video_st->flags &= ~VIDEO_FLAG_WINDOW_TITLE_UPDATE;
3194+
if (n >= len)
3195+
return len ? len - 1 : 0;
3196+
return n;
31943197
}
3195-
return video_st->window_title_len;
3198+
return 0;
31963199
}
31973200

31983201
void video_driver_update_title(void *data)

menu/menu_displaylist.c

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2764,14 +2764,16 @@ static int create_string_list_rdb_entry_string(
27642764
{
27652765
char tmp[128];
27662766
char out_lbl[NAME_MAX_LENGTH];
2767-
size_t _len = strlcpy(out_lbl, label, sizeof(out_lbl));
2768-
_len += strlcpy(out_lbl + _len, "|", sizeof(out_lbl) - _len);
2769-
_len += strlcpy(out_lbl + _len, actual_string, sizeof(out_lbl) - _len);
2770-
_len += strlcpy(out_lbl + _len, "|", sizeof(out_lbl) - _len);
2771-
strlcpy(out_lbl + _len, path, sizeof(out_lbl) - _len);
2772-
_len = strlcpy(tmp, desc, sizeof(tmp));
2773-
_len += strlcpy(tmp + _len, ": ", sizeof(tmp) - _len);
2774-
strlcpy(tmp + _len, actual_string, sizeof(tmp) - _len);
2767+
size_t _len = 0;
2768+
strlcpy_append(out_lbl, sizeof(out_lbl), &_len, label);
2769+
strlcpy_append(out_lbl, sizeof(out_lbl), &_len, "|");
2770+
strlcpy_append(out_lbl, sizeof(out_lbl), &_len, actual_string);
2771+
strlcpy_append(out_lbl, sizeof(out_lbl), &_len, "|");
2772+
strlcpy_append(out_lbl, sizeof(out_lbl), &_len, path);
2773+
_len = 0;
2774+
strlcpy_append(tmp, sizeof(tmp), &_len, desc);
2775+
strlcpy_append(tmp, sizeof(tmp), &_len, ": ");
2776+
strlcpy_append(tmp, sizeof(tmp), &_len, actual_string);
27752777
menu_entries_append(list, tmp, out_lbl,
27762778
enum_idx,
27772779
0, 0, 0, NULL);

0 commit comments

Comments
 (0)