1024801 - nsUrlClassifierPrefixSet::GetPrefixes leaks |prefixArray| if it does |return NS_ERROR_FILE_CORRUPTED|

Status: RESOLVED FIXED

(Toolkit :: Safe Browsing, defect)

Comment 1

[Andrew McCreight [:mccr8]]

Thinking about this some more, this one makes me a little nervous.  I'm guessing this array can be decently large, and I know with the old network cache they ended up getting corrupted or invalidating quite frequently, so maybe that's a possibility here.  I'll just fix this.

Comment 2

[Andrew McCreight [:mccr8]]

Attachment: Free prefixArray if nsUrlClassifierPrefixSet::GetPrefixes returns an error.

Comment 3

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 8446015 [details] [diff] [review]
Free prefixArray if nsUrlClassifierPrefixSet::GetPrefixes returns an error.

Review of attachment 8446015 [details] [diff] [review]:
-----------------------------------------------------------------

It looks to me like the caller has exactly the same bug:
http://dxr.mozilla.org/mozilla-central/source/toolkit/components/url-classifier/LookupCache.cpp#699

This probably doesn't trigger right now because they're infallible arrays, but that tends to change. Can you fix that one as well?

Comment 4

[Andrew McCreight [:mccr8]]

Attachment: part 2 - LookupCache::GetPrefixes should free arr even on failure.

This is a little odd so I'm asking for review.

Comment 5

[Andrew McCreight [:mccr8]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/11dccfc5d6c3
https://hg.mozilla.org/integration/mozilla-inbound/rev/cc67c4e78d8d

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/11dccfc5d6c3
https://hg.mozilla.org/mozilla-central/rev/cc67c4e78d8d