Bug 219434

Summary: [WebAuthn] Crash of the browser when rp.icon is too long and device is Yubikey (overflow?)
Product: WebKit Reporter: Greg ORIOL <fortin81>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: NEW ---    
Severity: Major CC: ap, bfulgham, jiewen_tan, loginllama, smoley, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Safari 14   
Hardware: Mac   
OS: macOS 10.15   
Bug Depends on:    
Bug Blocks: 181943    
Description Flags
Data to pass to navigator.credentials.create to reproduce
Crashlog from Safari 14
Crashlog from Safari Technology Preview 14.1 none

Description Greg ORIOL 2020-12-02 09:28:53 PST
Created attachment 415228 [details]
Data to pass to navigator.credentials.create to reproduce


I'm not sure of the implications of this issue, but I believe someone might be interested to have a look as this js webauthn call generates a crash on most current browsers.

The problem seems to happen with navigator.credentials.create(data) when providing the data with an rp.icon containing more than 8kb of base64 encoded png and using a Yubikey USB device (model "5 NFC" here), the browsers just crash.

An example data to reproduce, with rp.icon being of 9kb, is attached to this bug, I encoded it with JSON.stringify to save it.

It happens on Safari 14.0 (15610., 15610), Safari Technology Preview 116 (and also Chrome 87.0.4280.67, but not Firefox 83.0) on macOS Catalina 10.15.7.

If the USB device is not connected, the browsers don't crash.

I don't have any other USB webauthn device to check if this is specific to Yubikeys, but when using SoftU2F the call doesn't make the browsers crash.
I also don't have a windows or linux to check if this is specific to macOS.
Comment 1 Alexey Proskuryakov 2020-12-02 19:25:38 PST
This isn't trivial enough to reproduce (navigator.credentials doesn't seem to be available in Web Inspector). Could you please attach a complete crash log?
Comment 2 Greg ORIOL 2020-12-03 00:39:09 PST
You are right, it's a little bit tricky (needs https+button), so here is a working demonstration:
https://gregoriol.github.io/webkit-bug-219434 (with all code in the source of the html)

And I'm also attaching a crashlog from Safari 14.
Comment 3 Greg ORIOL 2020-12-03 00:39:43 PST
Created attachment 415280 [details]
Crashlog from Safari 14
Comment 4 Alexey Proskuryakov 2020-12-03 09:31:49 PST
Perfect, thank you!

I couldn't actually reproduce (neither on a machine with a YubiKey nor on one with a Touch ID) though.

Does the crash look the same every time this reproduces for you (SIGILL in CtapHidDriver::transact)?
Comment 5 Radar WebKit Bug Importer 2020-12-03 09:32:01 PST
Comment 6 Greg ORIOL 2020-12-03 10:09:11 PST
Yes, same crashlog every time, reproduced 100% of the times.
Btw, machine is mid-2014 Retina MacBook Pro (no Touch ID for webauthn).
I'm attaching another crashlog with Safari Technology Preview for reference.
Comment 7 Greg ORIOL 2020-12-03 10:10:21 PST
Created attachment 415309 [details]
Crashlog from Safari Technology Preview 14.1
Comment 8 login Llama 2021-03-03 10:57:46 PST
In webAuthn level 1 authenticators could ignore icon values greater than 128 bytes.

In webAuthn level 2 icons were removed completely as they were never displayed by browsers.

A guess off the top of my head is that if Safari is trying to send a 9kb value that is going to blow up the buffer.  

I did file another issue 220415 about Safari ignoring authenticators maxMsgSize.
That was in the context of exclude lists however, the same bug would more than explain this.

They Yubikey 5 has a maxMsgSize of 1200bytes.  If Safari sends more than that it will get back an error. 

Over NFC a NFC layer error SW_DATA_INVALID
Over USB a CTAP error is returned CTAP1_ERR_INVALID_LENGTH after the first APDU is received.

In neither case is Safari gracefully dealing with the error. 

Safari shouldn't be exceeding maxMsgSize in the first place.  

I hope that solves the mystery of why there is an error.  It also won't happen if the authenticator is U2F only, or may fail in CTAP2.0 then retry in U2F and succeed.