covid-alert-ny / covid-green-app

Covid Alert NY - React-Native App
Apache License 2.0
6 stars 1 forks source link

Android App Crashes When Submitting Exposure Keys (CRITICAL) #70

Closed dharding closed 4 years ago

dharding commented 4 years ago

The Android version of the app crashes when trying to share exposure keys.

Platform: Android Build: 13

Error:

AndroidRuntime: FATAL EXCEPTION: mqt_native_modules
Process: gov.ny.health.proximity, PID: 19935
AndroidRuntime: java.lang.NullPointerException: Attempt to invoke virtual method 'byte[] java.lang.String.getBytes()' on a null object reference
AndroidRuntime:     at android.util.Base64.decode(Base64.java:119)
AndroidRuntime:     at com.rajivshah.safetynet.RNGoogleSafetyNetModule.stringToBytes(Unknown Source:1)
AndroidRuntime:     at com.rajivshah.safetynet.RNGoogleSafetyNetModule.sendAttestationRequest(Unknown Source:0)
AndroidRuntime:     at java.lang.reflect.Method.invoke(Native Method)
AndroidRuntime:     at com.facebook.react.bridge.JavaMethodWrapper.invoke(Unknown Source:149)
AndroidRuntime:     at com.facebook.react.bridge.JavaModuleWrapper.invoke(Unknown Source:21)
AndroidRuntime:     at com.facebook.react.bridge.queue.NativeRunnable.run(Native Method)
AndroidRuntime:     at android.os.Handler.handleCallback(Handler.java:883)
AndroidRuntime:     at android.os.Handler.dispatchMessage(Handler.java:100)
AndroidRuntime:     at com.facebook.react.bridge.queue.MessageQueueThreadHandler.dispatchMessage(Unknown Source:0)
AndroidRuntime:     at android.os.Looper.loop(Looper.java:214)
AndroidRuntime:     at com.facebook.react.bridge.queue.MessageQueueThreadImpl$4.run(Unknown Source:37)
AndroidRuntime:     at java.lang.Thread.run(Thread.java:919)

Steps to reproduce:

  1. Complete the onboarding normally
  2. Go to the "My COVID Alerts" tab
  3. Tap on "What to do if you test positive COVID-19"
  4. Tap on "Share your Close Contact Codes"
  5. Enter any code
  6. Tap the "Next" button
  7. App will either crash at this point or after you allow Android to share your keys (depending on if you have allowed this successfully before)
segfault commented 4 years ago

Looking at the app code it seems like we're failing in Safetynet verification. The only call is at https://github.com/project-vagabond/covid-green-app/blob/nys/src/services/api/index.ts#L35

Given that code the only was we would see a null passed to the base64 function is if the nonce is null. From the upstream RN wrapper library: https://github.com/rajivshah3/react-native-google-safetynet/blob/master/android/src/main/java/com/rajivshah/safetynet/RNGoogleSafetyNetModule.java#L84

This is a bit of a mystery since to view these screens you must have completed registration with the backend api which provides you with the nonce and a token. Somehow we're getting into a state where the app is trying to verify itself against the server but doesn't have a nonce after registration has completed and initial Safetynet verification has passed.

floridemai commented 4 years ago

@dharding @segfault I'll take a look at this and come up with all the details when I'll be able to get hold of some codes to tests the whole flow. Will pick something else in the meantime. /cc @arybka777

floridemai commented 4 years ago

Issue was: any error on the server-side was treated as being successful and we were calling a safetynet verification with an invalin none (i think). All of this was address by simply enabling ssl pinning (https://github.com/project-vagabond/covid-green-app/pull/88). Issue should be solved in build 15.

dharding commented 4 years ago

@floridemai have you been able to confirm if this issue is addressed?

segfault commented 4 years ago

Are we logging the error on the client side?

floridemai commented 4 years ago

@dharding Yes, I think so. I tried doing the same thing as before but got an "code is expired" and could not replicate the issue.

floridemai commented 4 years ago

Are we logging the error on the client side?

@segfault We do log errors on the client side, but in this case it was a native error, and our try/catch blocks didn't have any effect.

segfault commented 4 years ago

I meant the upstream request fail not the SafetyNet RN api call. That call was passed null and somewhere prior there was a call or fetch from something that can and should catch/log the issue. Either we got back a bad value from the backend api or we pulled a bad value. Looking through the full exposure upload workflow it seems we might have stored a bad value for the uploadToken.

Reading through there code there appear to be a few ways for this to fail. https://github.com/project-vagabond/covid-green-app/blob/nys/src/components/views/upload-keys.tsx#L53

fahd-arshad commented 4 years ago

Needs to be validated in build 15+

fahd-arshad commented 4 years ago

@dharding was unable to submit a number (got error invalid code).

floridemai commented 4 years ago

@dharding I was able to submit codes on both iOS and android. @fiacc tried on android on build 15 and worked for him as well. Note that the codes were generated with https://push.prd.ctprox.health.ny.gov/notify/positive. Let us know if you still can't submit them. Thanks

dharding commented 4 years ago

@floridemai I will take a look

dharding commented 4 years ago

I was able to enter a positive test case code and then agree to share my keys without a crash. Closing.