Skip to content

Possible new[] / scalar delete mismatch for RTCAudioSink audio samples #52

Description

@OvOhao

Possible new[] / scalar delete mismatch for RTCAudioSink audio samples

I found a possible allocation/deallocation mismatch (CWE-762, undefined behavior /
heap-corruption class). The audio-sample buffer is allocated with array new[] but freed
with scalar delete on the conversion path, on every audio data event.

Files: src/interfaces/rtc_audio_sink.cc, src/dictionaries/node_webrtc/rtc_on_data_event_dict.cc

Allocation (RTCAudioSink::OnData) — array new[]:

std::unique_ptr<uint8_t[]> audio_data_copy(new uint8_t[byte_length]);   // ARRAY new
...
memcpy(audio_data_copy.get(), audio_data, byte_length);
Dispatch(CreateCallback<RTCAudioSink>(
    [this, audio_data_copy = std::move(audio_data_copy), ...]() mutable {
      RTC_ON_DATA_EVENT_DICT dict = {audio_data_copy.release(), ...};   // -> dict.samples (uint8_t*)
      ...
    }));

Deallocation (TO_NAPI_IMPL(RTC_ON_DATA_EVENT_DICT, pair)) — scalar delete:

std::unique_ptr<uint8_t> samples(dict.samples);          // scalar unique_ptr -> scalar delete
...
auto maybeArrayBuffer = Napi::ArrayBuffer::New(
    env, samples.release(), byteLength,
    [](Napi::Env, void *samples) { delete static_cast<uint8_t *>(samples); });  // scalar delete on a new[] allocation

The samples buffer is allocated with new uint8_t[byte_length] but is owned here by a
std::unique_ptr<uint8_t> (scalar, not uint8_t[]) and ultimately freed by the
Napi::ArrayBuffer finalizer using scalar delete. Mismatching new[] with scalar delete
is undefined behavior (ASan/UBSan alloc-dealloc-mismatch; on some allocators it corrupts the
heap). It fires on every audio frame delivered to an RTCAudioSink.

The sibling code frees the same kind of buffer correctly with delete[]
(src/interfaces/rtc_audio_source.hh and src/converters/webrtc.cc), which confirms the
scalar delete/unique_ptr<uint8_t> here is an oversight rather than a deliberate choice.

JS trigger:

const { RTCAudioSink } = require('wrtc').nonstandard   // or the fork's equivalent
const sink = new RTCAudioSink(audioTrack)
sink.ondata = () => {}                                  // every audio frame -> new[]/scalar-delete mismatch

Suggested fix: use std::unique_ptr<uint8_t[]> samples(dict.samples); and a finalizer that
does delete[] static_cast<uint8_t *>(samples); (matching the allocation and the sibling
converters).

Related latent double-free (same lambda): if the conversion returns Invalid after
samples.release() has handed the pointer to the ArrayBuffer, the caller in
rtc_audio_sink.cc runs delete[] dict.samples while the ArrayBuffer finalizer will free the
same pointer again. Reported as latent — I could not force a mid-function napi throw from JS.

Note: verify against the actively-maintained fork you target (e.g. @roamhq/wrtc); the
RTCAudioSink/rtc_on_data_event_dict conversion code is shared across the node-webrtc forks.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions