search

akumaigorodski / wallet

Bitcoin wallet for Android
https://sbw.app
Apache License 2.0
240 stars 74 forks source link

Cryptographic APIs misuses #126

Closed misterAnderson90 closed 2 years ago

misterAnderson90 commented 2 years ago

I'm a PhD student interested in finding security vulnerabilities in open source projects.

We found a total of 112 warnings (indicating potential vulnerabilities) when running the CogniCrypt static analyzer (*) on wallet (or its library dependencies). We documented each one of these issues in private gists for the sake of confidentiality (non-disclosure).

Can you please let us know whether we can share these gists with you? We are eager to evaluate the perception of developers (e.g. severity of these warnings) and improve wallet's security, and the quality of the reports of static analysis tools. (*) https://github.com/CROSSINGTUD/CryptoAnalysis

akumaigorodski commented 2 years ago

Yes, please do share these gists, thanks.

misterAnderson90 commented 2 years ago

Hello @btcontract,

From the 112 warnings that CogniCrypt reported, we randomly selected 5 gists to share with you. If you are interested, we can share the report with all warnings.

Gist 01 - Cipher Gist 02 - Mac Gist 03 - SecretKeySpec Gist 04 - MessageDigest Gist 05 - IvParameterSpec

For any doubts, you can comment directly in the gists and we can clarify the issues.

Best regards,

akumaigorodski commented 2 years ago

Thanks, looking at these I don't see issues yet:

Gist 01 - Cipher - init method is called in cipher method prior to calling doFinal, perhaps analyzer was not able to see this.

Gist 02 - Mac - bitcoinj has been removed from project recently.

Gist 03 - SecretKeySpec - bitcoinj has been removed from project recently.

Gist 04 - MessageDigest - bitcoinj has been removed from project recently.

Gist 05 - IvParameterSpec - initVector data is supposed to be randomized prior to being supplied to method cipher.