Skip to content

Potential NULL Dereference in cleanup_old_requests #5

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.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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