1398268 - [U2F, WebAuthn] Crash when switching between browsers during many verification attempts

Status: VERIFIED FIXED

(Core :: DOM: Device Interfaces, defect, P2)

Description

[Matt Wobensmith [:mwobensmith][:matt:]]

Attachment: log.txt

This test requires Google Chrome.

1. Insert USB hardware key.
2. Open both development build of Fx and latest Google Chrome.
3. Navigate to https://u2f.bin.coffee/ in both browsers.
4. In Fx, click the "U2F Register" button and perform USB verification.
5. Switch to Chrome and click on the "U2F Register" button 15 or more times, but do NOT perform verification.
6. Switch back to Fx and click on "U2F Register" 1-3 times.

Result:

Crash.

Comment 1

[Matt Wobensmith [:mwobensmith][:matt:]]

There are other ways to reach this crash, but this set of steps seemed the most easily reproducible. Also, this can be done with either U2F demo or WebAuthN demo, no difference.

Comment 2

[Tim Taubert [:ttaubert] (inactive)]

From the log:

Thread 64 Crashed:
0   libmozglue.dylib              	0x000000010dd50b19 mozalloc_abort(char const*) + 41
1   libmozglue.dylib              	0x000000010dd50b80 abort + 16
2   XUL                           	0x0000000118e3dab9 __rust_start_panic + 9
3   XUL                           	0x0000000118e3afcd 0x112feb000 + 98893773
4   XUL                           	0x0000000118e3af84 std::panicking::rust_panic_with_hook::h8b9b25777425677b + 436
5   XUL                           	0x0000000118348bbb 0x112feb000 + 87415739
6   XUL                           	0x00000001183386a0 0x112feb000 + 87348896
7   XUL                           	0x0000000118331737 0x112feb000 + 87320375
8   com.apple.framework.IOKit     	0x00007fffa93a6d38 __IOHIDDeviceInputReportApplier + 81
9   com.apple.CoreFoundation      	0x00007fffa73e39b2 __CFSetApplyFunction_block_invoke + 18
10  com.apple.CoreFoundation      	0x00007fffa73cfc8a CFBasicHashApply + 122
11  com.apple.CoreFoundation      	0x00007fffa73e3959 CFSetApplyFunction + 185
12  com.apple.framework.IOKit     	0x00007fffa93a6c37 __IOHIDDeviceInputReportWithTimeStampCallback + 130
13  com.apple.iokit.IOHIDLib      	0x0000000129965e14 IOHIDDeviceClass::_hidReportHandlerCallback(void*, int, void*) + 366
14  com.apple.CoreFoundation      	0x00007fffa741e213 __CFMachPortPerform + 291
15  com.apple.CoreFoundation      	0x00007fffa741e0d9 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE1_PERFORM_FUNCTION__ + 41
16  com.apple.CoreFoundation      	0x00007fffa741e051 __CFRunLoopDoSource1 + 465
17  com.apple.CoreFoundation      	0x00007fffa7415cc5 __CFRunLoopRun + 2389
18  com.apple.CoreFoundation      	0x00007fffa7415114 CFRunLoopRunSpecific + 420

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

Looks like the CF calls our hid report handler callback and we somehow panic in there. The CF APIs really are fun to deal with.

Comment 4

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

I see three places for the handler to panic:

1) The report argument is null
2) The context argument wasn't actually a living Sender (tx)
3) Maybe you're not supposed to keep Send-ing if the last Send was SendErr?


It appears that #1 is unlikely, because other libraries (hidapi, for example) don't check for null there and the API docs don't say it can be null.

#3 isn't in the documentation, and seems unlikely too.

#2 Seems feasible.

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

I'd say this is the same as bug 1400969. This probably isn't caused by interaction with Chrome but simply our faulty macOS code.

Comment 6

[Tim Taubert [:ttaubert] (inactive)]

Not sure what happened, but it seems that I can't reproduce this or even bug 1400969 anymore. Matt, can you please try with the latest Nightly on macOS?

Comment 7

[Matt Wobensmith [:mwobensmith][:matt:]]

Latest Nightly on Mac is fine now, so I'd call this fixed.

Comment 8

[Matt Wobensmith [:mwobensmith][:matt:]]

Wait, I spoke too soon. Still reproduces.

Comment 9

[Tim Taubert [:ttaubert] (inactive)]

Whee, I can reproduce this finally.

> read_new_data_cb 0x1235618c0
> read_new_data_cb 0x1235618c0
> Device::drop() 0x1235618c0
> read_new_data_cb 0x1235618c0

We're of course trying to read data after the device was dropped.

Comment 11

[Tim Taubert [:ttaubert] (inactive)]

Patch and review here: https://github.com/jcjones/u2f-hid-rs/pull/52

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6fab343d63b6
[u2f-hid-rs] Rewrite macOS IOHIDManager communication and state machine r=jcj

Comment 13

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/6fab343d63b6

Comment 14

[Matt Wobensmith [:mwobensmith][:matt:]]

Confirmed fixed as of Nightly 59.0a1, 2017-11-20.