Closed Bug 999697 Opened 6 years ago Closed 6 years ago

Faulty: Assertion failure: false (should be implemented or not used) in CompositableHost::CreatedIncrementalTexture

Categories

(Core :: Graphics: Layers, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla31

People

(Reporter: bjacob, Assigned: bjacob)

Details

Attachments

(2 files)

Attached file Faulty session
Found while fuzzing.
Group: core-security
Keywords: sec-critical
Ouch, this is another "PReinterpretCast" type bug.

The parent stack frame is in CompositableParentManager::ReceiveCompositableUpdate, and is doing:

bool
CompositableParentManager::ReceiveCompositableUpdate(const CompositableOperation& aEdit,
                                                     EditReplyVector& replyv)
{
  switch (aEdit.type()) {
    case CompositableOperation::TOpCreatedIncrementalTexture: {
      MOZ_LAYERS_LOG(("[ParentSide] Created texture"));
      const OpCreatedIncrementalTexture& op = aEdit.get_OpCreatedIncrementalTexture();

      CompositableParent* compositableParent =
        static_cast<CompositableParent*>(op.compositableParent());
      CompositableHost* compositable = compositableParent->GetCompositableHost();

      compositable->CreatedIncrementalTexture(compositableParent->GetCompositableManager(),
                                              op.textureInfo(),
                                              op.bufferRect());
      break;
    }

The CreatedIncrementalTexture above is the one crashing with this "unimplemented" assert.


From the log:

 2:51.30 [Faulty] pickle field {int} of value: 3 changed to: 2


From LayersMessages.h in the objdir:

class CompositableOperation MOZ_FINAL
{
public:
    enum Type {
        T__None,
        TOpUpdatePictureRect = 1,
        TOpCreatedIncrementalTexture,
        TOpPaintTextureRegion,

So "3 changed to 2" means: TOpPaintTextureRegion changed to TOpCreatedIncrementalTexture.

So we are, again, using an enum value from an untrusted IPC message, to decide how to reinterpret an existing object...

I'm removing the sec-critical keyword for now because I need some time to actually think about the security implications. Not fully clear to me here yet if the virtual call here is unsafe.
Keywords: sec-critical
OK, this is not actually a PReinterpretCast pattern, and not actually security sensitive.

All what's happening here is that we're calling a virtual method that, when not overridden, crashes. Instead, we just need to make this gracefully fail and propagate an error.
Group: core-security
Attachment #8411019 - Flags: review?(nical.bugzilla) → review+
https://hg.mozilla.org/mozilla-central/rev/e3bc878ad761
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in before you can comment on or make changes to this bug.