Closed Bug 1458048 Opened 3 years ago Closed 2 years ago

Likely write beyond bounds in sctp_load_addresses_from_init()

Categories

(Core :: WebRTC: Networking, defect, P1)

59 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla62
Tracking Status
firefox-esr52 61+ fixed
firefox-esr60 61+ fixed
firefox60 --- wontfix
firefox61 + fixed
firefox62 + fixed

People

(Reporter: mozillabugs, Assigned: dminor)

References

Details

(Keywords: csectype-bounds, sec-high, Whiteboard: [adv-main61+][adv-esr60.1+][adv-esr52.9+])

Attachments

(2 files, 1 obsolete file)

sctp_load_addresses_from_init() (netwerk\sctp\src\netinet\sctp_pcb.c) is likely to write beyond bounds if the INIT argument it parses does not include an |SCTP_RANDOM| parameter. The argument comes from a (presumably untrusted) network source.

The bug is in the calculation of |keylen|. Lines 7740-44 calculate |keylen| and allocate |sizeof (sctp_key_t) + keylen| bytes. But if there is no |SCTP_RANDOM| parameter in the INIT, control skips lines 7562-82, and |p_random| is NULL (its initial value, set by line 7120) when control reaches line 7739:

7100: int
7101: sctp_load_addresses_from_init(struct sctp_tcb *stcb, struct mbuf *m,
7102:                               int offset, int limit,
7103:                               struct sockaddr *src, struct sockaddr *dst,
7104:                               struct sockaddr *altsa, uint16_t port)
7105: {
7106: 	/*
7107: 	 * grub through the INIT pulling addresses and loading them to the
7108: 	 * nets structure in the asoc. The from address in the mbuf should
7109: 	 * also be loaded (if it is not already). This routine can be called
7110: 	 * with either INIT or INIT-ACK's as long as the m points to the IP
7111: 	 * packet and the offset points to the beginning of the parameters.
7112: 	 */
...
7120:	struct sctp_auth_random *p_random = NULL;
7121:	uint16_t random_len = 0;
...
7131:	uint32_t keylen;
7132:	int got_random = 0, got_hmacs = 0, got_chklist = 0;
...
7237: 	/* now we must go through each of the params. */
7238: 	phdr = sctp_get_next_param(m, offset, &param_buf, sizeof(param_buf));
7239: 	while (phdr) {
7240: 		ptype = ntohs(phdr->param_type);
7241: 		plen = ntohs(phdr->param_length);
...
7562: 		} else if (ptype == SCTP_RANDOM) {
7563: 			if (plen > sizeof(random_store))
7564: 				break;
7565: 			if (got_random) {
7566: 				/* already processed a RANDOM */
7567: 				goto next_param;
7568: 			}
7569: 			phdr = sctp_get_next_param(m, offset,
7570: 						   (struct sctp_paramhdr *)random_store,
7571: 						   plen);
7572: 			if (phdr == NULL)
7573: 				return (-26);
7574: 			p_random = (struct sctp_auth_random *)phdr;
7575: 			random_len = plen - sizeof(*p_random);
7576: 			/* enforce the random length */
7577: 			if (random_len != SCTP_AUTH_RANDOM_SIZE_REQUIRED) {
7578: 				SCTPDBG(SCTP_DEBUG_AUTH1, "SCTP: invalid RANDOM len\n");
7579: 				return (-27);
7580: 			}
7581: 			got_random = 1;
7582:		} ...
...
7668: 	next_param:
7669: 		offset += SCTP_SIZE32(plen);
7670: 		if (offset >= limit) {
7671: 			break;
7672: 		}
7673: 		phdr = sctp_get_next_param(m, offset, &param_buf,
7674: 					   sizeof(param_buf));
7675: 	}
...

...This, in turn, means that line 7748...

7739: 	/* concatenate the full random key */
7740: 	keylen = sizeof(*p_random) + random_len + sizeof(*hmacs) + hmacs_len;
7741: 	if (chunks != NULL) {
7742: 		keylen += sizeof(*chunks) + num_chunks;
7743: 	}
7744: 	new_key = sctp_alloc_key(keylen);
7745: 	if (new_key != NULL) {
7746: 		/* copy in the RANDOM */
7747: 		if (p_random != NULL) {

...which resets |keylen| to properly index into |new_key->key|, will not be executed...

7748: 			keylen = sizeof(*p_random) + random_len;
7749: 			memcpy(new_key->key, p_random, keylen);
7750: 		}

And that means that lines 7752-61 use the old value of |keylen| for addressing the destinations of the memcpy()s on lines 7753-54 and 7759-60. But this value is actually the length, in bytes, of |new_key->key|, so both memcpy()s will write beyond bounds:

7751: 		/* append in the AUTH chunks */
7752: 		if (chunks != NULL) {
7753: 			memcpy(new_key->key + keylen, chunks,
7754: 			       sizeof(*chunks) + num_chunks);
7755: 			keylen += sizeof(*chunks) + num_chunks;
7756: 		}
7757: 		/* append in the HMACs */
7758: 		if (hmacs != NULL) {
7759: 			memcpy(new_key->key + keylen, hmacs,
7760: 			       sizeof(*hmacs) + hmacs_len);
7761: 		}
7762: 	} else {
7763: 		/* failed to get memory for the key */
7764: 		return (-34);
7765: 	}
...
7772: 	return (0);
7773: }

Nothing in the function prevents control from reaching lines 7740 et seq if a |SCTP_RANDOM| parameter is not present.

I'll try to produce a POC.
Additionally, in the same circumstances as above, the actual key buffer |new_key->key| is not set, and presumably contains either poison bytes or whatever data that the heap block previously contained, which results in a weak key.
Flags: sec-bounty?
Component: Networking → WebRTC: Networking
Nils: can you have someone look into this please
Group: core-security → media-core-security
Flags: needinfo?(drno)
Michael, Lennart: what do you think of this?
Flags: needinfo?(tuexen)
Flags: needinfo?(lennart.grahl)
Flags: needinfo?(drno)
Whiteboard: [need info tuexen,lgrahl 2018-05-03]
Will look into it...
Flags: needinfo?(lennart.grahl)
The only help I can offer at the moment is reminding Michael from time to time. :]
To see this bug in action, start FF, attach a debugger to it (remembering that if E10S is enabled, you might need to do this more than once!), and set a BP on line 7563, above. Now load https://webrtc.github.io/samples/src/content/datachannel/datatransfer . Click "Generate and send data".

When the BP fires, use the debugger to simulate a missing SCTP_RANDOM parameter by setting the next statement to line 7567 (the "goto next_param"). Set a BP on line 7744 (above) and proceed. When that BP fires, step line 7744-61 and watch lines 7753-54 copy 6 bytes of data beyond the end of the |new_key->key| buffer, and lines 7759-60 copy an additional 6 bytes beyond that. Also notice that no one initialized the actual |new_key->key| buffer itself.

It should be possible to cause more data to be copied beyond bounds by providing a longer SCTP chunk list (the sample uses only 2) and/or a longer HMAC list.
Confirmed in ASAN by disabling the if (got_random) check at line 7565 of sctp_pcb.c (made into if (1)).  With that change, ASAN triggers on this testcase.

sec-high on the assumption that a partner could start a connection without SCTP_RANDOM being set in fee->addrp.ph.param_type (see line 7472), which I'm pretty sure is the case.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OK, I can confirm that there is a problem if the peer sends no RANDOM parameter, but a HMAC-ALGO or a CHUNKS parameter. The problem is that the code writes outside the allocated memory.

However, in the case the RANDOM parameter is not supplied by the peer, the SCTP AUTH extension is not used (see line 7699):

	if ((stcb->asoc.auth_supported == 1) &&
	    ((peer_supports_auth == 0) ||
	     (got_random == 0) || (got_hmacs == 0))) {
		stcb->asoc.auth_supported = 0;
	}

So there is no problem in using weak key material. In addition to that, SCTP AUTH is not used within the WebRTC context.

Please note also, that if the peer enables or disabled SCTP AUTH and conforms to the specification, it will not send an HMAC-ALGO or CHUNKS parameter, but no RANDOM parameter. However, this does not mean that the peer can't send hand crafted packets to trigger this bug.

@mozillabugs What name should I use to acknowledge you in the commit message when fixing this for usrsctp and FreeBSD?
Flags: needinfo?(mozillabugs)
I'm planning to fix the issue by using:

diff --git a/usrsctplib/netinet/sctp_auth.c b/usrsctplib/netinet/sctp_auth.c
index 20805c5..0ca20c6 100755
--- a/usrsctplib/netinet/sctp_auth.c
+++ b/usrsctplib/netinet/sctp_auth.c
@@ -1530,6 +1530,8 @@ sctp_auth_get_cookie_params(struct sctp_tcb *stcb, struct mbuf *m,
                if (p_random != NULL) {
                        keylen = sizeof(*p_random) + random_len;
                        memcpy(new_key->key, p_random, keylen);
+               } else {
+                       keylen = 0;
                }
                /* append in the AUTH chunks */
                if (chunks != NULL) {
diff --git a/usrsctplib/netinet/sctp_pcb.c b/usrsctplib/netinet/sctp_pcb.c
index c834503..9860fa4 100755
--- a/usrsctplib/netinet/sctp_pcb.c
+++ b/usrsctplib/netinet/sctp_pcb.c
@@ -7747,6 +7747,8 @@ sctp_load_addresses_from_init(struct sctp_tcb *stcb, struct mbuf *m,
                if (p_random != NULL) {
                        keylen = sizeof(*p_random) + random_len;
                        memcpy(new_key->key, p_random, keylen);
+               } else {
+                       keylen = 0;
                }
                /* append in the AUTH chunks */
                if (chunks != NULL) {

Since this part of the code is up for a rewrite to address https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=224218, I prefer to keep the fix small and improve the handling of strange combinations of parameters in the rewrite.
Flags: needinfo?(tuexen)
(In reply to Michael Tüxen from comment #9)
> I'm planning to fix the issue by using:
> 
> diff --git a/usrsctplib/netinet/sctp_auth.c b/usrsctplib/netinet/sctp_auth.c
> index 20805c5..0ca20c6 100755
> --- a/usrsctplib/netinet/sctp_auth.c
> +++ b/usrsctplib/netinet/sctp_auth.c
> @@ -1530,6 +1530,8 @@ sctp_auth_get_cookie_params(struct sctp_tcb *stcb,
> struct mbuf *m,
>                 if (p_random != NULL) {
>                         keylen = sizeof(*p_random) + random_len;
>                         memcpy(new_key->key, p_random, keylen);
> +               } else {
> +                       keylen = 0;
>                 }
>                 /* append in the AUTH chunks */
>                 if (chunks != NULL) {

In this case

7740: 	keylen = sizeof(*p_random) + random_len + sizeof(*hmacs) + hmacs_len;

still allocates |sizeof(*p_random)| bytes for the random data, so won't the proposed fix misposition the remaining data? Maybe line 7740 shouldn't allocate |sizeof(*p_random)| bytes when there isn't any random data?

> @mozillabugs What name should I use to acknowledge you in the commit message when fixing this for usrsctp and FreeBSD?

Use "Ronald E. Crane".
Flags: needinfo?(mozillabugs)
I committed a fix upstream in https://github.com/sctplab/usrsctp/commit/8789a6da02e2c7c03522bc6f275b302f6ef977fe.

Thanks for reporting the issue. And you are correct, the stuff copied in the buffer is useless. But it is not used. Since I'll rewrite the code shortly to reduce the stack usage, I'll clean it up then.
We'll want to cherrypick the fix, or import an SCTP update if this is a "Good Point" to import.  Michael?
Flags: needinfo?(tuexen)
Flags: needinfo?(drno)
Should be OK.
Flags: needinfo?(tuexen)
Flags: needinfo?(drno)
Dan can I ask you to cherry pick the change from comment #11 and prepare that as a patch for this bug here please?
Assignee: nobody → dminor
Rank: 9
Flags: needinfo?(dminor)
Priority: -- → P1
Will do.
Flags: needinfo?(dminor)
Attachment #8986232 - Flags: review?(drno)
Comment on attachment 8986232 [details]
Bug 1458048 - Cherry pick usersctp rev 8789a6da02e2c7c03522bc6f275b302f6ef977fe; r=drno

Nils Ohlmeier [:drno] has approved the revision.

https://phabricator.services.mozilla.com/D1705
Attachment #8986232 - Flags: review+
Comment on attachment 8986232 [details]
Bug 1458048 - Cherry pick usersctp rev 8789a6da02e2c7c03522bc6f275b302f6ef977fe; r=drno

Manually resetting the r?, because there is already a r+ from me through phabricator
Attachment #8986232 - Flags: review?(drno)
Comment on attachment 8986232 [details]
Bug 1458048 - Cherry pick usersctp rev 8789a6da02e2c7c03522bc6f275b302f6ef977fe; r=drno

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
Not sure, but this patch has already landed upstream, so the cat is already out of the bag.

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

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 it looks like the patch should apply cleanly to all supported branches.

How likely is this patch to cause regressions; how much testing does it need?
Very unlikely.
Attachment #8986232 - Flags: sec-approval?
I believe we've already build the release candidate for Firefox 61 and associated ESR releases. 

Ryan, is there any way to get this into the release safely, if necessary?

Dan, Michael basically says that this isn't really a sec-high because the data copied into the buffer is not used. Is is ACTUALLY dangerous in any way? This looks more like a code cleanup of unused code. I'm looping in Ryan in case I'm wrong.

(In reply to Michael Tüxen from comment #11)

> I committed a fix upstream in
> https://github.com/sctplab/usrsctp/commit/
> 8789a6da02e2c7c03522bc6f275b302f6ef977fe.
> 
> Thanks for reporting the issue. And you are correct, the stuff copied in the
> buffer is useless. But it is not used. Since I'll rewrite the code shortly
> to reduce the stack usage, I'll clean it up then.
Flags: needinfo?(ryanvm)
Flags: needinfo?(dminor)
(In reply to Al Billings [:abillings] from comment #20)
> I believe we've already build the release candidate for Firefox 61 and
> associated ESR releases. 
> 
> Ryan, is there any way to get this into the release safely, if necessary?
> 
> Dan, Michael basically says that this isn't really a sec-high because the
> data copied into the buffer is not used. Is is ACTUALLY dangerous in any
> way? This looks more like a code cleanup of unused code. I'm looping in Ryan
> in case I'm wrong.
> 
> (In reply to Michael Tüxen from comment #11)
> 
> > I committed a fix upstream in
> > https://github.com/sctplab/usrsctp/commit/
> > 8789a6da02e2c7c03522bc6f275b302f6ef977fe.
> > 
> > Thanks for reporting the issue. And you are correct, the stuff copied in the
> > buffer is useless. But it is not used. Since I'll rewrite the code shortly
> > to reduce the stack usage, I'll clean it up then.

The buffer isn't used, but network-source data is still written beyond its end. https://bugzilla.mozilla.org/show_bug.cgi?id=1458048#c7
We *could* respin the RC builds if we really had to, but I don't have any drivers for doing so at this time unless people feel really strong about this one.
Flags: needinfo?(ryanvm)
An attacker can cause an arbitrary (more or less) length bounds-violation write with arbitrary attacker-supplied data from a network packet.

I'd take it and respin.  The patch is super-safe and obvious.

comment 6:
"It should be possible to cause more data to be copied beyond bounds by providing a longer SCTP chunk list (the sample uses only 2) and/or a longer HMAC list"
Flags: needinfo?(ryanvm)
Flags: needinfo?(abillings)
(In reply to mozillabugs from comment #21)
> The buffer isn't used, but network-source data is still written beyond its
> end. https://bugzilla.mozilla.org/show_bug.cgi?id=1458048#c7

Sorry I don't understand your reference to comment #7 from Randell here. Randell in that comment was only providing a way to reproduce the problem you reported. Are you trying to say that even with the patch applied ASAN still assert about this?
See comment #24 for my question.
Flags: needinfo?(mozillabugs)
I believe Ron is countering the claim in comment 20 that this isn't sec-high because "the data ... is not used". Maybe it's not, but the vulnerability was from writing it outside the buffer and comment 7 shows that does happen.

I don't know how closely the upstream here is watched but "Don't overflow a buffer if..." is a pretty clear red flag if it is, and the comment points right to Firefox as a downstream user. We should take this on all the branches.
I'm checking with ASAN if I can trigger a failure with michael's fix.  

Michael: what's your response to comment 20?

Also: we should alert the chrome team if tuexen hasn't already.
Flags: needinfo?(tuexen)
Comment on attachment 8986232 [details]
Bug 1458048 - Cherry pick usersctp rev 8789a6da02e2c7c03522bc6f275b302f6ef977fe; r=drno

Approval Request Comment
[Feature/Bug causing the regression]: N/A
[User impact if declined]:
   vulnerability published upstream with reference to mozilla
[Is this code covered by automated tests?]:
   generally yes, but not this condition or it would have been caught before
[Has the fix been verified in Nightly?]:
   not yet. upstream though yes.
[Needs manual test from QE? If yes, steps to reproduce]: 
[List of other uplifts needed for the feature/fix]: N/A
[Is the change risky?]: No
[Why is the change risky/not risky?]: Fixes a length value

sec-approval=dveditz
[String changes made/needed]:

Approval Request Comment
[Feature/Bug causing the regression]:
[User impact if declined]:
[Is this code covered by automated tests?]:
[Has the fix been verified in Nightly?]:
[Needs manual test from QE? If yes, steps to reproduce]: 
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]:
[Why is the change risky/not risky?]:
[String changes made/needed]:

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: 
Fix Landed on Version:
Risk to taking this patch (and alternatives if risky): 
String or UUID changes made by this patch: 

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Attachment #8986232 - Flags: sec-approval?
Attachment #8986232 - Flags: sec-approval+
Attachment #8986232 - Flags: approval-mozilla-release?
Attachment #8986232 - Flags: approval-mozilla-esr60?
Attachment #8986232 - Flags: approval-mozilla-esr52?
Attachment #8986232 - Flags: approval-mozilla-beta?
I have tested with the upstream fix and the hack to ignore RANDOMs to trigger this (as described in comment 6, but in code instead of debugger - I never trust changing the execution line in a debugger).  Same test I used to verify the report.

It does *not* fail with the upstream patch.  Note this checks the sctp_pcb.c path; it doesn't directly the sctp_auth.c flow since we don't normally trigger that in webrtc.  The code is basically the same, so I believe it is also fixed.

I believe comment 20 by mozillabugs is referring to *without* the patch it overwrites; he's responding to Al's comment.
(In reply to Randell Jesup [:jesup] from comment #27)
>
> Also: we should alert the chrome team if tuexen hasn't already.

Crap, if they don't know and affected, I need to embargo the advisory on this so who can communicate with them? Dan?
Flags: needinfo?(abillings)
Comment on attachment 8986232 [details]
Bug 1458048 - Cherry pick usersctp rev 8789a6da02e2c7c03522bc6f275b302f6ef977fe; r=drno

Approved for Fx61 RC3 and ESR 60.1 RC2. Needs a rebased patch for ESR52.
Flags: needinfo?(rjesup)
Attachment #8986232 - Flags: approval-mozilla-release?
Attachment #8986232 - Flags: approval-mozilla-release+
Attachment #8986232 - Flags: approval-mozilla-esr60?
Attachment #8986232 - Flags: approval-mozilla-esr60+
Attachment #8986232 - Flags: approval-mozilla-esr52?
Attachment #8986232 - Flags: approval-mozilla-beta?
(In reply to Al Billings [:abillings] from comment #31)
> (In reply to Randell Jesup [:jesup] from comment #27)
> >
> > Also: we should alert the chrome team if tuexen hasn't already.
> 
> Crap, if they don't know and affected, I need to embargo the advisory on
> this so who can communicate with them? Dan?

According to this https://chromium.googlesource.com/external/github.com/sctplab/usrsctp/+/master/usrsctplib/netinet/sctp_auth.c#1533 and https://chromium.googlesource.com/external/github.com/sctplab/usrsctp/+/master/usrsctplib/netinet/sctp_pcb.c#7750 they have the patch in their repro. Not sure how to figure out in which Chrome versions it's included (but I guess that is not our problem to solve).
> According to this
> they have the patch in their
> repro. Not sure how to figure out in which Chrome versions it's included
> (but I guess that is not our problem to solve).

Landed on their trunk on 6/2 (18 days ago) by Michael Tuexen, the same day he committed the fix upstream (I believe) and posted comment 11 here.
This is the same patch for ESR52
Attachment #8986656 - Flags: review+
Assignee: dminor → rjesup
Status: NEW → ASSIGNED
Attachment #8986657 - Attachment is obsolete: true
Flags: needinfo?(rjesup)
(In reply to Nils Ohlmeier [:drno] from comment #33)

> According to this
> https://chromium.googlesource.com/external/github.com/sctplab/usrsctp/+/
> master/usrsctplib/netinet/sctp_auth.c#1533 and
> https://chromium.googlesource.com/external/github.com/sctplab/usrsctp/+/
> master/usrsctplib/netinet/sctp_pcb.c#7750 they have the patch in their
> repro. Not sure how to figure out in which Chrome versions it's included
> (but I guess that is not our problem to solve).

Dan wrote to them and they replied thanking us so they're aware of the bug now.
Attachment #8986657 - Flags: approval-mozilla-esr52+
Comment on attachment 8986657 [details] [diff] [review]
Cherry pick usersctp rev 8789a6da02e2c7c03522bc6f275b302f6ef977fe

woops picked the wrong attachment
Attachment #8986657 - Flags: approval-mozilla-esr52+
Attachment #8986656 - Flags: approval-mozilla-esr52+
Assignee: rjesup → dminor
The problem is that a peer can force data to be written out of a buffer on the heap.

My statement regarding the data written is not used is meant that it is not used
for SCTP AUTH. For WebRTC this extension isn't used at all, so that it not an
issue.

How critical it is to put something on the heap outside the buffer bounds is
something hard to say for me.

But you need a malicious peer, this can't happen by accident.
Flags: needinfo?(tuexen)
It might make sense to disable all SCTP extensions not used by WebRTC... Should
I provide a patch for that?
https://hg.mozilla.org/mozilla-central/rev/492697e43fb4
Group: media-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Flags: needinfo?(dminor)
(In reply to Michael Tüxen from comment #39)
> The problem is that a peer can force data to be written out of a buffer on
> the heap.
> 
> My statement regarding the data written is not used is meant that it is not
> used
> for SCTP AUTH. For WebRTC this extension isn't used at all, so that it not an
> issue.
> 
> How critical it is to put something on the heap outside the buffer bounds is
> something hard to say for me.
> 
> But you need a malicious peer, this can't happen by accident.

We assume a malicious peer.

ASAN will catch bounds violations.  With your patch, I see none in the sctp_pcb.c case (the flow we hit), so at that level, this fix does what it's intended to do.  I don't know how to force the sctp_auth.c flow and trigger the instance of this bug you patched there, so I can't easily test that (probably have to simulate receiving auth/hmac packets?  I could put test code in to force that to verify the patch, if you can tell me what they should look like.)

Writing random data into a random-sized allocation on the heap is fine and no sec issue, so long as it does not extend outside the allocation.  Is there a way for an attacker (malicious peer) to fore that with your patch?  If so, then this is not (fully) fixed.  I don't see how that can happen in the sctp_pcb.c case.

And yes, it likely makes sense to disable extensions that aren't part of WebRTC/datachannels.
Flags: needinfo?(tuexen)
For testing this you need to handcraft packets... I'm using packetdrill for testing the kernel, but this does not support the AUTH extensions yet...

With the patch, you are not writing outside the allocation.

Let me setup a build environment for FF and I can provide a patch for disabling unused extensions.
Flags: needinfo?(tuexen)
Ok, if we aren't writing outside of the allocation, all is good for release. :-)

I think disabling unused extensions is a useful "belt and suspenders" fix - if they're disabled, they can't be leveraged into a vulnerability.  We can open another bug for that one (and it doesn't need to be a sec bug).
I agree, disabling unused extensions should be handled separately.
(In reply to Nils Ohlmeier [:drno] from comment #25)
> See comment #24 for my question....Sorry I don't understand your reference to comment #7 from Randell here. Randell in that comment was only providing a way to reproduce the problem you reported. Are you trying to say that even with the patch applied ASAN still assert about this?

No. I was noting that, *without* the patch, data is written beyond the end of the buffer, even though the buffer's contents aren't used for anything.
Flags: needinfo?(mozillabugs)
Whiteboard: [need info tuexen,lgrahl 2018-05-03] → [need info tuexen,lgrahl 2018-05-03][adv-main61+][adv-esr60.1+][adv-esr52.9+]
Whiteboard: [need info tuexen,lgrahl 2018-05-03][adv-main61+][adv-esr60.1+][adv-esr52.9+] → [adv-main61+][adv-esr60.1+][adv-esr52.9+]
Flags: qe-verify-
Flags: sec-bounty? → sec-bounty+
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.