search

status-im / status-mobile

a free (libre) open source, mobile OS for Ethereum
https://status.app
Mozilla Public License 2.0
3.9k stars 987 forks source link

Please update Bouncy Castle #10335

Closed Neustradamus closed 1 year ago

Neustradamus commented 4 years ago

Please update Bouncy Castle

flexsurfer commented 4 years ago

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

Neustradamus commented 4 years ago

@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 commented 4 years ago

@jakubgs are you aware about that?

jakubgs commented 4 years ago

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 commented 4 years ago

@cammellos: It is not solved!!!

cammellos commented 4 years ago

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

Neustradamus commented 4 years ago

@cammellos: At this time, it is Bouncy Castle 1.66

cammellos commented 3 years ago

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 :crossed_fingers: . Please re-open if that's not the case, thanks!

Neustradamus commented 3 years ago

@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 commented 3 years ago

@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

jakubgs commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

cammellos commented 3 years ago

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

Neustradamus commented 3 years ago

Any news about it?

At this time, the latest version is 1.69.

jakubgs commented 3 years ago

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 commented 3 years ago

In relation https://github.com/status-im/status-react/issues/10335#issuecomment-737229515, 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 commented 3 years ago

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 commented 3 years ago

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

jakubgs commented 3 years ago

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 commented 2 years ago

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

Neustradamus commented 2 years ago

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.

jakubgs commented 2 years ago

As I said in https://github.com/status-im/status-mobile/issues/10335#issuecomment-883184339, 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.

Neustradamus commented 1 year ago

@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 commented 1 year ago

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.

jakubgs commented 1 year ago

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.