Brave-ios: brave crashing often when cancelling/retrying under demo.yubico.com/u2f

[kjozwiak]

Description:

Brave crashes pretty often when cancelling/retrying during the Registration & Authentication process under https://demo.yubico.com/u2f.

Steps to Reproduce

It's not 100% but I usually end up crashing Brave by simply clicking on Cancel on the modal and retrying 3-4 times in a row. Eventually you'll get a crash.

Example: https://youtu.be/bYWIHMlf71c

Actual result:

Expected result:

Brave shouldn't never be crashing when attempting multiple Cancel & Retry actions under demo.yubico.com/u2f.

Reproduces how often: [Easily reproduced, Intermittent Issue]

Pretty reproducible, I've received ~10 crashes so far while playing around with Cancel & Retry via demo.yubico.com/u2f.

Brave Version:

• Using 1.11.1 (19.08.12.19) from Beta TF

Device details:

• Using iPhone 6s+ iOS 12.4

Website problems only:

• did you check with Brave Shields down? N/A
• did you check in Safari/Firefox (WkWebView-based browsers)? N/A

[jumde]

@iccub and I tried repro'ing this on iPhone 7, iPad6th Gen, iPhone SE. Not able to repro yet. Waiting for the crash logs from app store connect.

[kjozwiak]

I can also reproduce this using 1.11.1 (19.08.13.17) which was just released ~30min ago.

[Brandon-T]

Crash (authentication crash) happens on line 559 of U2FExtensions.swift with a call to the function fido2Service.execute(getAssertionRequest)

which calls a Yubico function and ends up at (after many calls to other functions) -[YKFKeyFIDO2MakeCredentialResponse parseAttestationMap:]:

However, Yubico library calls cbz w0, 0x104a4dc10 (compare if zero - CBZ instruction) which seems to be [NSDictionary isEqualToString:] as the stack pointer is a dictionary and the argument is a selector(isEqualToString:).. But a dictionary can never be equal to a string and so it throws an NSException which cannot be caught at all.

I don't know enough about Yubico yet to figure out what happened but the above should be helpful stack-trace to whoever debugs it.

EDIT:

Second crash (on registration):

-[NSTaggedPointerString objectForKeyedSubscript:]:

called from:

-[YKFKeyFIDO2GetAssertionResponse parseResponseMap:]:

called from: the same line as the previous crash..

[jumde]

This is helpful @Brandon-T , did you repro this with the latest development?

[jumde]

There is one more possibility of a crash I'm investigating:

https://github.com/brave/brave-ios/blob/f96dad3ce0ec107cb1a5e278e41e220a64f2d2b8/Client/U2FExtensions.swift#L380

On cancelling the request using the modals, the code is not terminating this execute request. When the user cancels and retries the execute, a new request is added to the queue which causes multiple requests to be queued evenutally causing a crash.

I’m not sure what’s the best way to cancel this request when sendFIDOReg/AuthErrors is invoked.

[Brandon-T]

How I reproduce the crash like in the video:

1. Enable wireless debugging (so you can debug while the key is plugged into a device).
2. Go to the "https://demo.yubico.com/webauthn-technical" mentioned in the ticket.
3. Press next.
4. You'll be on registration screen.
5. Cancel.
6. Retry.
7. Cancel.
8. Retry - touch the YubiKey to register (until success or crash or you're on Authentication screen).
9. Tap Authenticate if you get here.
10. Tap next.
11. Cancel.
12. Retry.
13. Cancel.
14. Retry (tapping on the key to authenticate).
15. Repeat steps 10 through 14 until it crashes. If it successfully authenticates, go to step 1.

I debugged further by setting a breakpoint on the crash address with lldb b -set 0x09xxxx. I was able to find out the following:

(lldb) po 0x28240a4c0
{
    id = <fa7a5ac1 ... 520278e6>;
    type = "public-key";
}

The above is a representation of the dictionary that is being compared to a string.. which is of course wrong. This happens within the Yubico library itself. It seems to be the exact same dictionary that is pass in from: getAssertionRequest.allowList = allowList

A symbolic breakpoint on -[YKFKeyFIDO2MakeCredentialResponse parseAttestationMap:] or -[NSObject(NSObject) doesNotRecognizeSelector:] will certainly help debug it! IMHO, I think it's actually the Yubico library but we may also be using it weird/incorrectly? Not sure. I don't know enough about it.

[LaurenWags]

I experienced a crash once when attempting to authenticate on https://webauthntest.azurewebsites.net/ but haven't reproduced yet. Was using iPad 5th Gen with 1.11.1 (19.08.13.17).

[kjozwiak]

Moving this into https://github.com/brave/brave-ios/milestone/28 as we won't be fixing it for the 1.11.3(originally 1.11.1) as discussed previously.

[kjozwiak]

Reproduced multiple times with FW 5.2.3 using 1.11.3 (19.08.16.20).

[srirambv]

Verification passed on iPhone XR with iOS 13.1 beta 2 running 1.12(19.09.07.03)

• Followed steps from original issue video, no crash
• Verified no crash from https://github.com/brave/brave-ios/issues/1388#issuecomment-521057419

Verification PASSED on iPad Air 3rd Generation with iOS 13.1 running 1.12 (19.09.10.13):

• Went through and verified the original case via https://github.com/brave/brave-ios/issues/1388#issue-480285100
• Went through and verified https://github.com/brave/brave-ios/issues/1388#issuecomment-521057419

Verification PASSED on iPhone 6s+ using iOS 12.4.1 running 1.12 (19.09.13.06):

• Went through and verified the original case via https://github.com/brave/brave-ios/issues/1388#issue-480285100
• Went through and verified https://github.com/brave/brave-ios/issues/1388#issuecomment-521057419