Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Security issue with cardinal commerce dependency #305

Closed
calvarez-ov opened this issue Jul 30, 2020 · 13 comments
Closed

Security issue with cardinal commerce dependency #305

calvarez-ov opened this issue Jul 30, 2020 · 13 comments

Comments

@calvarez-ov
Copy link

calvarez-ov commented Jul 30, 2020

General information

  • SDK/Library version: cardinalmobilesdk:2.2.3-2
  • Environment: Production
  • Android Version and Device: Not device specific
  • Braintree dependencies:
    From ./gradlew dependencies:
com.braintreepayments.api:drop-in:4.6.0
+--- com.braintreepayments.api:braintree:3.11.1 -> 3.12.0
|    +--- com.braintreepayments.api:google-payment:3.3.1
|    +--- com.braintreepayments.api:core:3.12.0
|    |    \--- com.braintreepayments:browser-switch:0.2.0 -> com.libon.android.braintreepayments:browser-switch:0.2.0-5+efa3414
|    +--- com.paypal.android.sdk:paypal-one-touch:3.12.0
|    |    +--- com.braintreepayments.api:core:3.12.0 (*)
|    |    \--- com.paypal.android.sdk:data-collector:3.12.0
|    \--- androidx.appcompat:appcompat:1.0.1 -> 1.1.0 (*)
+--- com.braintreepayments:card-form:4.3.0
|    \--- com.google.android.material:material:1.0.0 -> 1.1.0 (*)
+--- com.braintreepayments.api:three-d-secure:3.11.1 -> 3.12.0
     +--- org.jfrog.cardinalcommerce.gradle:cardinalmobilesdk:2.2.3-2

Issue description

The Google Play console shows a warning about our apk:

Your app contains unsafe cryptographic encryption patterns. Please see this Google Help Center article for details.

Vulnerable classes:

d.f.d.a.i.d.b

From our deobfuscation file, we see that this is related to the cardinal commerce library:

com.cardinalcommerce.shared.cs.utils.d -> d.f.d.a.i.d:
    com.cardinalcommerce.shared.cs.utils.a c -> d
    android.content.SharedPreferences b -> a
    com.cardinalcommerce.shared.cs.utils.d a -> c
    com.cardinalcommerce.shared.cs.utils.c d -> b
    1:1:java.lang.String com.cardinalcommerce.shared.cs.utils.a.a(byte[]):0:0 -> b
    1:1:void a(java.lang.String,java.lang.String):0 -> b
    2:2:void a(java.lang.String,java.lang.String):0:0 -> b
    3:3:java.lang.String com.cardinalcommerce.shared.cs.utils.a.a(byte[]):0:0 -> b
    3:3:void a(java.lang.String,java.lang.String):0 -> b
    4:4:void a(java.lang.String,java.lang.String):0:0 -> b
    1:1:java.lang.String com.cardinalcommerce.shared.cs.utils.a.b(byte[]):0:0 -> c
    1:1:java.lang.String b(java.lang.String,java.lang.String):0 -> c
    2:2:java.lang.String b(java.lang.String,java.lang.String):0:0 -> c

Indeed, if you see this com.cardinalcommerce.shared.cs.utils.a.a method in Android Studio, it shows the decompiled code, and it looks exactly like what the google play article is warning about: containing a secret key in the code.

@jcloquell
Copy link

@calvarez-ov You were 10 minutes faster than me, I was about to write the same issue today 🙏

On a side note, I updated the Braintree SDK to the latest version (3.13.0) and the issue is still happening because the cardinal commerce dependency hasn't been updated, so the secret key is still generated from a statically computed string.

Also, a funny thing I found out is that if I submit my app to Google Play Store with ProGuard being disabled (so no code obfuscation), the warning doesn't show up in the developer console 🤷‍♂️

@calvarez-ov
Copy link
Author

@jcloquell Thanks for the additional info!

It's weird, because this is the second release we uploaded with the same versions of the libraries, and I didn't notice a warning on the previous release. Not sure why.

Maybe adding a proguard keep rule for cardinalcommerce classes can avoid the warning in the play store console 🤔

@hollabaq86
Copy link
Contributor

👋 @calvarez-ov and @jcloquell thanks for bringing this to our attention.

We're sending this feedback to our MPI provider CardinalCommerce so they can update their SDK. I don't have an ETA on when we'll get a new version of Cardinal's SDK that resolves this warning, so in the meantime we'll keep this issue open to track updates.

@keval1040
Copy link

keval1040 commented Sep 25, 2020

Hello @scannillo and @sshropshire this issue open is July 30 not any update your side which time is fixed by because client is not wait for your time so this impact your Braintree products. client choose another payment method.

@sshropshire
Copy link
Contributor

Hi @keval1040 unfortunately this is a third-party dependency. We have reached out and are awaiting feedback. I believe all of us are experiencing longer than usual wait times with the onset of the COVID-19 pandemic. We will provide an update as soon as possible. Thank you for your patience.

@calvarez-ov
Copy link
Author

Is there an open issue tracker with cardinal commerce where this has been reported?

@sshropshire
Copy link
Contributor

@calvarez-ov No but they are working on a solution. They are aware of the Google Play store impact.

@GauravBalbhadra
Copy link

@sshropshire Any update on this. can you ask the cardinal team to update asap because our client is waiting for the app to go live.

@keval1040
Copy link

keval1040 commented Oct 10, 2020

@sshropshire Any update on this. can you ask the cardinal team to update asap because our client is waiting for the app to go live.

@GauravBalbhadra Please add proguard rules cardinal then Play store approved your application this is temporary solution

@calvarez-ov
Copy link
Author

@GauravBalbhadra Please add proguard rules cardinal then Play store approved your application this is temporary solution

Which specific rules did you validate as fixing this problem? Not everyone will want to completely disable proguard for their whole app.

@GauravBalbhadra
Copy link

GauravBalbhadra commented Oct 10, 2020

@keval1040 Thanks.. your suggestion worked. Able to successfully get approval from the play store.
@calvarez-ov I haven't completely disabled the Proguard, have added this rule to disable the only cardinalcommerce like this:

-keep class com.cardinalcommerce.** { *; } -dontwarn com.cardinalcommerce.**

@shubhagarwal85
Copy link

@keval1040 In my app, the ProGuard is disabled, yet I am getting warning from the play store side. Any suggestions
?

@sshropshire
Copy link
Contributor

Cardinal provided an update. This should now be fixed in version 3.14.2.

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

No branches or pull requests

7 participants