#22626 closed defect (fixed)

Missing stream NULL check in tor_compress_impl

Reported by: teor Owned by: ahf
Priority: Medium Milestone: Tor: 0.3.1.x-final
Component: Core Tor/Tor Version: Tor: 0.3.1.1-alpha
Severity: Normal Keywords: memory-safety
Cc: ahf Actual Points:
Parent ID: #22502 Points: 1
Reviewer: Sponsor:

Description

The second time we create a stream in tor_compress_impl, we don't check if it's NULL.

Child Tickets

Change History (6)

comment:1 Changed 23 months ago by teor

Parent ID: #22502
Points: 0.11

Given the number of times we don't check the output of tor_compress_new(), we should probably hard assert if it would return NULL, or check in all those places, or something?

comment:2 Changed 23 months ago by nickm

Probably asserting makes more sense?

comment:3 Changed 23 months ago by nickm

Owner: set to ahf
Status: newassigned

comment:4 Changed 23 months ago by teor

I wonder if it would be best to tor_assert() on UNKNOWN_METHOD, or BUG(), and then return a non-NULL state. An edge case where we don't check state correctly before calling could easily turn into a DoS bug.

    case UNKNOWN_METHOD:
      goto err;
  }

  atomic_counter_add(&total_compress_allocation,
                     sizeof(tor_compress_state_t));
  return state;

 err:
  tor_free(state);
  return NULL;

comment:5 Changed 23 months ago by teor

Also, there is a fix for this in the branch in #22502, but it's not the tor_assert() fix.

comment:6 Changed 22 months ago by nickm

Resolution: fixed
Status: assignedclosed

merged as part of #22502

Note: See TracTickets for help on using tickets.