Skip to content

Potential NULL Dereference in cleanup_old_requests #5

@LinuxMainframe

Description

@LinuxMainframe

Major: Potential NULL Dereference in cleanup_old_requests

Priority: MAJOR

Type: Memory Safety

Location

libwsv5.c line 940 in cleanup_old_requests()

Issue

Code accesses old->response fields without checking if response is NULL:

if (now - (*req)->timestamp > 30) {
    pending_request_t *old = *req;
    *req = old->next;
    
    pthread_mutex_lock(&old->mutex);
    old->completed = true;
    old->response->success = false;  // ← NULL dereference if response is NULL!
    old->response->error_message = strdup("Request timeout");
    pthread_cond_broadcast(&old->cond);
    pthread_mutex_unlock(&old->mutex);
}

Impact

Race condition scenario:

  1. obsws_send_request() transfers ownership: req->response = NULL
  2. cleanup_old_requests() runs concurrently
  3. Tries to access old->response->success → NULL pointer dereference → crash

Additional Note

The current design has a subtle issue: after timeout cleanup, the response object might be partially populated. Consider whether timed-out requests should have their response freed entirely or kept with error flag.

Metadata

Metadata

Assignees

No one assigned

    Labels

    MAJORNot critical, but also important to fix ASAP

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions