Commit 60bda54
committed
cheevos/cheevos_client: fix OOM NULL-deref in badge download taskdata and debug JSON override
Two unchecked malloc sites in cheevos_client.c plus two
unchecked operations in the CHEEVOS_JSON_OVERRIDE debug path.
=== rcheevos_client_download_badge: taskdata malloc ===
The badge download path allocates a task-data struct and
immediately writes three fields before dispatching the HTTP
task:
taskdata = (rc_client_download_task_data_t*)malloc(sizeof(*taskdata));
taskdata->queue = queue;
strlcpy(taskdata->badge_fullpath, badge_fullpath, sizeof(taskdata->badge_fullpath));
strlcpy(taskdata->badge_name, badge_name, sizeof(taskdata->badge_name));
task_push_http_transfer_with_user_agent(url,
true, "GET", rcheevos_locals->user_agent_core,
rcheevos_client_download_task_callback, taskdata);
The malloc was unchecked. Two failure modes on OOM:
1. The three field writes immediately NULL-deref.
2. If the writes somehow didn't crash (they do),
task_push_http_transfer_with_user_agent would dispatch the
download task with a NULL taskdata user-data pointer. The
completion callback rcheevos_client_download_task_callback
dereferences it via callback_data->queue and
callback_data->badge_fullpath - so the crash would surface
much later on the task thread rather than at the alloc
site, making the OOM bug look like an unrelated task-queue
or HTTP-layer issue.
Fix: NULL-check the malloc and return false on failure. The
caller treats false as 'download failed', matching the
behaviour of the path_is_valid() early-return above
which returns false when the badge is already cached.
When invoked via the badge queue (rcheevos_client_fetch_next_
badge), the queue continues with the next badge on the next
poll.
=== CHEEVOS_JSON_OVERRIDE debug path: fopen and malloc ===
rcheevos_client_http_load_response is a debug-only function
compiled in when CHEEVOS_JSON_OVERRIDE is defined to a file
path that supplies a canned response. The pre-patch code was
FILE* file = fopen(CHEEVOS_JSON_OVERRIDE, "rb");
fseek(file, 0, SEEK_END);
_len = ftell(file);
...
contents = (char*)malloc(_len + 1);
fread((void*)contents, 1, _len, file);
...
contents[_len] = 0;
callback(contents, 200, callback_data);
with no NULL handling on either fopen or malloc. A missing or
unreadable override file, or an OOM on the contents alloc,
would segfault inside the debug path.
This is developer-only scaffolding (not compiled in any
shipping configuration), but the bugs are trivial and easy
to trip over when flipping the macro on during development.
Fix: NULL-check the fopen with an immediate
callback(NULL, 0, callback_data) + return to match the
zeroed-response contract used by the production callers when
the HTTP layer fails, and NULL-check the malloc with the
same early-return path (plus fclose() of the opened file).
=== Swept-clean in the same pass ===
Verified NULL-checked in cheevos/:
- cheevos_rvz.c: 22 alloc sites, all NULL-checked. This file
has been carefully audited previously.
- cheevos_client.c other 2 sites: contents malloc
(now guarded by this commit); rc_client_http_task
data malloc (guarded by prior audit commit with its own
comment in-place); download queue calloc (guarded
by prior audit commit).
- cheevos.c: descriptors malloc (guarded); retry info
malloc (guarded with proper free-task cleanup); shotname
malloc (guarded by 'if (shotname)' wrap); cdfs_file_t
malloc (intentionally unchecked - cdfs_open_file tolerates
NULL file and returns 0, causing the caller to fall through
to CHEEVOS_FREE(NULL) + cdfs_close_track cleanup).
- cheevos_menu.c: 2 sites - menuitems realloc (tmp-pattern,
NULL-checked, rolls back menuitem_capacity on failure) and
malloc branch (NULL-checked, resets capacity to 0 on
failure).
Thread-safety: the download_badge path runs on the main menu
thread at badge queue poll time. The JSON_OVERRIDE path runs
synchronously on whichever thread called rcheevos_client_
server_call - but since it's a debug-only path, no
concurrency concerns worth tracking.
Reachability: the download_badge OOM is reachable on normal
game load with Hardcore Achievements enabled - the badge
fetch queue fires on every newly-unlocked achievement and
each call takes this path. OOM here typically surfaces when
a large badge queue is being processed on a memory-starved
embedded target. The JSON_OVERRIDE crash is only reachable
during development.1 parent 31103bb commit 60bda54
1 file changed
Lines changed: 32 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
297 | 297 | | |
298 | 298 | | |
299 | 299 | | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
300 | 313 | | |
301 | 314 | | |
302 | 315 | | |
303 | 316 | | |
304 | 317 | | |
| 318 | + | |
| 319 | + | |
| 320 | + | |
| 321 | + | |
| 322 | + | |
| 323 | + | |
| 324 | + | |
| 325 | + | |
305 | 326 | | |
306 | 327 | | |
307 | 328 | | |
| |||
480 | 501 | | |
481 | 502 | | |
482 | 503 | | |
| 504 | + | |
| 505 | + | |
| 506 | + | |
| 507 | + | |
| 508 | + | |
| 509 | + | |
| 510 | + | |
| 511 | + | |
| 512 | + | |
| 513 | + | |
| 514 | + | |
483 | 515 | | |
484 | 516 | | |
485 | 517 | | |
| |||
0 commit comments