1871838 - (CVE-2024-0745) stack-buffer-overflow webaudio/AudioEventTimeline.cpp:510 in AudioEventTimeline::GetValuesAtTimeHelperInternal

Status: RESOLVED FIXED

(Core :: Web Audio, defect, P1)

Description

[m.cooolie]

Attachment: poc.html

#Reproduce
OS:Win10 X64
123.0a1 (2023-12-19) (64-bit)

#Step:

1. python -m http.server 1337
2. python -m ffpuppet firefox.exe -p prefs.js -d -u http://localhost:1337/poc.html

#Analysis

missing boundary checks for ticks + start, detune + start, end - start

https://searchfox.org/mozilla-central/source/dom/media/webaudio/OscillatorNode.cpp#300

  void ProcessBlock(AudioNodeTrack* aTrack, GraphTime aFrom,
                    const AudioBlock& aInput, AudioBlock* aOutput,
                    bool* aFinished) override {
    MOZ_ASSERT(mSource == aTrack, "Invalid source track");
    TRACE("OscillatorNodeEngine::ProcessBlock");

    TrackTime ticks = mDestination->GraphTimeToTrackTime(aFrom);
    if (mStart == -1) {
      ComputeSilence(aOutput);
      return;
    }

    if (ticks + WEBAUDIO_BLOCK_SIZE <= mStart || ticks >= mStop) {
      ComputeSilence(aOutput);

    } else {
      aOutput->AllocateChannels(1);
      float* output = aOutput->ChannelFloatsForWrite(0);

      uint32_t start, end;
      FillBounds(output, ticks, start, end);

      float frequency[WEBAUDIO_BLOCK_SIZE];
      float detune[WEBAUDIO_BLOCK_SIZE];
      if (ParametersMayNeedUpdate()) {
        if (mFrequency.HasSimpleValue()) {
          std::fill_n(frequency, WEBAUDIO_BLOCK_SIZE, mFrequency.GetValue());
        } else {
          mFrequency.GetValuesAtTime(ticks + start, frequency + start,
                                     end - start);
        }
        if (mDetune.HasSimpleValue()) {
          std::fill_n(detune, WEBAUDIO_BLOCK_SIZE, mDetune.GetValue());
        } else {
          mDetune.GetValuesAtTime(ticks + start, detune + start, end - start);
        }
      }

Comment 1

[m.cooolie]

Attachment: log_ffp_asan_5560.log.6744.txt

asan log

Comment 2

[m.cooolie]

Attachment: prefs.js

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1853598

Comment 4

[Karl Tomlinson (:karlt)]

end < start, which always leads to 2^32 - 126 <= aSize < 2^32, for a write controlled by content. The loop on this (audio rendering) thread will eventually seg fault before exiting. If the OS has a guard page at the high address end of the stack, then I don't see a way for memory accessed by other threads to be corrupted. This thread does not perform nested event loops and so the stack should not grow large, even in unoptimized builds without a stack limit.

Comment 5

[Karl Tomlinson (:karlt)]

Attachment: Bug 1871838 simply produce silence if stop time is before start time r?padenot

OscillatorNode is now similar in this way to ConstantSourceNode.
https://searchfox.org/mozilla-central/rev/f6776253b65c05351b4004fe7c3353cac8d8a4af/dom/media/webaudio/ConstantSourceNode.cpp#88-89

Comment 6

[Karl Tomlinson (:karlt)]

Attachment: Bug 1871838 crashtest with OscillatorNode stop time before start time with AudioParam r?padenot

Depends on D197405

Comment 7

[Karl Tomlinson (:karlt)]

Comment on attachment 9370470 [details]
Bug 1871838 simply produce silence if stop time is before start time r?padenot

Security Approval Request

• How easily could an exploit be constructed based on the patch?: I'm not aware of a way to exploit this beyond DoS (comment 4), despite the content-controlled stack buffer write overflow.
  The first change in the patch is necessary and identifies how to trigger a crash.
  The second change is an assert identifying the flaw, and could be removed if preferred.
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
• Which older supported branches are affected by this flaw?: Versions from Fx 119
• If not all supported branches, which bug introduced the flaw?: Bug 1853598
• Do you have backports for the affected branches?: Yes
• If not, how different, hard to create, and risky will they be?: Same. Easy.
• How likely is this patch to cause regressions; how much testing does it need?: Very unlikely. Change is small and similar to existing code elsewhere.
• Is Android affected?: Yes

Comment 8

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 9370470 [details]
Bug 1871838 simply produce silence if stop time is before start time r?padenot

Approved to land and request uplift

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/47fc814524de
simply produce silence if stop time is before start time r=media-playback-reviewers,alwu

Comment 10

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/47fc814524de

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:karlt, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox122 to wontfix.

For more information, please visit BugBot documentation.

Comment 12

[Karl Tomlinson (:karlt)]

Attachment: Bug 1871838 simply produce silence if stop time is before start time r?padenot

OscillatorNode is now similar in this way to ConstantSourceNode.
https://searchfox.org/mozilla-central/rev/f6776253b65c05351b4004fe7c3353cac8d8a4af/dom/media/webaudio/ConstantSourceNode.cpp#88-89

Original Revision: https://phabricator.services.mozilla.com/D197405

Comment 13

[Phabricator Automation]

Uplift Approval Request

• Needs manual QE test: no
• Is Android affected?: yes
• Code covered by automated testing: yes
• Fix verified in Nightly: no
• Risk associated with taking this patch: very low
• String changes made/needed: none
• Explanation of risk level: Change is small and similar to existing code elsewhere. Affects only OscillatorNode
• Steps to reproduce for manual QE testing: Load https://bugzilla.mozilla.org/attachment.cgi?id=9370190
• User impact if declined: content-controlled stack buffer write overflow

Comment 14

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/c0d623436ac1

Comment 15

[Jesse Schwartzentruber (:truber)]

Attachment: advisory.txt

Comment 16

[BugBot [:suhaib / :marco/ :calixte]]

a month ago, tjr placed a reminder on the bug using the whiteboard tag [reminder-test 2024-03-05] .

karlt, please refer to the original comment to better understand the reason for the reminder.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/73447637506a
crashtest with OscillatorNode stop time before start time with AudioParam r=padenot

Comment 18

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/73447637506a

Comment 19

[Daniel Veditz [:dveditz]]

Making Firefox 122 security bugs public. [bugspam filter string: Pilgarlic-Towers]