1614249 - Potential UAF in ipc::MessageChannel::Close()

Status: RESOLVED INVALID

(Core :: IPC, defect)

Description

[Deian Stefan]

Attachment: messagechannel.ll

Our tool flagged a potential use-after-free in ipc::MessageChannel::Close(). The tool operates on LLVM IR, where I think the UAF is pretty clear. I'm not super sure if this is a real bug or a false positive (I don't fully grok how refcounting and move within lambda play). Apologize ahead of time if this is a false report.

Mapping the LLVM back to source, I think we're looking at these lines:

    RefPtr<RefCountedMonitor> monitor(mMonitor);
    auto exit = MakeScopeExit([m = std::move(monitor)]() { m->Unlock(); }); 

    if (ChannelError == mChannelState || ChannelTimeout == mChannelState) {
      // See bug 538586: if the listener gets deleted while the
      // IO thread's NotifyChannelError event is still enqueued
      // and subsequently deletes us, then the error event will
      // also be deleted and the listener will never be notified
      // of the channel error.
      if (mListener) {
        exit.release();  // Explicitly unlocking, clear scope exit.  
        mMonitor->Unlock();  // Tool thinks the use of the freed thing is here

Attached is relevant path we flagged on FF compiled clang -O2.

Comment 1

[Deian Stefan]

Sorry about this. If this were moving mMonitor the bug would be valid, but that's not the case here. I think the basic block (from the ref counting) that free's the object in the LLVM IR is essentially dead code.