firebase / firebase-admin-java

Firebase Admin Java SDK
https://firebase.google.com/docs/admin/setup
Apache License 2.0
540 stars 267 forks source link

Netty dependency version is vulnerable #423

Closed alkoclick closed 4 years ago

alkoclick commented 4 years ago

Hey folks,

You are using a vulnerable version of Netty with an impressive 9.8/10 criticality score. Suggest patching ASAP to 4.1.50.Final which should have binary compatibility as they are on modified semver

I'd submit a PR but I'm awaiting a confirmation on signing the Google CLA and I assume you want to move fast here. Tests seemed to pass in my local fork though

Cheers!

google-oss-bot commented 4 years ago

I found a few problems with this issue:

alkoclick commented 4 years ago

Ping, is this a concern to any maintainer?

hiranya911 commented 4 years ago

It's not a high priority for a couple of reasons:

  1. We only use Netty for opening client-side WebSocket connections to RTDB. So the issue described in the CVE doesn't quite apply.
  2. Developers can easily upgrade Netty version at their end by overriding the version we have currently specified.

We will eventually bump the Netty version in our pom file.

hiranya911 commented 4 years ago

Fixed in #438

theHilikus commented 4 years ago

@hiranya911 the problematic pulled netty dependency is not a direct dependency of firebase-admin. firebase-admin 6.13 is pulling netty via two different paths:

+- com.google.firebase:firebase-admin:jar:6.13.0:compile
|  +- io.netty:netty-codec-http:jar:4.1.49.Final:compile
|  |  +- io.netty:netty-common:jar:4.1.49.Final:compile
|  |  +- io.netty:netty-buffer:jar:4.1.49.Final:compile
|  |  \- io.netty:netty-codec:jar:4.1.49.Final:compile
|  +- io.netty:netty-handler:jar:4.1.49.Final:compile
|  |  \- io.netty:netty-resolver:jar:4.1.49.Final:compile
|  \- io.netty:netty-transport:jar:4.1.49.Final:compile

and

+- com.google.firebase:firebase-admin:jar:6.13.0:compile
|  +- com.google.cloud:google-cloud-firestore:jar:1.31.0:compile
|  |  +- com.google.cloud:google-cloud-core-grpc:jar:1.91.3:compile
|  |  |  +- com.google.api:gax:jar:1.49.1:compile
|  |  |  \- com.google.api:gax-grpc:jar:1.49.1:compile
|  |  |     +- io.grpc:grpc-stub:jar:1.23.0:compile
|  |  |     |  \- io.grpc:grpc-api:jar:1.23.0:compile
|  |  |     |     \- org.codehaus.mojo:animal-sniffer-annotations:jar:1.17:compile
|  |  |     +- io.grpc:grpc-auth:jar:1.23.0:compile
|  |  |     +- io.grpc:grpc-protobuf:jar:1.23.0:compile
|  |  |     |  \- io.grpc:grpc-protobuf-lite:jar:1.23.0:compile
|  |  |     +- io.grpc:grpc-netty-shaded:jar:1.23.0:compile

The direct dependency of firebase-admin is netty 4.1.49, which is not affected by CVE-2020-11612. The problematic netty is the second one since grpc-netty-shaded:1.23.0 is shading io.netty:netty-transport:4.1.38.Final.

I just want to confirm that your assessment that it's not an issue because you only use the client part of the library to connect to RTDB is still valid for this second path. What about google-cloud-firestore, your direct dependency that pulls the problematic netty? That shaded version of netty (4.1.38) has several bad CVEs:

  1. CVE-2019-16869
  2. CVE-2019-20444
  3. CVE-2019-20445
  4. CVE-2020-11612 (the one mentioned here)
hiranya911 commented 4 years ago

438 has upgraded our dependency on Firestore to the latest available version. If Firestore is still using an older version of Netty it needs to be fixed at https://github.com/googleapis/java-firestore.

Either way, you can always specify a more recent version of Netty in your own application dependency list, and override the older versions getting pulled via transitive dependencies.

theHilikus commented 4 years ago

About your second point, i don't think you can. that was the first thing i tried: overriding the buggy version of grpc-netty-shaded. it conflicts with other dependency paths.

Could not resolve version conflict among [com.google.firebase:firebase-admin:jar:6.13.0 -> com.google.cloud:google-cloud-firestore:jar:1.31.0 -> com.google.cloud:google-cloud-core-grpc:jar:1.91.3 -> com.google.api:gax-grpc:jar:1.49.1 -> io.grpc:grpc-netty-shaded:jar:1.23.1 -> io.grpc:grpc-core:jar:[1.23.1,1.23.1], com.google.firebase:firebase-admin:jar:6.13.0 -> com.google.cloud:google-cloud-firestore:jar:1.31.0 -> com.google.cloud:google-cloud-core-grpc:jar:1.91.3 -> com.google.api:gax-grpc:jar:1.49.1 -> io.grpc:grpc-alts:jar:1.23.0 -> io.grpc:grpc-grpclb:jar:1.23.0 -> io.grpc:grpc-core:jar:[1.23.0,1.23.0], com.google.firebase:firebase-admin:jar:6.13.0 -> com.google.cloud:google-cloud-firestore:jar:1.31.0 -> com.google.cloud:google-cloud-core-grpc:jar:1.91.3 -> com.google.api:gax-grpc:jar:1.49.1 -> io.grpc:grpc-alts:jar:1.23.0 -> io.grpc:grpc-core:jar:[1.23.0,1.23.0], com.google.firebase:firebase-admin:jar:6.13.0 -> com.google.cloud:google-cloud-firestore:jar:1.31.0 -> com.google.cloud:google-cloud-core-grpc:jar:1.91.3 -> io.grpc:grpc-core:jar:1.24.1, com.google.firebase:firebase-admin:jar:6.13.0 -> com.google.cloud:google-cloud-firestore:jar:1.31.0 -> io.grpc:grpc-core:jar:1.24.1, com.google.firebase:firebase-admin:jar:6.13.0 -> com.google.cloud:google-cloud-firestore:jar:1.31.0 -> io.opencensus:opencensus-contrib-grpc-util:jar:0.24.0 -> io.grpc:grpc-core:jar:1.22.1] -> [Help 1]

Overriding higher in the dependency chain also didn't work due to similar errors and it also has its own set of risks

About your first point, i will check if the updates fix the problem and if not i will report the bug upstream, I was just trying to get an idea if the next update on firebase-admin would be a "do it when you can" or if it should be "do it immediatly since 6.13 has a bad security flaw"

hiranya911 commented 4 years ago

Why not override the version of netty instead of grpc-netty-shaded?

You can also add a dependency exclusion for netty through firebase-admin.

theHilikus commented 4 years ago

Why not override the version of netty instead of grpc-netty-shaded?

do you mean the direct dependency of firebase-admin? the one you updated in #438 ? if so, because the direct dependency of netty does not have any critical CVEs opened to it. Like I mentioned in my first post, firebase-admin has 2 different dependencies on netty. The direct one (the one you updated) is fine, no CVEs. the problem is the shaded/indirect one: grpc-netty-shaded

If by "netty instead of grpc-netty-shaded" you mean something else please explain further. But in short, the bad CVEs are in grpc-netty-shaded. that's the dependency that i would like to remove from my classpath. Keep in mind i'm talking about the released 6.13 version of firebase-admin. I haven't checked if the buggy grpc-netty-shaded is still there after your updates

About excluding grpc-netty-shaded, I have no idea where these classes are used so I can't really tell if this will create NoClassDefFoundErrors at runtime

hiranya911 commented 4 years ago

You need to override the dependencies as higher in the tree as possible. With firebase-admin:6.13.0 and google-cloud-firestore:1.34.0, my dependency tree comes out like this:

[INFO] com.hkj:firebase-test:jar:1.0-SNAPSHOT
[INFO] +- com.google.firebase:firebase-admin:jar:6.13.0:compile
[INFO] |  +- com.google.api-client:google-api-client:jar:1.30.1:compile
[INFO] |  |  \- com.google.oauth-client:google-oauth-client:jar:1.30.1:compile
[INFO] |  +- com.google.api-client:google-api-client-gson:jar:1.30.1:compile
[INFO] |  |  \- com.google.http-client:google-http-client-gson:jar:1.30.1:compile
[INFO] |  +- com.google.http-client:google-http-client:jar:1.30.1:compile
[INFO] |  +- com.google.api:api-common:jar:1.8.1:compile
[INFO] |  +- com.google.auth:google-auth-library-oauth2-http:jar:0.17.1:compile
[INFO] |  +- com.google.cloud:google-cloud-storage:jar:1.91.0:compile
[INFO] |  |  +- com.google.cloud:google-cloud-core-http:jar:1.90.0:compile
[INFO] |  |  |  +- com.google.http-client:google-http-client-appengine:jar:1.31.0:compile
[INFO] |  |  |  \- com.google.api:gax-httpjson:jar:0.65.1:compile
[INFO] |  |  \- com.google.apis:google-api-services-storage:jar:v1-rev20190624-1.30.1:compile
[INFO] |  +- com.google.guava:guava:jar:26.0-android:compile
[INFO] |  +- org.slf4j:slf4j-api:jar:1.7.25:compile
[INFO] |  +- io.netty:netty-codec-http:jar:4.1.45.Final:compile
[INFO] |  |  +- io.netty:netty-common:jar:4.1.45.Final:compile
[INFO] |  |  +- io.netty:netty-buffer:jar:4.1.45.Final:compile
[INFO] |  |  \- io.netty:netty-codec:jar:4.1.45.Final:compile
[INFO] |  +- io.netty:netty-handler:jar:4.1.45.Final:compile
[INFO] |  \- io.netty:netty-transport:jar:4.1.45.Final:compile
[INFO] |     \- io.netty:netty-resolver:jar:4.1.45.Final:compile
[INFO] \- com.google.cloud:google-cloud-firestore:jar:1.34.0:compile
[INFO]    +- com.google.cloud:google-cloud-core-grpc:jar:1.93.5:compile
[INFO]    +- io.grpc:grpc-core:jar:1.29.0:compile
[INFO]    +- com.google.android:annotations:jar:4.1.1.4:compile
[INFO]    +- io.perfmark:perfmark-api:jar:0.19.0:compile
[INFO]    +- org.apache.httpcomponents:httpclient:jar:4.5.12:compile
[INFO]    +- commons-logging:commons-logging:jar:1.2:compile
[INFO]    +- commons-codec:commons-codec:jar:1.11:compile
[INFO]    +- org.apache.httpcomponents:httpcore:jar:4.4.13:compile
[INFO]    +- io.opencensus:opencensus-contrib-http-util:jar:0.24.0:compile
[INFO]    +- com.google.api.grpc:proto-google-cloud-firestore-admin-v1:jar:1.34.0:compile
[INFO]    +- com.google.api.grpc:proto-google-cloud-firestore-v1:jar:1.34.0:compile
[INFO]    +- com.google.api.grpc:proto-google-cloud-firestore-v1beta1:jar:0.87.0:compile
[INFO]    +- com.google.auto.value:auto-value-annotations:jar:1.7.2:compile
[INFO]    +- io.opencensus:opencensus-contrib-grpc-util:jar:0.24.0:compile
[INFO]    +- com.google.code.findbugs:jsr305:jar:3.0.2:compile
[INFO]    +- javax.annotation:javax.annotation-api:jar:1.3.2:compile
[INFO]    +- io.grpc:grpc-protobuf:jar:1.29.0:compile
[INFO]    +- io.grpc:grpc-protobuf-lite:jar:1.29.0:compile
[INFO]    +- io.grpc:grpc-context:jar:1.29.0:compile
[INFO]    +- com.google.protobuf:protobuf-java:jar:3.12.2:compile
[INFO]    +- com.google.api.grpc:proto-google-common-protos:jar:1.18.0:compile
[INFO]    +- com.google.api:gax:jar:1.56.0:compile
[INFO]    +- io.grpc:grpc-api:jar:1.29.0:compile
[INFO]    +- com.google.errorprone:error_prone_annotations:jar:2.3.4:compile
[INFO]    +- org.codehaus.mojo:animal-sniffer-annotations:jar:1.18:compile
[INFO]    +- com.google.api:gax-grpc:jar:1.56.0:compile
[INFO]    +- io.grpc:grpc-auth:jar:1.29.0:compile
[INFO]    +- io.grpc:grpc-netty-shaded:jar:1.29.0:compile
[INFO]    +- io.grpc:grpc-alts:jar:1.29.0:compile
[INFO]    +- io.grpc:grpc-grpclb:jar:1.29.0:compile
[INFO]    +- org.apache.commons:commons-lang3:jar:3.5:compile
[INFO]    +- org.conscrypt:conscrypt-openjdk-uber:jar:2.2.1:compile
[INFO]    +- com.google.guava:failureaccess:jar:1.0.1:compile
[INFO]    +- com.google.guava:listenablefuture:jar:9999.0-empty-to-avoid-conflict-with-guava:compile
[INFO]    +- org.checkerframework:checker-compat-qual:jar:2.5.5:compile
[INFO]    +- com.google.j2objc:j2objc-annotations:jar:1.3:compile
[INFO]    +- org.threeten:threetenbp:jar:1.4.4:compile
[INFO]    +- io.grpc:grpc-stub:jar:1.29.0:compile
[INFO]    +- io.opencensus:opencensus-api:jar:0.24.0:compile
[INFO]    +- com.google.auth:google-auth-library-credentials:jar:0.20.0:compile
[INFO]    +- com.google.cloud:google-cloud-core:jar:1.93.5:compile
[INFO]    +- com.google.api.grpc:proto-google-iam-v1:jar:0.13.0:compile
[INFO]    +- com.google.http-client:google-http-client-jackson2:jar:1.35.0:compile
[INFO]    +- com.google.code.gson:gson:jar:2.8.6:compile
[INFO]    +- com.fasterxml.jackson.core:jackson-core:jar:2.11.0:compile
[INFO]    \- com.google.protobuf:protobuf-java-util:jar:3.12.2:compile
theHilikus commented 4 years ago

ok. if you say it's ok to update google-cloud-firestore from 1.31.0 to 1.34.0 in firebase-admin 6.13, i will do it. i didn't want to take that risk since i don't know the code enough to assess the risk. Thank you for your help @hiranya911

hiranya911 commented 4 years ago

Feel free to always use the latest minor and patch versions of Firestore and Storage.

Psijic commented 1 year ago

implementation ("com.google.firebase:firebase-admin:9.1.1") Provides transitive vulnerable dependency maven:io.netty:netty-codec:4.1.84.Final