1499020 - Crash in js::GetArrayBufferViewLengthAndData

Status: RESOLVED FIXED

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

Description

[[:philipp]]

This bug was filed from the Socorro interface and is
report bp-6d215931-804b-4879-9919-821120181012.
=============================================================

Top 10 frames of crashing thread:

0 xul.dll js::GetArrayBufferViewLengthAndData js/src/vm/ArrayBufferObject.cpp:2086
1 xul.dll mozilla::dom::TypedArray_base<unsigned char, &js::UnwrapArrayBufferView dom/bindings/TypedArray.h:142
2 xul.dll mozilla::dom::CryptoBuffer::Assign dom/crypto/CryptoBuffer.cpp:61
3 xul.dll mozilla::dom::AesTask::Init dom/crypto/WebCryptoTask.cpp:612
4 xul.dll mozilla::dom::AesTask::AesTask dom/crypto/WebCryptoTask.cpp:537
5 xul.dll mozilla::dom::WebCryptoTask::CreateEncryptDecryptTask dom/crypto/WebCryptoTask.cpp:3251
6 xul.dll mozilla::dom::SubtleCrypto::Decrypt dom/base/SubtleCrypto.cpp:69
7 xul.dll static bool mozilla::dom::SubtleCryptoBinding::decrypt dom/bindings/SubtleCryptoBinding.cpp:5575
8 xul.dll static bool mozilla::dom::SubtleCryptoBinding::decrypt_promiseWrapper dom/bindings/SubtleCryptoBinding.cpp:5589
9 xul.dll mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ConvertExceptionsToPromises> dom/bindings/BindingUtils.cpp:3260

=============================================================

crash reports with this signature started spiking up a few days ago on 2018-10-11 across firefox versions and platforms, so it'll likely be some server-side change that is triggering this crash now.

stack traces for those crashes commonly show that some webcrypto code is involved.
a number of comments in the crash reports are also referring to captchas on websites, so perhaps google's recaptcha service has added/changed some of its checks recently to weed out real-world machines from bots, and the webcrypto api is part of that.

curiously there's a high number of reports purportedly coming from firefox 49.0.2 and even 49.0.2esr(???).

Comment 1

[[:philipp]]

crash addresses from reports of older firefox versions indicate the issue might be sec-sensitive, so hiding the bug as a precaution...

Comment 2

[Ryan VanderMeulen [:RyanVM]]

All the recent crashes look safe. Not sure what the deal is with the 49.* ones, but I'm not sure this bug needs to be hidden anyway.

Comment 3

[Dana Keeler (she/her) (use needinfo) [:keeler]]

I have no idea how this is possible. The objects being operated on are either member-local or were specifically checked for being available. Maybe the problem is in JS somewhere?

Comment 4

[Christoph Kerschbaumer [:ckerschb]]

JC, can you please take a look or delegate if needed?

Comment 5

[Daniel Veditz [:dveditz]]

The newer-version crashes are a lot of null derefs and then crashing at "0xffffffffffffffff" which on Windows often means it got a pointer that wasn't valid. In the raw dump I see "rax": "0xbad0bad1b8fa9310" which looks like someone's pattern for "bad stuff is happening". Not sure if that's ours, a library, or the OS (I don't see "bad0bad1" in our source tree).

Comment 6

[J.C. Jones [:jcj] (he/they)]

All of these stack traces come down to the additional authenticated data being passed to AES-GCM, which is a BufferSource [1].

Buffer Sources are guaranteed not to be thread-safe, but it looks like the data is copied into a CryptoBuffer (using ReplaceElementsAt) at the start of the AES task.

So, like Dana in comment 3, I am at a loss.

I think to make progress here, we might need to fuzz WebCrypto AES-GCM using AADs. Particularly changing and/or invalid arrays.

I'll send an email to the Fuzzing team to ask.

[1] https://www.w3.org/TR/WebCryptoAPI/#aes-gcm-params

Comment 7

[Daniel Veditz [:dveditz]]

We don't need to hide this bug on behalf of possible vulnerabilities in ancient unsupported versions of Firefox. We already _know_ those versions are unsafe.

Comment 9

[Daniel Veditz [:dveditz]]

Surely this is a dupe that will be fixed by bug 1500759

Comment 11

[Julien Cristau [:jcristau]]

This seems to be gone in 64, which is consistent with comment 9.