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

Google Play blocker; Vulnerable class define unsafe HostnameVerifier #840

Closed
ClaudiaJ opened this issue May 18, 2020 · 10 comments
Closed

Google Play blocker; Vulnerable class define unsafe HostnameVerifier #840

ClaudiaJ opened this issue May 18, 2020 · 10 comments

Comments

@ClaudiaJ
Copy link

Our security team identified the following vulnerability as a Google Play blocker, the source of which traced to our usage of Sentry SDK


The vulnerable classes define a custom HostnameVerifier that does not perform any validation of the server's hostname:

/**
* HostnameVerifier allowing wildcard certificates to work without adding them to the truststore.
*/
private static final HostnameVerifier NAIVE_VERIFIER = new HostnameVerifier() {
@Override
public boolean verify(String hostname, SSLSession sslSession) {
return true;
}
};

Hence, connections using this HostnameVerifier will accept any certificate signed by a valid Certificate Authority for any hostname as valid, allowing an attacker to use a CA-signed certificate issued for a domain they own to perform a man-in-the-middle attack against the App.

Regardless of whether the affected classes are actually used at runtime or not, Google Play is blocking any App that defines such an insecure HostnameVerifier, as detailed on Google's support page:

"Beginning March 1, 2017, Google Play will block publishing of any new apps or updates that use an unsafe implementation of HostnameVerifier."

@marandaneto
Copy link
Contributor

hey @ClaudiaJ thanks for raising this.

it's an opt-in feature as it needs bypassSecurity enabled, but as you said, it doesn't matter if it's used at runtime or not.

We don't call setDefaultHostnameVerifier though, but setHostnameVerifier, not sure if Google will complain about this and to be honest, we've never got complainings about App's being rejected due to this reason.

Could you link me where it says that setHostnameVerifier is also a vulnerable method? thanks :)

@ClaudiaJ
Copy link
Author

Hi thanks for the quick response!

The content below the horizontal rule I've copied straight from the report back from our security audit tooling, with the sample code replaced with a link to the location I was able to find in which HostnameVerifier had been overridden with behavior identified in the report and matching their included sample code.

I'm suspecting the audit tooling only identifies the fact code exists overriding behavior on HostnameVerifier, and thus I'm working with our security team to identify evidence proving this to be a false positive.

Thanks for pointing out the difference in setHostnameVerifier and setDefaultHostnameVerifier, I missed that distinction in my initial investigation into the report.

I did another Search here in GitHub and found this related PR #786 from October, closed as having been unnecessary change and that this snippet was identified may be the result of improper audit.

@marandaneto
Copy link
Contributor

@ClaudiaJ yep, I guess we are fine, but let's keep this open and please, let us know when your security team replies, so we'd be sure if we'd need to do anything or not.

I'd also invite you to try our new Android SDK, v2 and GA since February which lies in this repo https://github.com/getsentry/sentry-android

Thanks :)

@ClaudiaJ
Copy link
Author

Hi @marandaneto I got word from Security, this is now considered a non-issue for us with the evidence identified in this Issue :) Thanks again for your patience!

@marandaneto
Copy link
Contributor

@ClaudiaJ thanks for this.

I'll close this then :)

@jacky-ew
Copy link

jacky-ew commented Jan 8, 2021

Hi, im on a react native project, facing similar issue, in our project could not locate any phrases matching HostnameVerifier, yet we still got rejected due to it.

May i know is android bridge of sentry fixed this issue?

version in used: "@sentry/react-native": "^1.3.3",

@marandaneto
Copy link
Contributor

marandaneto commented Jan 8, 2021

@jacky-ew this part of the code has been removed so try to upgrade your Sentry RN dependency to min. 2.0.0
PR: #944

@jacky-ew
Copy link

Hi on the other app we're working on was with sentry v2.1.0, but it seem like we still having the hostname verifier issue on google

@marandaneto
Copy link
Contributor

@jacky-ew which sentry? sentry-react-native or sentry-android? I'd love more details as this code has been entirely removed and solved for a few other users.
Consider opening a new issue with a full report as well, thanks.

@jacky-ew
Copy link

Hi @marandaneto , after dropping google a ticket it appears that we are having hostnameverifier on other library instead of sentry, thanks for assisting. Sorry for the false alarm

@getsentry getsentry locked and limited conversation to collaborators Jan 11, 2021
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

3 participants