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

Please update Bouncy Castle #10335

Closed
Neustradamus opened this issue Apr 15, 2020 · 27 comments
Closed

Please update Bouncy Castle #10335

Neustradamus opened this issue Apr 15, 2020 · 27 comments

Comments

@flexsurfer
Copy link
Member

hi, what do you mean by the update? could you please elaborate on what is this?

@Neustradamus
Copy link
Author

@flexsurfer: Thanks for your comment!
I have done several tickets in this organization, it has been solved for one:

Here in this project: https://github.com/status-im/status-react/search?q=bouncycastle&unscoped_q=bouncycastle

Currently the last version is 1.65, please look previous links, there are CVEs...

@flexsurfer
Copy link
Member

@jakubgs are you aware about that?

@jakubgs
Copy link
Member

jakubgs commented Apr 16, 2020

I'm not aware of every single dependency we use but we clearly use it here:
https://github.com/status-im/status-keycard-java/blob/4a69788473fbe25fc2227e9971dce7b63a6a8273/lib/src/main/java/im/status/keycard/applet/RecoverableSignature.java#L3-L10
So seems like someone will have to update it.

@Neustradamus
Copy link
Author

@cammellos
Copy link
Member

Oh my bad, I thought it was solved for keycard, re-opening then, thank you!

@Neustradamus
Copy link
Author

@cammellos
Copy link
Member

We upgraded to 1.6.0, we could not upgrade to the latest version as we are running on some issues with 1.6.5/1.6.6.
I have looked at open CVE https://www.cvedetails.com/vulnerability-list/vendor_id-7637/Bouncycastle.html here and I could only find one issue that affects 1.6.3 only, but could not find anything that affects 1.6.0, so it should be fixed 🤞 .
Please re-open if that's not the case, thanks!

@Neustradamus
Copy link
Author

@cammellos: Thanks for your reply but badly the problem is always here, it is not solved, please reopen this ticket:

I confirm you that it is important to have up-to-date Bouncy Castle.

Note: Other applications have been updated to better Bouncy Castle 1.65/1.66...

@cammellos
Copy link
Member

@Neustradamus I wasn't checking nix/* files, I am not sure that correctly describes the dependencies we use, but rather what's available.

@jakubgs could you confirm if that's correct? (i.e what's listed under nix/* is not what we pull in, but rather we should trust app:dependencies)

I am running the command:

STATUS_GO_ANDROID_LIBDIR=./app/obj/local  ./gradlew app:dependencies

and I have attached the output which only includes 1.6.0

Note: Other applications have been updated to better Bouncy Castle 1.65/1.66...

Yes, I believe is likely an issue on our side, could you please confirm that though 1.6.0 has no known vulnerabilities?

output.txt

Re-opening until confirmed, thanks

@cammellos cammellos reopened this Dec 2, 2020
@jakubgs
Copy link
Member

jakubgs commented Dec 2, 2020

@cammellos These versions are 1.60, not 1.6.0. And as far as I can tell the dependency on 1.56 is caused by sdk-common:26.5.3, which in turn comes from lint-gradle:26.5.3:

\--- com.android.tools.lint:lint-gradle:26.5.3
     +--- com.android.tools:sdk-common:26.5.3
     |    +--- com.android.tools:sdklib:26.5.3
     |    +--- org.bouncycastle:bcpkix-jdk15on:1.56
     |    |    \--- org.bouncycastle:bcprov-jdk15on:1.56
     |    +--- org.bouncycastle:bcprov-jdk15on:1.56

Which I can confirm using go-maven-resolver:

[nix-shell:~/work/status-react]$ echo 'com.android.tools:sdk-common:26.5.3' | go-maven-resolver | grep castle
https://repo.maven.apache.org/maven2/org/bouncycastle/bcpkix-jdk15on/1.56/bcpkix-jdk15on-1.56.pom
https://repo.maven.apache.org/maven2/org/bouncycastle/bcprov-jdk15on/1.56/bcprov-jdk15on-1.56.pom

The dependency on 1.60 appears to be pulled in by react-native-status-keycard:

+--- project :react-native-status-keycard
|    +--- com.facebook.react:react-native:+ -> 0.62.2 (*)
|    +--- org.bouncycastle:bcprov-jdk15on:1.60
|    +--- org.apache.commons:commons-lang3:3.9
|    \--- com.github.status-im.status-keycard-java:android:3.0.4
|         \--- com.github.status-im.status-keycard-java:lib:3.0.4
|              \--- org.bouncycastle:bcprov-jdk15on:1.60
[nix-shell:~/work/status-react]$ echo 'com.github.status-im.status-keycard-java:lib:3.0.4' | go-maven-resolver
https://repository.sonatype.org/content/groups/sonatype-public-grid/com/github/status-im/status-keycard-java/lib/3.0.4/lib-3.0.4.pom
https://repo.maven.apache.org/maven2/org/bouncycastle/bcprov-jdk15on/1.60/bcprov-jdk15on-1.60.pom

Not sure about 1.48, let me check that out.

@jakubgs
Copy link
Member

jakubgs commented Dec 2, 2020

Okay, it appears 1.48 is coming from an ancient gradle:1.5.0:

\--- com.android.tools.build:gradle:1.5.0
     \--- com.android.tools.build:gradle-core:1.5.0
          +--- com.android.tools.build:builder:1.5.0
          |    +--- org.bouncycastle:bcpkix-jdk15on:1.48
          |    |    \--- org.bouncycastle:bcprov-jdk15on:1.48
          |    +--- org.bouncycastle:bcprov-jdk15on:1.48
[nix-shell:~/work/status-react]$ echo 'com.android.tools.build:builder:1.5.0' | go-maven-resolver | grep castle
https://repo.maven.apache.org/maven2/org/bouncycastle/bcprov-jdk15on/1.48/bcprov-jdk15on-1.48.pom
https://repo.maven.apache.org/maven2/org/bouncycastle/bcpkix-jdk15on/1.48/bcpkix-jdk15on-1.48.pom

Which in turn comes from the react-native-fs module:
https://github.com/status-im/status-react/blob/d2493c0725c69138ed8029f50ad99782cd55d922/package.json#L45
Which on version 2.14.1 seems to be indeed using Gradle 1.5.0:
https://github.com/itinance/react-native-fs/blob/2.14.1/android/build.gradle#L11
Which has a newer version 2.15.1 which still uses Gradle version 1.5.0:
https://github.com/itinance/react-native-fs/blob/2.15.1/android/build.gradle#L11
Which is pretty silly if you ask me.

@jakubgs
Copy link
Member

jakubgs commented Dec 2, 2020

could you confirm if that's correct? (i.e what's listed under nix/* is not what we pull in, but rather we should trust app:dependencies)

What's defined in nix/deps/gradle is the result of calling:
https://github.com/status-im/status-react/blob/dd6b5a78304a5091a49cf3f98418b6b449335372/nix/deps/gradle/get_deps.sh#L32-L35
For every react-native-* dependency we have plus our app itself.
Which essentially calls :buildEnvironment and :dependencies.

@churik
Copy link
Member

churik commented May 7, 2021

@cammellos is this issue still relevant or can be closed?

@cammellos
Copy link
Member

Still relevant I believe, we can check keycard if they updated though

@Neustradamus
Copy link
Author

Any news about it?

At this time, the latest version is 1.69.

@jakubgs
Copy link
Member

jakubgs commented Jul 20, 2021

Currently we depend on version from 1.48 to 1.65:

https://github.com/status-im/status-react/blob/56d29866da4fdc05193faa454da1f4ef3e5c5d5f/nix/deps/gradle/deps.urls#L625-L630

Which is most probably because they are pulled in by different dependencies.

I highly doubt we will EVER arrive at a setup where we only use the most recent verison.

@jakubgs
Copy link
Member

jakubgs commented Jul 20, 2021

In relation #10335 (comment), the 2.18.0 version STILL uses 1.5.0:

        classpath 'com.android.tools.build:gradle:1.5.0'

https://github.com/itinance/react-native-fs/blob/v2.18.0/android/build.gradle#L11

@jakubgs
Copy link
Member

jakubgs commented Jul 20, 2021

In addition to that the react-native-keychain also depends on 1.1.3 of com.android.tools.build:gradle:

    classpath 'com.android.tools.build:gradle:1.1.3'

https://github.com/status-im/react-native-keychain/blob/v.3.0.0-status/android/build.gradle#L7

But that's our own fork, so it could be updated fairly easily... probably.

Also react-native-dialogs also depends on 1.3.1, but that's not a fork:

        classpath 'com.android.tools.build:gradle:1.3.1'

https://github.com/react-native-dialogs/react-native-dialogs/blob/v1.1.0/android/build.gradle#L7

@Neustradamus
Copy link
Author

@jakubgs: Thanks for your replies!
For security, without CVEs please update it...

@jakubgs
Copy link
Member

jakubgs commented Jul 20, 2021

Maybe @cammellos might disagree, but I really don't think those older versions matter.

Does it really matter if something like react-native-dialog uses the most recent version?
Maybe for react-native-keychain it matters, but I'm not even sure it's used for anything, it might just be a dependency for technical reasons and isn't being actively used in our app.

@churik
Copy link
Member

churik commented Jul 27, 2022

Closing as a stale issue; please, feel free to reopen if it matters from a technical POV.
Thank you!

@churik churik closed this as completed Jul 27, 2022
@Neustradamus
Copy link
Author

Dear @churik,

It is not solved, always CVEs in your products!

Currently, the last version of Bouncy Castle is 1.71.

Please reopen this ticket and solve all vulnerabilities.

@churik churik reopened this Jul 27, 2022
@jakubgs
Copy link
Member

jakubgs commented Jul 27, 2022

As I said in #10335 (comment), do we really care?

The only reason for caring is possibly the dependency of react-native-status-keycard on bouncycastle:

    implementation 'org.bouncycastle:bcprov-jdk15on:1.60'

https://github.com/status-im/react-native-status-keycard/blob/af61b021/android/build.gradle#L47

From a short skimming of the source code the only place it is actually used is in two files:
https://github.com/status-im/react-native-status-keycard/blob/b9f242a/android/src/main/java/im/status/ethereum/keycard/Installer.java#L10
https://github.com/status-im/react-native-status-keycard/blob/b9f242a/android/src/main/java/im/status/ethereum/keycard/SmartCard.java#L43
In both cases they import only org.bouncycastle.util.encoders.Hex, simply to use Hex.decode() or Hex.toHexString().

Seems like quite the overkill to pull in such a big cryptographic library like BouncyCastle just to do hex decoding...

But this is a question for Mobile team devs. But as far as I can tell we really should not care about these updates. I HIGHTLY doubt there are actually dangerous vulnerabilities in hex decoding/encoding.

@churik churik closed this as completed Dec 5, 2022
@Neustradamus
Copy link
Author

@churik: It is not solved: https://github.com/status-im/status-mobile/search?q=bouncycastle

nix/deps/gradle/deps.list
org.apache.httpcomponents:httpmime:4.5.6
org.bouncycastle:bcpkix-jdk15on:1.48
org.bouncycastle:bcpkix-jdk15on:1.56
org.bouncycastle:bcprov-jdk15on:1.48
org.bouncycastle:bcprov-jdk15on:1.56
org.bouncycastle:bcprov-jdk15on:1.60
org.checkerframework:checker-qual:2.5.2

nix/deps/gradle/deps.urls
https://repo.maven.apache.org/maven2/org/apache/httpcomponents/project/7/project-7.pom
https://repo.maven.apache.org/maven2/org/bouncycastle/bcpkix-jdk15on/1.48/bcpkix-jdk15on-1.48.pom
https://repo.maven.apache.org/maven2/org/bouncycastle/bcpkix-jdk15on/1.56/bcpkix-jdk15on-1.56.pom

nix/deps/gradle/deps.json
      "sha256": "1hs8m8cgyypd6vb882zjyns58nhzrkm7cpbb0kg5i9amhm1blvix"
    }
  },

  {
    "path": "org/bouncycastle/bcpkix-jdk15on/1.48/bcpkix-jdk15on-1.48",
    "path": "org/bouncycastle/bcpkix-jdk15on/1.56/bcpkix-jdk15on-1.56",
    "host": "https://repo.maven.apache.org/maven2",

@jakubgs
Copy link
Member

jakubgs commented Dec 6, 2022

We don't care. Our minimal usage of Hex.decode() or Hex.toHexString() in react-native-status-keycard is hardly a vast attack vector, and it doesn't matter to us. We will probably just drop usage of this library eventually. Stop pestering us.

@status-im status-im locked as resolved and limited conversation to collaborators Dec 6, 2022
@jakubgs
Copy link
Member

jakubgs commented Dec 6, 2022

Based on comment in:

The CVE-2020-15522 present in 1.60 does not affect us. And attempts to upgrade cause difficult to debug runtime crashes.

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

No branches or pull requests

5 participants