openMF / android-client

An android client for the MifosX platform
https://openmf.github.io/mobileapps.github.io/
Mozilla Public License 2.0
193 stars 570 forks source link

`TrustManager` that accepts all certificates #1978

Closed Onyx2406 closed 10 months ago

Onyx2406 commented 1 year ago

Summary:

The app trusts all digital certificates, which isn't safe. It should only trust certificates from known, reliable sources.

Steps to reproduce:

  1. Find the part of the app code in MifosOkHttpClient.
  2. It uses a TrustManager that accepts all certificates.
  3. This means the app is willing to make secure connections with any server, even if it can't prove it's trustworthy.

To deploy CodeQL into your forked repository, use this codeQL.yml script : codeQL.yml

Expected behaviour:

The app should only trust certificates from servers it knows are safe. It should have a list of trusted certificates, and it should reject connections from servers not on that list.

Observed behaviour:

Right now, the app doesn't check certificates properly. It will make secure connections with any server, even if it can't prove it's safe. This could let a attacker pretend to be a safe server, tricking the app into sharing private information.

Device and Android version:

This isn't about a specific Android phone or Android version. It's about code in master branch.

Screenshots:

image

References:

Android Developers: Security with HTTPS and SSL.

Malaydewangan09 commented 1 year ago

Hey! @Onyx2406 I want to work on this.

Onyx2406 commented 1 year ago

Hey! @Onyx2406 I want to work on this.

Thanks for showing interest. You can work on it and create a PR for the same, if you have any approach to solve this, we can discuss that as well. While I will ask the the appropriate people to assign you this issue.

Malaydewangan09 commented 1 year ago

@Onyx2406 There can be multiple approaches for this I guess

Please do correct me if these are not the correct ways.

Onyx2406 commented 1 year ago

@Onyx2406 There can be multiple approaches for this I guess

  • We can just remove the custom Trust manager in the code and use default validation.
  • We can include the certificates in a .crt format
  • We can use certificate Pinning (much secure).

Please do correct me if these are not the correct ways.

I believe they are correct. You can implement whichever option is most suitable for you, while prioritizing increased security. You can open a PR and then we can test it out on emulator.