1424891 - MOZ_CRASH(Unable to serialize principal.) creating blob uri within moz-icon

Status: RESOLVED DUPLICATE of bug 1424261

(Firefox :: Untriaged, defect)

Description

[Abdulrahman Alqabandi]

Attachment: crashicon.html

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/62.0.3202.94 Safari/537.36

Steps to reproduce:

Originally reported https://bugzilla.mozilla.org/show_bug.cgi?id=1424261#c5

When attempting to create a blob uri within a moz-icon, a tab crash occurs that indicates possible crash corruption.




Actual results:

Some frames appear to be hex which may be a sign of memory corruption.

https://crash-stats.mozilla.com/report/index/b8b3001b-e0b2-42ec-957d-463540171212
https://crash-stats.mozilla.com/report/index/87bf5f3a-51f0-41fc-a831-4364b0171212
https://crash-stats.mozilla.com/report/index/68a44f6a-88fb-43a2-9431-0a2100171212


Expected results:

No crash

Comment 1

[:Gijs (he/him)]

This looks like something's going wrong in IPC/blob land to me, but I'm not an expert. baku/bkelly?

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

Andrea is definitely more knowledgeable about blob code.  CC'ing a couple more folks who might know about this.

Comment 3

[Andrew McCreight [:mccr8]]

The crashes are all intentional crashes at: MOZ_CRASH(Unable to serialize principal.)

Weird stacks values like that are probably due to JIT code and are expected.

So I don't think there are any memory corruption issues apparent here. It is probably a good idea for somebody to figure out if the principal-related crash has any security implications.

Comment 4

[:Gijs (he/him)]

(In reply to Andrew McCreight [:mccr8] from comment #3)
> The crashes are all intentional crashes at: MOZ_CRASH(Unable to serialize
> principal.)
> 
> Weird stacks values like that are probably due to JIT code and are expected.
> 
> So I don't think there are any memory corruption issues apparent here. It is
> probably a good idea for somebody to figure out if the principal-related
> crash has any security implications.

This is because this check fails for principals composed of a moz-icon URI, which gets passed as an nsMozIconURI* here (from what I can tell in xcode, anyway):

https://dxr.mozilla.org/mozilla-central/rev/93b37aa497c48a6e28a9463eeb753b2ce3964f42/xpcom/io/nsBinaryStream.cpp#323-325

  nsCOMPtr<nsIClassInfo> classInfo = do_QueryInterface(aObject);
  nsCOMPtr<nsISerializable> serializable = do_QueryInterface(aObject);

  ...

  if (NS_WARN_IF(!classInfo) || NS_WARN_IF(!serializable)) {
    return NS_ERROR_NOT_AVAILABLE;
  }


I'm a bit confused by this because:

https://dxr.mozilla.org/mozilla-central/rev/93b37aa497c48a6e28a9463eeb753b2ce3964f42/image/decoders/icon/nsIconURI.cpp#69-75

NS_INTERFACE_MAP_BEGIN(nsMozIconURI)
  NS_INTERFACE_MAP_ENTRY(nsIMozIconURI)
  NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIURI)
  NS_INTERFACE_MAP_ENTRY(nsIURI)
  NS_INTERFACE_MAP_ENTRY(nsIIPCSerializableURI)
  NS_INTERFACE_MAP_ENTRY_CONDITIONAL(nsINestedURI, mIconURL)
NS_INTERFACE_MAP_END

so I don't understand how we implement nsIIPCSerializableURI but not classinfo/serializable, and thus this write fails. URIs themselves seem to be serialized OK.

Valentin, you looked at the interface definitions here recently, is this something you understand, and/or do you know someone who does? IPC (de)serialization is not really my area.


Note that in bug 1424261 hopefully we finish restricting moz-icon: from content, so it shouldn't be possible to encounter this case with moz-icon. That said, perhaps we need to audit our nsIURI implementations so we ensure we don't run into this with other URIs? Or perhaps I'm just wildly off...

Comment 5

[Valentin Gosu [:valentin] (he/him)]

(In reply to :Gijs from comment #4)
> so I don't understand how we implement nsIIPCSerializableURI but not
> classinfo/serializable, and thus this write fails. URIs themselves seem to
> be serialized OK.
> 
> Valentin, you looked at the interface definitions here recently, is this
> something you understand, and/or do you know someone who does? IPC
> (de)serialization is not really my area.

So, I didn't look into this too much, but I think the problem here is that nsMozIconURI doesn't implement nsISerializable (which is different from nsIIPCSerializableURI) - which is why serialization fails.
 
> Note that in bug 1424261 hopefully we finish restricting moz-icon: from
> content, so it shouldn't be possible to encounter this case with moz-icon.
> That said, perhaps we need to audit our nsIURI implementations so we ensure
> we don't run into this with other URIs? Or perhaps I'm just wildly off...

The only other nsIURI implementation that doesn't implement nsISerializable is NullPrincipalURI.

Comment 6

[Andrew McCreight [:mccr8]]

I'm going to mark this sec-other because it sounds like it is not a security issue, but it refers to a sec bug.

Comment 7

[Andrea Marchesini [:baku]]

It seems that this bug cannot be reproduced after bug 1424261.
Maybe this bug can be reproduced using a JSM, but not content.
valentin, can you confirm it?

Comment 8

[Valentin Gosu [:valentin] (he/him)]

Indeed, in Bug 1424261 we disallowed nesting any scheme other than file under moz-icon://
I can't reproduce this bug anymore, and I don't think you can reproduce it in JSM either.

Comment 9

[Andrea Marchesini [:baku]]

Let's close it as dup of bug 1424261.