1891882 - Free of uninitialized pointer in nr_reg_register_callback()

Status: RESOLVED FIXED

(Core :: WebRTC: Networking, defect)

Description

[mozillabugs]

nr_reg_register_callback() (dom/media/webrtc/transport/third_party/nrappkit/src/registry/registrycb.c) destroys (and frees via RFREE()) various structures pointed to by an uninitialized pointer if an error occurs.

The bug is that the r_assoc * assoc on line 121 is not initialized. If control ends up on line 141, and the call to r_create_assoc() on line 147 fails (producing a nonzero status r), then control moves to line 180. (Note that r_assoc_create() doesn't initialize assoc on failure). The test on line 183 will pass, and line 185 eventually will get control. At that point, create_assoc is 1, and assoc is still uninitialized. If it is nonzero, line 185 calls nr_reg_assoc_destroy() on it, which destroys and frees various structures pointed to by the bogus pointer.

r_assoc_create() can fail only on OOM. The allocator RCALLOC() resolves to malloc(), so it's fallible.

A debugger-based POC is:

1. Run FF and attach a debugger to it.
2. Set a BP on nr_reg_register_callback().
3. Load https://webrtc.github.io/samples/src/content/peerconnection/audio/ .
4. Click Call.
5. When the BP fires, step to (but not into) line 147 (the conditions on lines 139-40 are always true in this POC).
6. Step into r_assoc_create() and simulate an OOM on the first call to RCALLOC() in that function.
7. Step back out into nr_reg_register_callback().
8. Step to (but not into) line 185.
9. At this point, assoc is still uninitialized.
10. Step into nr_reg_assoc_destroy() and watch it use the bogus pointer to destroy and free various objects.

Code from FIREFOX_124_0_2_RELEASE:

dom/media/webrtc/transport/third_party/nrappkit/src/registry/registrycb.c:

    117: int
    118: nr_reg_register_callback(NR_registry name, char action, void (*cb)(void *cb_arg, char action, NR_registry name), void *cb_arg)
    119: {
    120:     int r, _status;
    121:     r_assoc *assoc;
    122:     int create_assoc = 0;
    123:     nr_reg_cb_info *info;
    124:     int create_info = 0;
    125:     unsigned char cb_id[SIZEOF_CB_ID];
    ...
    139:     if ((r=r_assoc_fetch(nr_registry_callbacks, name, strlen(name)+1, (void*)&assoc))) {
    140:       if (r == R_NOT_FOUND)
    141:         create_assoc = 1;
    142:       else
    143:         ABORT(r);
    144:     }
    145: 
    146:     if (create_assoc) {
    147:       if ((r=r_assoc_create(&assoc, r_assoc_crc32_hash_compute, 5)))
    148:         ABORT(r);
    ...
    152:     }
    ...
    180:   abort:
    ...
    183:     if (_status) {
    184:       if (create_info && info) RFREE(info);
    185:       if (create_assoc && assoc) nr_reg_assoc_destroy(&assoc);
    186:     }
    187:     return(_status);
    188: }

dom/media/webrtc/transport/third_party/nrappkit/src/util/libekr/r_assoc.c -- note that the function doesn't initialize *assocp unless everything is successful:

    122: int r_assoc_create(assocp,hash_func,bits)
    123:   r_assoc **assocp;
    ...
    126:   {
    127:     r_assoc *assoc=0;
    128:     int _status;
    129: 
    130:     if(!(assoc=(r_assoc *)RCALLOC(sizeof(r_assoc))))
    131:       ABORT(R_NO_MEMORY);
    132:     assoc->size=(1<<bits);
    ...
    136:     if(!(assoc->chains=(r_assoc_el **)RCALLOC(sizeof(r_assoc_el *)*
    137:       assoc->size)))
    138:       ABORT(R_NO_MEMORY);
    139: 
    140:     *assocp=assoc;
    141: 
    142:     _status=0;
    143:   abort:
    144:     if(_status){
    145:       r_assoc_destroy(&assoc);
    146:     }
    147:     return(_status);
    148:   }

Comment 1

[mozillabugs]

How would Mozilla like to handle upstream reporting for this bug?

Comment 2

[Daniel Veditz [:dveditz]]

ni? to :jib for comment 1

Comment 3

[Byron Campen [:bwc]]

We're the de-facto official repo of nICEr and nrappkit at this point, so we'd be pushing our change if anything.