827865 - crash in nsTArrayInfallibleAllocator::SizeTooBig, called from toolkit/components/url-classifier/HashStore.cpp

Status: RESOLVED WONTFIX

(Toolkit :: Safe Browsing, defect, P2)

Description

[Mats Palmgren (inactive)]

This bug was filed from the Socorro interface and is 
report bp-0a14477b-0b05-47a5-b14e-40b202130107 .
============================================================= 

mozalloc_abort	memory/mozalloc/mozalloc_abort.cpp:23
nsTArrayInfallibleAllocator::SizeTooBig	obj-firefox/dist/include/nsTArray.h:73
nsTArray_base<nsTArrayDefaultAllocator>::EnsureCapacity	obj-firefox/dist/include/nsTArray-inl.h:112
nsTArray<char,nsTArrayDefaultAllocator>::InsertElementsAt	obj-firefox/dist/include/nsTArray.h:1074
nsTArray<char,nsTArrayDefaultAllocator>::SetLength	obj-firefox/dist/include/nsTArray.h:1034
mozilla::safebrowsing::InflateReadTArray<unsigned char>	toolkit/components/url-classifier/HashStore.cpp:596
mozilla::safebrowsing::ByteSliceRead	toolkit/components/url-classifier/HashStore.cpp:662
mozilla::safebrowsing::HashStore::ReadSubPrefixes	toolkit/components/url-classifier/HashStore.cpp:714
mozilla::safebrowsing::HashStore::ReadHashes	toolkit/components/url-classifier/HashStore.cpp:389
mozilla::safebrowsing::HashStore::BeginUpdate	toolkit/components/url-classifier/HashStore.cpp:405
mozilla::safebrowsing::Classifier::ApplyUpdates	toolkit/components/url-classifier/Classifier.cpp:384 
[...]


Possible regression from bug 673470?
Maybe we read a bogus value for 'inLen' here (if the file is corrupt):
http://hg.mozilla.org/releases/mozilla-release/annotate/c23c45132139/toolkit/components/url-classifier/HashStore.cpp#l596

Comment 1

[Gian-Carlo Pascutto [:gcp]]

>(if the file is corrupt)

(Un)fortunately, the file is checksummed, so that's quite unlikely.

http://hg.mozilla.org/releases/mozilla-release/annotate/c23c45132139/toolkit/components/url-classifier/HashStore.cpp#l593

I'm guessing here the read is failing and we're getting a dummy size. That should have been a real check, not a debug-only assertion.

But still, how do we get to such an incomplete file that still has a valid checksum?

Comment 2

[Gian-Carlo Pascutto [:gcp]]

I think there's only one crash report with this HashStore::Read* signature (several with SizeTooBig, but those aren't actually related), and this user has an additional "visicom_antiphishing.dll" injected into Firefox.

Comment 3

[Gian-Carlo Pascutto [:gcp]]

Here's a second one without the DLL:
https://crash-stats.mozilla.com/report/index/5698e9c6-0c63-4f82-9c63-4f8152130109

Comment 4

[Gian-Carlo Pascutto [:gcp]]

Looking through the code, the writing part doesn't check the success of the call:
http://hg.mozilla.org/releases/mozilla-release/annotate/c23c45132139/toolkit/components/url-classifier/HashStore.cpp#l574

That could in theory explain the bad file with a good MD5.

The assertion should have been a real check even in release mode. That said, I can't see how that 4 byte write could fail and the subsequent write not, doesn't really make sense.

However, I'm not sure nsIOutputStream->Write is returning NS_OK or not if it didn't write all bytes requested. If that's not the case, the WriteTArray and friends functions should do extra error checking.

Comment 5

[François Marier [:francois]]

No crash reports since Firefox 25. Closing.