Closed Bug 1435994 Opened 7 years ago Closed 3 years ago

Crash in nsTSubstring<T>::Replace

Categories

(Core :: DOM: Security, defect, P2)

59 Branch
defect

Tracking

()

RESOLVED WORKSFORME
Tracking Status
firefox-esr52 --- unaffected
firefox58 --- wontfix
firefox59 - fix-optional
firefox60 --- affected

People

(Reporter: philipp, Assigned: ckerschb)

Details

(Keywords: crash, Whiteboard: [domsecurity-active])

Crash Data

This bug was filed from the Socorro interface and is report bp-f1e4018a-b966-458a-a2a8-415850180202. ============================================================= Top 10 frames of crashing thread: 0 xul.dll nsTSubstring<char>::Replace xpcom/string/nsTSubstring.cpp:650 1 xul.dll nsBase64Encoder::Write netwerk/base/nsBase64Encoder.cpp:27 2 xul.dll nsBinaryOutputStream::WriteFully xpcom/io/nsBinaryStream.cpp:133 3 xul.dll nsBinaryOutputStream::WriteBoolean xpcom/io/nsBinaryStream.cpp:157 4 xul.dll NS_WriteOptionalCompoundObject dist/include/nsIObjectOutputStream.h:118 5 xul.dll nsCSPContext::Write dom/security/nsCSPContext.cpp:1619 6 xul.dll nsBinaryOutputStream::WriteCompoundObject xpcom/io/nsBinaryStream.cpp:352 7 xul.dll NS_WriteOptionalCompoundObject dist/include/nsIObjectOutputStream.h:120 8 xul.dll ContentPrincipal::Write caps/ContentPrincipal.cpp:523 9 xul.dll nsBinaryOutputStream::WriteCompoundObject xpcom/io/nsBinaryStream.cpp:352 ============================================================= these crash reports with an infinite recursion in the stack are spiking up in volume since 2018-02-01. the increase in crashes seems to come from brazilian users with comments often pointing towards https://email.bol.uol.com.br/login as source of the crashes.
Component: General → XPCOM
This looks like we're crashing in some CSP code; the topmost interesting frame is: 89 xul.dll ContentPrincipal::Write(nsIObjectOutputStream*) caps/ContentPrincipal.cpp:523 which is: https://hg.mozilla.org/releases/mozilla-release/annotate/c2db4a50dc5c/caps/ContentPrincipal.cpp#l523 rv = NS_WriteOptionalCompoundObject(aStream, mCSP, NS_GET_IID(nsIContentSecurityPolicy), true); and interspersed in the crashing frames is: 86 xul.dll nsCSPContext::Write(nsIObjectOutputStream*) dom/security/nsCSPContext.cpp:1619 which is: https://hg.mozilla.org/releases/mozilla-release/annotate/c2db4a50dc5c/dom/security/nsCSPContext.cpp#l1619 nsresult rv = NS_WriteOptionalCompoundObject(aStream, mSelfURI, NS_GET_IID(nsIURI), true); and also: 83 xul.dll nsHostObjectURI::Write(nsIObjectOutputStream*) dom/file/nsHostObjectURI.cpp:95 which is: return NS_WriteOptionalCompoundObject(aStream, mPrincipal, NS_GET_IID(nsIPrincipal), true); So that all looks like a big circular mess. But none of this code is particularly new to 58; a lot of it has been around since FF 49 or even far before that. ni? to a couple people who might have better ideas about who to talk to and/or what might have changed in the 58 timeframe to cause problems here. (It's possible the website in question is doing something screwy which messes with our assumptions, too...)
Component: XPCOM → DOM: Security
Flags: needinfo?(mrbkap)
Flags: needinfo?(kmaglione+bmo)
Flags: needinfo?(ckerschb)
Christoph would know more about this than I would.
Flags: needinfo?(mrbkap)
It looks like we have a blob: URI object with a principal that has a CSP, and the CSP's self URI points back to the original URI object. I don't know what might have changed to trigger that. If I had to guess, I'd say the circular reference issue isn't new, but we weren't previously trying to send the problematic blob: URIs across processes as often.
Flags: needinfo?(kmaglione+bmo)
Although... that is a bit odd. The self URI in the CSP for a blob: URI's principal shouldn't be a blob: URI. It should be the URI of the principal it inherits. I wonder if we're attaching a CSP from a blob: document to a principal that it inherited from a non-blob: document... There are probably security implications to that. Either way, I suppose bug 965637 should fix this.
(In reply to Kris Maglione [:kmag] (long backlog; ping on IRC if you're blocked) from comment #3) > It looks like we have a blob: URI object with a principal that has a CSP, > and the CSP's self URI points back to the original URI object. That is very likely. For the loadingContext and loadingPrincipal we use weakPtrs and I think we haven't really thought about blobs for that. I'll take a closer look at this bug in the next days.
Assignee: nobody → ckerschb
Status: NEW → ASSIGNED
Flags: needinfo?(ckerschb)
Priority: -- → P2
Whiteboard: [domsecurity-active]
adding two crash signatures that look related.
Crash Signature: [@ nsTSubstring<T>::Replace] → [@ nsTSubstring<T>::Replace] [@ mozilla::PrintfTarget::vprint] [@ memcpy | nsBase64Encoder::Write]
and the one from fennec...
Crash Signature: [@ nsTSubstring<T>::Replace] [@ mozilla::PrintfTarget::vprint] [@ memcpy | nsBase64Encoder::Write] → [@ nsTSubstring<T>::Replace] [@ mozilla::PrintfTarget::vprint] [@ memcpy | nsBase64Encoder::Write] [@ nsTSubstring<T>::ReplacePrep]
What has probably changed is the canary in nsStringBuffer introduced in bug 1410276 in an effort to hunt down other problems.
Depends on: 1410276
Oh, I may be wrong. The canary crashes appear to be a small-ish subset of this crash.
No longer depends on: 1410276
This looks pretty frequent on release 58. We've only got a week left in the 59 cycle before the first RC build, but it would be great if we could come up with some sort of mitigation before then.
the crash volume of this has dropped on 2018-02-24 - probably there were changes made on the brazilian webmail site to fix this crashing issue.
We are pretty unlikely to fix this before 59 release. Looks like the issue improved dramatically with 59 beta 12, so that's encouraging.
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #12) > We are pretty unlikely to fix this before 59 release. Looks like the issue > improved dramatically with 59 beta 12, so that's encouraging. Hey Liz, the issue here improved and I don't think we will be able to fix that for 59, but I am still getting the |Monday Mar 12 -- Daily Release Tracking Alert| email. Can you update flags accordingly please?
Flags: needinfo?(lhenry)
Yes, it looks much better! I'll untrack. I think I will also follow up to ask that we don't send tracking alert messages for "fix-optional" issues.
Flags: needinfo?(lhenry)
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #14) > Yes, it looks much better! I'll untrack. I think I will also follow up to > ask that we don't send tracking alert messages for "fix-optional" issues. FWIW, I'd like to still receive tracking alerts for fix-optional issues. They may not be blockers, but I'd generally still like to have them on my radar in case I happen to have the bandwidth to fix them in time...

I am closing out old bugs and I think the problem reported here was fixed by bug 965637, hence marking as WORKSFORME.

Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.