Skip to content

libobs: bound volmeter plane index against MAX_AV_PLANES#737

Open
summeroff wants to merge 1 commit into
streamlabsfrom
harden-volmeter-plane-bounds
Open

libobs: bound volmeter plane index against MAX_AV_PLANES#737
summeroff wants to merge 1 commit into
streamlabsfrom
harden-volmeter-plane-bounds

Conversation

@summeroff

Copy link
Copy Markdown

What

Adds an explicit plane_nr < MAX_AV_PLANES bound to the two plane-scan loops in libobs/obs-audio-controls.c (volmeter_process_peak and volmeter_process_magnitude).

- for (int plane_nr = 0; channel_nr < nr_channels; plane_nr++) {
+ for (int plane_nr = 0; channel_nr < nr_channels && plane_nr < MAX_AV_PLANES; plane_nr++) {

Why

Both loops index data->data[plane_nr] while plane_nr is advanced solely by the channel_nr < nr_channels condition — plane_nr itself is never checked against MAX_AV_PLANES.

It is safe today only because nr_channels is produced by get_nr_channels_from_audio_data(), which counts the non-NULL planes in the same data->data[] array. That guarantees the loop encounters enough planes before plane_nr leaves bounds, for any plane layout (contiguous, interleaved, or trailing-NULL).

The footgun: the safety lives in a different function than the unguarded access. If nr_channels were ever derived from a source's declared channel/speaker layout instead of from counting planes, the loop would read past data->data[MAX_AV_PLANES - 1]. Because MAX_AUDIO_CHANNELS == MAX_AV_PLANES == 8, the existing CLAMP in get_nr_channels_from_audio_data() would not catch that case either.

This is a behavior-preserving hardening change — under the current invariant the new guard is never the condition that ends the loop.

Testing

  • Built the libobs target locally (VS 2022 / RelWithDebInfo) — compiles clean, no new warnings.
  • Change is a no-op under current data flow; the guard only bounds the index.

Note on the clang-format CI check

The clang-format check is expected to come back red, and it is not caused by this change. CI runs clang-format (v19) over the entire changed file, and obs-audio-controls.c carries one pre-existing deviation unrelated to this PR (the obs_fader_add_callback signature, last formatted under the old 80-col config). That line was deliberately left untouched to keep this diff minimal and focused on the actual fix. Happy to fold in the one-line format normalization if preferred.

🤖 Generated with Claude Code

volmeter_process_peak() and volmeter_process_magnitude() walk
data->data[] with plane_nr advanced solely by the loop's
`channel_nr < nr_channels` condition, so plane_nr itself is never
checked against MAX_AV_PLANES. This is safe today only because
nr_channels comes from get_nr_channels_from_audio_data(), which counts
the non-NULL planes in the same array - guaranteeing the loop finds
enough planes before plane_nr leaves bounds, regardless of plane layout.

If nr_channels were ever sourced from a declared channel/speaker layout
instead of from counting planes, the loop would index past
data->data[MAX_AV_PLANES - 1] (an out-of-bounds read), and since
MAX_AUDIO_CHANNELS == MAX_AV_PLANES the existing CLAMP would not catch
it. Add an explicit `plane_nr < MAX_AV_PLANES` guard to both loops.

Behavior is unchanged under the current invariant; this only hardens
against a future change to how nr_channels is derived.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant