Skip to content

Commit 44b5197

Browse files
committed
cheevos/cheevos_client: NULL-check allocations in HTTP dispatch and badge download queue
Two unchecked alloc sites in the rc_client glue layer. === cheevos/cheevos_client.c: rcheevos_client_server_call === rc_client_http_task_data_t *taskdata = (rc_client_http_task_data_t*) malloc(sizeof(rc_client_http_task_data_t)); taskdata->callback = callback; /* NULL-deref on OOM */ taskdata->callback_data = callback_data; Unchecked malloc; the two field writes NULL-deref on OOM. This is the producer side of every rc_client server call (achievement unlock, leaderboard submit, session heartbeat, etc.). Failing silently isn't safe here - rc_client internally tracks outstanding requests via the callback contract: it expects exactly one callback invocation per call, success or failure. If we return without invoking the callback, rc_client waits forever for a response that will never come, which manifests as the achievements submenu sitting permanently on 'Awaiting response' or 'Logging in...'. Fix: NULL-check the malloc. On failure synthesise an error response (zeroed server_response - same form the existing rcheevos_client_http_task_callback above uses for network-failure and http_task-null paths at lines 252 and 257) and invoke the caller's callback with it before returning. rc_client sees the failed response and bubbles the error up through its state machine normally. === cheevos/cheevos_client.c: rcheevos_client_download_achievement_badges === rc_client_download_queue_t *queue = (rc_client_download_queue_t*) calloc(1, sizeof(*queue)); queue->client = client; /* NULL-deref on OOM */ queue->game = rc_client_get_game_info(client); ... Unchecked calloc; five field writes below NULL-deref on OOM. Function is void-returning and has no callback contract to honour (unlike the case above). Fix: NULL-check and early return. Badge downloading is best- effort (badges cache to disk and show on next session; missing ones fall back to the default placeholder icon in the menu), so silent skip is the right policy. === Not a bug, verified clean === Also audited during this sweep: * cheevos/cheevos_menu.c:141 - NULL-checked with explicit error-return path. * tasks/task_core_updater.c - 5 calloc sites across five task-spawn functions; each uses the goto-error pattern with a 'if (!handle || ...) goto error' sanity check. * tasks/task_content.c - 4 sites, nested if-chain guards each inner alloc. * tasks/task_playlist_manager.c - 2 sibling calloc sites, both use the !pl_manager goto-error pattern. * input/input_driver.c:1779 (input_remote_new) - NULL-checked. * input/input_driver.c:4932 (osk realloc) - realloc-to-tmp with NULL check; returns false without corrupting state on OOM. === Thread-safety === rcheevos_client_server_call runs on whichever thread initiated the server call (usually the runloop/main thread through rc_client's state machine). The callback we invoke on OOM is the same one rc_client supplied and expects to be called on the same thread context; no lock changes. rcheevos_client_download_achievement_badges runs on the main thread during game load. === Reachability === server_call: every achievement-system network operation. Hit frequently during active play (unlock submissions, rich-presence pings, leaderboard updates). download_achievement_badges: once per game load with achievements enabled - gathered all badge URLs for the game and queues up to RCHEEVOS_CONCURRENT_BADGE_DOWNLOADS parallel HTTP fetches.
1 parent f07549b commit 44b5197

1 file changed

Lines changed: 22 additions & 0 deletions

File tree

cheevos/cheevos_client.c

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,19 @@ void rcheevos_client_server_call(const rc_api_request_t* request,
319319
rc_client_http_task_data_t *taskdata = (rc_client_http_task_data_t*)
320320
malloc(sizeof(rc_client_http_task_data_t));
321321

322+
/* NULL-check the malloc: the two field writes below NULL-deref
323+
* on OOM. On failure invoke the callback with a zeroed server
324+
* response so rc_client doesn't wait forever for a reply it
325+
* won't get; matches the zeroed-response convention used in
326+
* rcheevos_client_http_task_callback above for failed requests. */
327+
if (!taskdata)
328+
{
329+
rc_api_server_response_t server_response;
330+
memset(&server_response, 0, sizeof(server_response));
331+
callback(&server_response, callback_data);
332+
return;
333+
}
334+
322335
taskdata->callback = callback;
323336
taskdata->callback_data = callback_data;
324337

@@ -615,6 +628,15 @@ void rcheevos_client_download_achievement_badges(rc_client_t* client)
615628
rc_client_download_queue_t *queue = (rc_client_download_queue_t*)
616629
calloc(1, sizeof(*queue));
617630

631+
/* NULL-check the calloc: the five field writes below NULL-deref
632+
* on OOM. Function is void-returning; the user-visible
633+
* consequence of skipping on OOM is that badge textures don't
634+
* download this session (existing cached badges still display;
635+
* missing ones show the default placeholder). Triggered again
636+
* next time the game is loaded. */
637+
if (!queue)
638+
return;
639+
618640
queue->client = client;
619641
queue->game = rc_client_get_game_info(client);
620642
queue->list = rc_client_create_achievement_list(client,

0 commit comments

Comments
 (0)