Skip to content
This repository has been archived by the owner on May 10, 2024. It is now read-only.

brave crashing often when cancelling/retrying under demo.yubico.com/u2f #1388

Closed
kjozwiak opened this issue Aug 13, 2019 · 10 comments · Fixed by #1396
Closed

brave crashing often when cancelling/retrying under demo.yubico.com/u2f #1388

kjozwiak opened this issue Aug 13, 2019 · 10 comments · Fixed by #1396

Comments

@kjozwiak
Copy link
Member

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:

IMG_9863

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
Copy link
Contributor

jumde commented Aug 13, 2019

@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
Copy link
Member Author

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

@Brandon-T
Copy link
Collaborator

Brandon-T commented Aug 14, 2019

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
Copy link
Contributor

jumde commented Aug 14, 2019

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

@jumde
Copy link
Contributor

jumde commented Aug 14, 2019

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

fido2Service.execute(makeCredentialRequest) { [weak self] response, error in

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
Copy link
Collaborator

Brandon-T commented Aug 14, 2019

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
Copy link
Member

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
Copy link
Member Author

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 kjozwiak modified the milestones: 1.11.3, 1.11.4 Aug 16, 2019
@kjozwiak
Copy link
Member Author

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

@srirambv
Copy link
Contributor

srirambv commented Sep 9, 2019

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

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

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants