1149542 - (CVE-2015-2710) Heap-buffer-overflow in SVGTextFrame::ResolvePositions

Status: VERIFIED FIXED

(Core :: SVG, defect)

Description

[Atte Kettunen]

Attachment: repro-file.svg

Tested on:

OS: Ubuntu 14.04

Firefox: ASAN build from https://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-linux64-asan/1427796124/


Note:

Repro-file will also crash stable Firefox 36.0.4 on Ubuntu 14.04

Crash report-ID: bp-6e47afc4-a88c-4cf3-a591-0a0552150331

ASAN-trace:

==1594==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x621000e67503 at pc 0x7fdce471f097 bp 0x7fffec35a250 sp 0x7fffec35a248
WRITE of size 1 at 0x621000e67503 thread T0 (Web Content)
    #0 0x7fdce471f096 in SVGTextFrame::ResolvePositions(nsIContent*, unsigned int, bool, bool&, nsTArray<gfxPoint>&) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/layout/svg/SVGTextFrame.cpp:4403
    #1 0x7fdce471ee90 in SVGTextFrame::ResolvePositions(nsIContent*, unsigned int, bool, bool&, nsTArray<gfxPoint>&) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/layout/svg/SVGTextFrame.cpp:4519
    #2 0x7fdce47203b5 in SVGTextFrame::ResolvePositions(nsTArray<gfxPoint>&, bool) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/layout/svg/SVGTextFrame.cpp:4564
    #3 0x7fdce4726769 in SVGTextFrame::DoGlyphPositioning() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/layout/svg/SVGTextFrame.cpp:5017
    #4 0x7fdce4715179 in UpdateGlyphPositioning /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/layout/svg/SVGTextFrame.cpp:5187
    #5 0x7fdce4715179 in SVGTextFrame::ReflowSVG() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/layout/svg/SVGTextFrame.cpp:3819
    #6 0x7fdce471759b in nsSVGDisplayContainerFrame::ReflowSVG() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/layout/svg/nsSVGContainerFrame.cpp:356
    #7 0x7fdce477cdbe in nsSVGOuterSVGFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/layout/svg/nsSVGOuterSVGFrame.cpp:437
.
.
.
0x621000e67503 is located 3 bytes to the right of 4096-byte region [0x621000e66500,0x621000e67500)
allocated by thread T0 (Web Content) here:
    #0 0x474c1b in realloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:95
    #1 0x49213d in moz_xrealloc /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/memory/mozalloc/mozalloc.cpp:121
    #2 0x7fdcde6d1adf in Realloc /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/obj-firefox/media/libstagefright/../../dist/include/nsTArray.h:185
    #3 0x7fdcde6d1adf in nsTArray_base<nsTArrayInfallibleAllocator, nsTArray_CopyWithMemutils>::EnsureCapacity(unsigned long, unsigned long) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/obj-firefox/media/libstagefright/../../dist/include/nsTArray-inl.h:182
    #4 0x7fdce471fe2a in AppendElement<mozilla::CharPosition> /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/obj-firefox/layout/svg/../../dist/include/nsTArray.h:1353
    #5 0x7fdce471fe2a in SVGTextFrame::ResolvePositions(nsTArray<gfxPoint>&, bool) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/layout/svg/SVGTextFrame.cpp:4555
    #6 0x7fdce4726769 in SVGTextFrame::DoGlyphPositioning() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/layout/svg/SVGTextFrame.cpp:5017
    #7 0x7fdce4715179 in UpdateGlyphPositioning /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/layout/svg/SVGTextFrame.cpp:5187
    #8 0x7fdce4715179 in SVGTextFrame::ReflowSVG() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/layout/svg/SVGTextFrame.cpp:3819
.
.
.

Comment 1

[Daniel Veditz [:dveditz]]

Potentially critical, but should re-evaluate when we know what the actual bug is.

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: testcase 2 (reduced)

Comment 3

[Daniel Holbert [:dholbert]]

SVG text bug --> heycam, can you take a look? We're walking off the end of the "mPositions" nsTArray in SVGTextFrame::ResolvePositions.

You don't need an ASAN build to reproduce this -- it triggers a fatal assertion (for out-of-bounds nsTArray access) in debug builds, too. (verified locally with testcase 2)

Comment 4

[Daniel Holbert [:dholbert]]

Specifically, we write off the end of the array here:
>        if (aInTextPath || ShouldStartRunAtIndex(mPositions, aDeltas, aIndex)) {
>          mPositions[aIndex].mRunBoundary = true;
>        }
https://mxr.mozilla.org/mozilla-central/source/layout/svg/SVGTextFrame.cpp?rev=41c005e9398e&mark=4403-4403#4380

Here, mPositions->mHdr.mLength is 7, and aIndex is 7 as well.

Comment 5

[Cameron McCormack (:heycam)]

Attachment: test case 2 reduced further

Thanks for reducing the test case, Daniel.  Here it is reduced slightly more.

Comment 6

[Cameron McCormack (:heycam)]

This is what the anonymous block child of the SVGTextFrame looks like:

  line 2aaad7211e58: count=1 state=inline,clean,prevmarginclean,not impacted,not wrapped,before:nobr,after:linebr[0x8100] {0,0,0,1140} {0,0,0,1140;cw=600} <
    Text(0)"\n"@2aaad7211a80 next=2aaad7211d30 {0,0,0,1140} [state=40008000a0600000] [content=2aaad598df70] [sc=2aaad72115b0:-moz-non-element^2aaad7210d58^2aaad72101e0^0] [run=2aaad720f280][0,1,T] 
  >
  line 2aaad7211f48: count=2 state=inline,clean,prevmarginclean,not impacted,wrapped,before:nobr,after:nobr[0x120] {0,1140,600,1140} {0,1140,600,1140;cw=600} <
    Letter(text)(3)@2aaad7211d30 next=2aaad722c758 next-in-flow=2aaad722c758 {0,1140,0,1140} [state=0000800000000000] [content=2aaacd0cd030] [sc=2aaad7211b60:first-letter^2aaad7210d58^2aaad72101e0^0]<
      Text(2)""@2aaad7211cc0 next-in-flow=2aaad722c6d0 {0,900,0,0} [state=4000800020200000] [content=2aaad7212110] [sc=2aaad7211c10:-moz-non-element] [run=2aaad720f700][0,0,F] 
    >
    Letter(text)(3)@2aaad722c758 prev-in-flow=2aaad7211d30 {0,1140,600,1140} [state=0000800000000004] [content=2aaacd0cd030] [sc=2aaad72115b0:-moz-non-element^2aaad7210d58^2aaad72101e0^0]<
      Text(2)"b"@2aaad722c6d0 prev-in-flow=2aaad7211cc0 {0,0,600,1140} [state=0000800080600004] [content=2aaad7212110] [sc=2aaad72115b0:-moz-non-element^2aaad7210d58^2aaad72101e0^0] [run=2aaad720f700][0,1,T] 
    >
  >
>

Comment 7

[Cameron McCormack (:heycam)]

Missed the first line of that:

Block(text)(3)@2aaad7211448 {0,0,600,2280} [state=8000900020d00000] [content=2aaacd0cd030] [sc=2aaad7211178:-moz-svg-text]<

Comment 8

[Cameron McCormack (:heycam)]

DetermineCharPositions should generate an array whose length is the number of descendant characters in the DOM, regardless of display:none.  It's returning an array of length 2 but it should be 3.  If I remove the text::first-letter rule, then I do get a 3 element array.

The comment for TextFrameIterator, which DetermineCharPositions uses, says

 * Note that any text frames that are empty -- whose ContentLength() is 0 --
 * will be skipped over.

However, there is a recorded "undisplayed characters before frame" value on the empty text frame (1, for the undisplayed 'a').  We're not getting this value in the TextFrameIterator loop in DetermineCharPositions because we completely skip over the empty text frame.

So either we need to return that empty text frame from the iterator, or the TextNodeCorrespondenceRecorder needs to skip over empty text frames too.

Comment 9

[Cameron McCormack (:heycam)]

The skipping of empty text frames was done in bug 841174.

Comment 10

[Cameron McCormack (:heycam)]

I think GetNonEmptyTextFrameAndNode needs to be updated to do the same test (checking the frame's content length, rather than the node's text length).

Comment 11

[Cameron McCormack (:heycam)]

(In reply to Daniel Veditz [:dveditz] from comment #1)
> Potentially critical, but should re-evaluate when we know what the actual
> bug is.

Critical.  We can control how far off the end of the array we write by having any number of display:none single-character <tspan> children which have a ::first-letter applying to them.  The kinds of things written include for example the values specified in the x="" and y="" attributes on the <text>.

Comment 12

[Cameron McCormack (:heycam)]

Attachment: Return early if we discover mPositions is not long enough.

Here is a protection against this and similar bugs where our mPositions is out of sync with the characters in the DOM.

Comment 13

[Cameron McCormack (:heycam)]

Attachment: Track undisplayed characters before empty text frames properly.

And a fix for the actual problem here.

Comment 14

[Daniel Holbert [:dholbert]]

Comment on attachment 8587158 [details] [diff] [review]
Return early if we discover mPositions is not long enough.

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

::: layout/svg/SVGTextFrame.cpp
@@ +4389,5 @@
>      if (length) {
> +      uint32_t end = aIndex + length;
> +      if (end > mPositions.Length()) {
> +        MOZ_ASSERT(false, "length of mPositions does not match characters "
> +                          "found by iterating content");

This is what MOZ_ASSERT_UNREACHABLE(...) is for -- use that instead of MOZ_ASSERT(false,...)

(I think there are 3 of these in the patch.)

::: layout/svg/SVGTextFrame.h
@@ +520,2 @@
>     * @param aInTextPath Whether we are currently under a <textPath> element.
>     * @param aForceStartOfChunk Whether the next character we find should start a

aForceStartOfChunk is in/out as well -- mark it as-such in the documentation, now that you're marking aIndex as such, or else it's implied that it's just an in-param.

Also: It looks like "aDeltas" is missing from the documentation here, and it's an in/out param as well. Could you add documentation for it?

@@ +527,1 @@
>                              bool aInTextPath, bool& aForceStartOfChunk,

Hmm -- it seems a bit confusing & error-prone to have two functions with the same name, one of which returns nsresult and the other of which returns bool.  Someone could easily call the nsresult one and check its return-value as if it were bool, by mistake ("if (ResolvePositions(...))"), and get the logic backwards as a result, since NS_OK == 0 == false.

So, it seems like being consistent about a "bool" return type would be safer here, from the perspective of getting logic correct. (and being able to sanity-check the logic without thinking too hard)

(I also think nsresult is an overly complex return type, for a function that just wants to return a basic success/failure indication & doesn't need any NS_ENSURE_SUCCESS() error-type-propagation in its caller.)

Even so, the bool return values would mean slightly different (but overlapping) things; which would also be a bit confusing; maybe that's why you chose not to use bool here.  But I think that'd be clearer if these two functions had slightly different names, so they were easier to distinguish -- maybe the helper-function should be called ResolvePositionsForNode(), since it takes a content-node and recursively processes that node?

So, tl;dr: I think both ResolvePositions funcs should return bool, and (perhaps in a separate bug) consider renaming the first one to something like ResolvePositionsForNode()

Comment 15

[Daniel Holbert [:dholbert]]

Comment on attachment 8587160 [details] [diff] [review]
Track undisplayed characters before empty text frames properly.

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

r=me with the following:

::: layout/svg/SVGTextFrame.cpp
@@ +297,5 @@
>    NS_ASSERTION(content && content->IsNodeOfType(nsINode::eTEXT),
>                 "unexpected content type for nsTextFrame");
>  
>    nsTextNode* node = static_cast<nsTextNode*>(content);
> +  MOZ_ASSERT(node->TextLength() != 0);

Two suggestions:
 (1) MOZ_ASSERTs without messages are a bit mysterious, unless it's dead-obvious why they must hold. There's some subtlety here (as shown by the existence of this bug), so I think this would benefit from an assertion-message -- e.g. "GetContentLength() should've been nonzero, if our node has text". (Not sure this is exactly true / what you're going for; tweak as-appropriate.)

 (2) It seems to be crucial that we return true here in exactly the same conditions that IsNonEmptyTextFrame returns true. (That's what's behind the TextFrameIterator behavior you mentioned in comment 8.) So, let's add an assertion to check this -- e.g.:
  MOZ_ASSERT(IsNonEmptyTextFrame(aFrame),
             "Our logic should agree with IsNonEmptyTextFrame");

(Maybe even worth restructuring this function slightly, so we can check this agreement for the return true *and* return false cases, right when the function ends.  e.g. instead of 'return false' / 'return true', set a local "bool isNonEmptyTF" to true or false, with a single final "return isNonEmptyTF" statement; and just before that return statement, we can assert that isNonEmptyTF == IsNonEmptyTextFrame(aFrame). Maybe that's overkill though.)

::: layout/svg/crashtests/1149542-1.svg
@@ +1,1 @@
> +<svg xmlns="http://www.w3.org/2000/svg">

(The crashtest should be split into its own patch, to land separately after we've shipped the fix to all affected branches & are ready to open the bug.)

Comment 16

[Cameron McCormack (:heycam)]

Attachment: Part 1: Return early if we discover mPositions is not long enough. (v2)

All your comments on this patch make sense.

Comment 17

[Cameron McCormack (:heycam)]

Attachment: Part 2: Track undisplayed characters before empty text frames properly. (v2) r=dholbert

Comment 18

[Cameron McCormack (:heycam)]

Attachment: Part 3: Crashtest. r=dholbert [DO NOT LAND until other patches have made it to all supported releases]

Comment 19

[Daniel Holbert [:dholbert]]

Comment on attachment 8587738 [details] [diff] [review]
Part 1: Return early if we discover mPositions is not long enough. (v2)

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

r=me with two nits:

First, I'd expand the commit message slightly:
> Bug 1149542 - Return early if we discover mPositions is not long enough.

Maybe expand to "Return early from SVG text layout if [...]". Otherwise, it's unclear what region of code is actually changing.

::: layout/svg/SVGTextFrame.cpp
@@ +4387,5 @@
>      // We found a text node.
>      uint32_t length = static_cast<nsTextNode*>(aContent)->TextLength();
>      if (length) {
> +      uint32_t end = aIndex + length;
> +      if (end > mPositions.Length()) {

This end > mPositions.Length() check should probably be wrapped in "MOZ_UNLIKELY(...)", as a branch-prediction hint, since we know up-front that it should never actually pass. (and if it does, it's a bug)

Same with the "if" checks for the other unexpected-early-returns that you're adding in this function.

(And you'll need an #include "mozilla/Likely.h" in order to use that macro.)

Comment 20

[Cameron McCormack (:heycam)]

Attachment: Part 1: Return early from SVG text layout if we discover mPositions is not long enough. (v3) r=dholbert

Comment 21

[Cameron McCormack (:heycam)]

Comment on attachment 8587761 [details] [diff] [review]
Part 3: Crashtest. r=dholbert [DO NOT LAND until other patches have made it to all supported releases]

(This is for all three patches.)

[Security approval request comment]
How easily could an exploit be constructed based on the patch?

If we just consider the first patch, then it's not clear from the code changes where the bug lies.  Looking at the second patch, it's conceivable that someone could notice that this is enforcing the expectations around zero-length text frames, look at bug 841174, and start looking for a similar problem.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

The test, yes, as you could easily open the test in a debug build and discover the out-of-bounds array writing.

Which older supported branches are affected by this flaw?

All.

If not all supported branches, which bug introduced the flaw?

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

No, but this code hasn't changed much recently and ought to apply easily.

How likely is this patch to cause regressions; how much testing does it need?

Unlikely; ::first-letter applying to SVG text content is relatively new in the SVG 2 specification and other browsers don't implement it yet, so it is quite unlikely anyone is relying on it.

Comment 22

[Daniel Holbert [:dholbert]]

(heycam: I suspect you meant to request sec-approval on the fixes, not the crashtest?)

> Do comments in the patch, the check-in comment, or tests included in the patch
> paint a bulls-eye on the security problem?
>
> The test, yes, [...]

(The test can be ignored, from a sec-approval perspective, since it won't be included in the patch. It won't land [I hope] until the fix(es) have made it to all affected users.)

Comment 23

[Cameron McCormack (:heycam)]

Yeah, I meant to choose the first patch.  Although my comments were about all three patches.

In terms of what we actually want to land first, it could either be just the first patch or both the first and second patches simultaenously (which is what I'd prefer).

Comment 24

[Al Billings [:abillings - ex-MoCo]]

Comment on attachment 8587786 [details] [diff] [review]
Part 1: Return early from SVG text layout if we discover mPositions is not long enough. (v3) r=dholbert

sec-approval+

Comment 25

[Cameron McCormack (:heycam)]

Thanks Al.  Just to make it clear, does the sec-approval+ apply to both the part 1 "avoid this kind of bug generally" patch and the part 2 "fix this specific issue" patch?

Comment 26

[Al Billings [:abillings - ex-MoCo]]

The sec-approval+ applies to whatever you were planning to check into trunk, which I assume was parts 1 and 2.

Comment 27

[Cameron McCormack (:heycam)]

Great, cheers.

Comment 28

[Cameron McCormack (:heycam)]

Sheriffs, can you help land this for me?  I don't know the exact set of trees to check out for the tracking flags set, and I want to be sure that they all land at once.  Please land only parts 1 and 2.  Thanks!

Comment 29

[Ryan VanderMeulen [:RyanVM]]

It still needs Aurora/Beta/esr31 approval independent of the sec-approval. Get that and we can handle the rest :)

Comment 30

[Daniel Holbert [:dholbert]]

You may want to just go ahead and push to inbound, even without aurora/beta approval.  Given that we're not shipping a firedrill release for this, it doesn't really matter if we land this on inbound first or land it on all branches simultaneously, because our release users will still be vulnerable until the next merge.

> Please land only parts 1 and 2.  Thanks!

When you're relying on someone else doing the landing, I'd suggest putting "DO NOT LAND" in the patch description for extremely-important-that-they-not-land-yet patches, to make sure this isn't missed. :)

Comment 31

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b08005b5d090
https://hg.mozilla.org/integration/mozilla-inbound/rev/d084a35e8d79

Needs aurora/beta/esr31 approvals ASAP if we want this to make 38b2 today.

Comment 32

[Daniel Holbert [:dholbert]]

Comment on attachment 8587786 [details] [diff] [review]
Part 1: Return early from SVG text layout if we discover mPositions is not long enough. (v3) r=dholbert

Requesting aurora/beta approval on heycam's behalf, since I think he's out until tomorrow. (and we'd benefit a bit from getting this into aurora/beta today, per comment 31)

Approval Request Comment
[Feature/regressing bug #]: SVG Text; bug 841174. (Affects all branches)

[User impact if declined]: potential security exploit; see comment 11.

[Describe test coverage new/current, TreeHerder]: We have a regression test (attached) which we can land once users are patched. We have existing testcases for SVG text rendering; I'm not sure how comprehensive they are (heycam could comment more on this), but this is low-risk enough that I'm not concerned.

[Risks and why]: Low-risk from a security perspective -- "part 1" adds explicit length checks to prevent other out-of-bounds reads of this type.  There's a small risk that this could break some SVG text edge-case that we didn't consider here, but I'd be surprised, because the patches are pretty targeted at making us more consistent & making sure we don't read past the end of an array.

[String/UUID change made/needed]: None

Comment 33

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8587786 [details] [diff] [review]
Part 1: Return early from SVG text layout if we discover mPositions is not long enough. (v3) r=dholbert

As it is critical, taking it for esr31 too.

Comment 34

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8587760 [details] [diff] [review]
Part 2: Track undisplayed characters before empty text frames properly. (v2) r=dholbert

Taking this one too.

Comment 35

[Daniel Holbert [:dholbert]]

Thanks! Yeah, I should've requested ESR31, too - thanks for adding that.

I think RyanVM's on top of landings for this; needinfo'ing him to be sure.

Comment 36

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/d299f9b81369
https://hg.mozilla.org/releases/mozilla-aurora/rev/bd79c06527cc

Comment 37

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/984f9cdef799
https://hg.mozilla.org/releases/mozilla-beta/rev/32d78bac2cfa

Comment 38

[Cameron McCormack (:heycam)]

Thanks Daniel and Ryan.

Comment 39

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b08005b5d090
https://hg.mozilla.org/mozilla-central/rev/d084a35e8d79

Comment 40

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/e27a6b9e1639
https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/2b5788d96549

https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/e73d44ebb7c4
https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/9b0cebf723b9

https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/41f7a1610d69
https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/a782e16d271f

https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/db399250e2e7
https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/d345a7899664

https://hg.mozilla.org/releases/mozilla-esr31/rev/c550eaa42df3
https://hg.mozilla.org/releases/mozilla-esr31/rev/c349d2691fc4

Comment 41

[Ryan VanderMeulen [:RyanVM]]

Bustage follow-up for Gecko <32:
https://hg.mozilla.org/releases/mozilla-esr31/rev/53ac54c69671
https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/68066b1f8694

Comment 42

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/e73d44ebb7c4
https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/9b0cebf723b9

https://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/41f7a1610d69
https://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/a782e16d271f

Comment 43

[Cameron McCormack (:heycam)]

I will land the test on May 13.

Comment 44

[Daniel Holbert [:dholbert]]

FWIW, mats asserted the following about tests on sec bugs, in bug 1134667 comment 22:

> Standard procedure is that we land testcases *after* the bug has been made
> public.
> (This is because we want to reveal as little as possible about the bug to our
> adversaries who we can assume is watching everything we do in Bugzilla and
> our repositories.)
> 
> You don't need to track that yourself, just mark the bug in-testsuite? and
> attach a patch with the test(s) and someone will land that for you when the
> time comes.

So if May 13 comes and for some reason we haven't made this bug public yet, probably best not to land the test (but maybe worth asking at that point if we're good to open the bug).

Comment 45

[Cameron McCormack (:heycam)]

(In reply to Daniel Holbert [:dholbert] from comment #44)
> So if May 13 comes and for some reason we haven't made this bug public yet,
> probably best not to land the test (but maybe worth asking at that point if
> we're good to open the bug).

OK thanks, will do.

Comment 46

[Atte Kettunen]

Should this be flagged with sec-bounty?

Comment 47

[Al Billings [:abillings - ex-MoCo]]

(In reply to Atte Kettunen from comment #46)
> Should this be flagged with sec-bounty?

It can be and I just did. We don't do it by default. We need to be asked on specific bugs, usually by email to [e-mail].

Comment 48

[Ada [:adalucinet]]

Reproduced the crash on Firefox 36.0.4 and the assertion on a Nightly debug build (from 2015-03-31) by using the attached TC:
'Assertion failure: aIndex < Length() (invalid array index), at ../../dist/include/nsTArray.h:941' 
Verified as fixed with ESR 31.7.0 build 2 (Build ID: 20150504194141), latest Nightly debug & stable (2015-05-04), under Ubuntu 14.04 x64.

Comment 49

[Ada [:adalucinet]]

Also verified as fixed with ESR 38.0 (Build ID: 20150505103531) under Ubuntu 13.10 64-bit.

Comment 50

[Atte Kettunen]

Is the crashtest for this bug already public?

Comment 51

[Al Billings [:abillings - ex-MoCo]]

(In reply to Atte Kettunen from comment #50)
> Is the crashtest for this bug already public?

I don't think it was checked in yet. Cameron?

Comment 52

[Cameron McCormack (:heycam)]

Not it didn't get checked in.  I'll do that now.

Comment 53

[Cameron McCormack (:heycam)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d263ff7885e9

Comment 54

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/d263ff7885e9