The attached crash information was submitted via the ASan Nightly Reporter on mozilla-central-asan-nightly revision 85.0a1-20201210155912-https://hg.mozilla.org/mozilla-central/rev/7a6d6b986a1ed263db6d19ecbd56e2f4210870cf.

For detailed crash information, see attachment.

From the attached file, it crashed when flipped the DirectoryLockImpl::mPending. And that means DirectoryLockImpl::NotifyOpenListener was called twice.

Anyway, mPending was added in bug 1680031. Not sure if this is a regression or the issue is revealed so I put it in see also now.

Adding the signature, we have at least one recent crash report from that function from a regular build.

Tom, I am not sure why you think DirectoryLockImpl::NotifyOpenListener was called twice here. It's not the diagnostic assertion that's triggered here AFAIU, but it's a UAF.

(In reply to Simon Giesecke [:sg] [he/him] from comment #4)

Adding the signature, we have at least one recent crash report from that function from a regular build.

Tom, I am not sure why you think DirectoryLockImpl::NotifyOpenListener was called twice here. It's not the diagnostic assertion that's triggered here AFAIU, but it's a UAF.

Ah, you are right. I was thinking at crashed the assertion to ensure mPending can only be flipped once, but I was wrong. This is a UAF.

Just looking at the stacks, it looks like the line mQuotaManager->RemovePendingDirectoryLock(*this); destroys this, so there's a UAF when it runs mPending.Flip();.

(In reply to Andrew McCreight [:mccr8] from comment #6)

Just looking at the stacks, it looks like the line mQuotaManager->RemovePendingDirectoryLock(*this); destroys this, so there's a UAF when it runs mPending.Flip();.

Right. Then it's a regression from Bug 1680031. We need to pin this, or flip the order of these two calls. Jan, can you give that a look?

This is a UAF, and a write, but the use seems to be immediately after the free so I don't know how exploitable it would actually be. You'd probably need some other thread to get allocated the same block, but I don't know enough about how jemalloc threading works to know if that might happen right away.

Ok, will take a look. Thanks for the initial investigation.

Since this was also detected by PHC bug 1683035 comment 0 has stack traces of where the affected object was allocated and then released.

(In reply to Gabriele Svelto [:gsvelto] from comment #11)

Since this was also detected by PHC bug 1683035 comment 0 has stack traces of where the affected object was allocated and then released.

AFAIU, that stack trace is the same situation as that in the stack trace attached here. But the one here is clearer since there's less inlining involved.

https://hg.mozilla.org/integration/autoland/rev/7d698f3696372c7dbc8473d6837fa82d482f7992
https://hg.mozilla.org/mozilla-central/rev/7d698f369637

Is this bug part of the bug bounty program?

Please request beta uplift when you get a chance.

(In reply to petcuandrei from comment #15)

Is this bug part of the bug bounty program?

Thanks for pointing this out. The sec-bounty? flag should have been set. I'll set it now.

The bounty program meets about once a week, so they'll consider this for a bug bounty when they meet next.

We don't seem to get crashes here after 85.0b6. Does this only affect LSNG?

Well, it also affects IndexedDB and DOM cache. However LSNG is much more used by sites so there are probably now less crashes after 86.0b6 when LSNG is disabled.

Comment on attachment 9196055 [details]
Bug 1682100 - Improve iteration of blocking directory locks; r=#dom-workers-and-storage-reviewers,sg

Beta/Release Uplift Approval Request

• User impact if declined: Users experience crashes in some situations.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): It's a simple change that makes sure an object is kept alive until its method is being executed.
• String changes made/needed: None

Comment on attachment 9196055 [details]
Bug 1682100 - Improve iteration of blocking directory locks; r=#dom-workers-and-storage-reviewers,sg

approved for 85.0b9

https://hg.mozilla.org/releases/mozilla-beta/rev/3e0ee91cdaf4