WebAudio use-after-free [@mozilla::dom::SelfCountedReference<mozilla::dom::ScriptProcessorNode>::ForceDrop]

RESOLVED WORKSFORME

Status

()

defect
--
critical
RESOLVED WORKSFORME
6 years ago
2 years ago

People

(Reporter: posidron, Unassigned)

Tracking

(Blocks 1 bug, {crash, csectype-uaf, sec-critical})

Trunk
x86_64
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox22 disabled, firefox23- disabled, firefox24+ disabled, firefox-esr17 unaffected, b2g18 unaffected)

Details

Attachments

(1 attachment)

Reporter

Description

6 years ago
Posted file callstack
alloc & free: js/src/jsscript.cpp:1106,1112

/* Adjust the amount of memory this script source uses for source data,
   reallocating if needed. */
bool
ScriptSource::adjustDataSize(size_t nbytes)
{
    // Allocating 0 bytes has undefined behavior, so special-case it.
    if (nbytes == 0) {
        if (data.compressed != emptySource)
*           js_free(data.compressed);
        data.compressed = const_cast<unsigned char *>(emptySource);
        return true;
    }

    // |data.compressed| can be NULL.
*   void *buf = js_realloc(data.compressed, nbytes);
    if (!buf && data.compressed != emptySource)
        js_free(data.compressed);
    data.compressed = static_cast<unsigned char *>(buf);
    return !!data.compressed;
}

re-use: content/media/webaudio/AudioNode.h:88
  void ForceDrop(T* t)
  {
*   if (mRefCnt > 0) {
      mRefCnt = 0;
      t->Release();
    }
  }


Note: this happened till now only one time and am till now not able to reproduce that issue with a testcase.


Tested with m-i changeset: 131837:3a56d9a0b092
Reporter

Comment 1

6 years ago
Hit it now twice - only the stack for the re-use is constant.

It only happens with large testcases (each testcase has 200 calls of WebAudio functions) and a page reload after every 100th testcase.

Comment 2

6 years ago
I'm changing this code slightly in bug 836599.  Would be interesting to retest after that lands.
Depends on: 836599
Group: dom-core-security
Reporter

Updated

6 years ago
Blocks: 875414

Comment 3

6 years ago
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #2)
> I'm changing this code slightly in bug 836599.  Would be interesting to
> retest after that lands.

Can you please retest?
Flags: needinfo?(cdiehl)
Reporter

Comment 4

6 years ago
I am not seeing it anymore in my latest runs.
Flags: needinfo?(cdiehl)
\o/
Ehsan want to close fixed via bug 836599?
Assignee: nobody → ehsan
Flags: needinfo?(ehsan)
Reporter

Comment 6

6 years ago
I believe this got fixed in bug 873335. The testcase produced the same stack.
Bug 836599 was only landed in 24, so even if this is fixed by something else, 23 remains affected.  Are we going leave it enabled on 22?  Does this affect 22 otherwise?
(In reply to Christoph Diehl [:cdiehl] from comment #6)
> I believe this got fixed in bug 873335. The testcase produced the same stack.

Thanks for the update.
Depends on: 873335

Comment 9

6 years ago
(In reply to Andrew McCreight [:mccr8] from comment #7)
> Bug 836599 was only landed in 24, so even if this is fixed by something
> else, 23 remains affected.  Are we going leave it enabled on 22?  Does this
> affect 22 otherwise?

Web Audio is disabled on 22 and will be disabled on 23 once it gets to beta too.

And we probably need to uplift bug 836599 for 23 anyway.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Flags: needinfo?(ehsan)
Resolution: --- → FIXED
Whiteboard: [fixed by bug 836599]
Reporter

Comment 10

6 years ago
Hit it again.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 11

6 years ago
(In reply to Christoph Diehl [:cdiehl] from comment #10)
> Hit it again.

On which test case?
Reporter

Comment 12

6 years ago
still, no specific testcase.

Updated

6 years ago
Assignee: ehsan → nobody
Keywords: testcase-wanted
Whiteboard: [fixed by bug 836599]

Comment 13

6 years ago
Does this still happen?
Flags: needinfo?(cdiehl)
Reporter

Comment 14

6 years ago
Haven't seen it anymore lately - testing against m-i changeset: 133183:1c67a51e0fe5
Flags: needinfo?(cdiehl)

Comment 15

6 years ago
Cool, marking as FIXED.
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED

Comment 16

6 years ago
Mass moving Web Audio bugs to the Web Audio component.  Filter on duckityduck.
Component: Video/Audio → Web Audio
Group: media-core-security
Not tracking since we're not shipping web audio in ff23

Comment 18

6 years ago
Sorry Kyle, but this bug is not actionable without a test case.  Assigning it to somebody is not going to change that situation.
Assignee: ehsan → nobody
The bug is fixed ... I was just doing bookkeeping ...

Updated

6 years ago
Resolution: FIXED → WORKSFORME

Comment 20

6 years ago
Well yeah.  We don't know if it's actually fixed yet, we just can't reproduce it.

Updated

4 years ago
Group: core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.