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

CVE-2023-3635 com.squareup.okio:okio-jvm:jar:3.0.0 #5637

Closed
patpatpat123 opened this issue Jul 19, 2023 · 16 comments
Closed

CVE-2023-3635 com.squareup.okio:okio-jvm:jar:3.0.0 #5637

patpatpat123 opened this issue Jul 19, 2023 · 16 comments
Labels
Bug Something isn't working needs author feedback Waiting for additional feedback from the author

Comments

@patpatpat123
Copy link

Describe the bug
Hello team,

I have a project where I have this dependency:

        <dependency>
            <groupId>io.opentelemetry</groupId>
            <artifactId>opentelemetry-exporter-otlp</artifactId>
        </dependency>

When running several static analysis scanners, such as dependency-check, sonarqube, blackduck, checkmarx, etc, all of them recognize this package as vulnerable, because it is pulling com.squareup.okio:okio-jvm:jar:3.0.0

Please see the dependency tree:

[INFO] +- io.opentelemetry:opentelemetry-exporter-otlp:jar:1.25.0:compile
[INFO] |  +- io.opentelemetry:opentelemetry-sdk-metrics:jar:1.25.0:compile
[INFO] |  +- io.opentelemetry:opentelemetry-exporter-otlp-common:jar:1.25.0:runtime
[INFO] |  |  +- io.opentelemetry:opentelemetry-exporter-common:jar:1.25.0:runtime
[INFO] |  |  \- com.squareup.okhttp3:okhttp:jar:4.10.0:runtime
[INFO] |  |     +- com.squareup.okio:okio-jvm:jar:3.0.0:runtime   // <----- HERE
[INFO] |  |     |  +- org.jetbrains.kotlin:kotlin-stdlib-jdk8:jar:1.8.22:runtime
[INFO] |  |     |  |  \- org.jetbrains.kotlin:kotlin-stdlib-jdk7:jar:1.8.22:runtime
[INFO] |  |     |  \- org.jetbrains.kotlin:kotlin-stdlib-common:jar:1.8.22:runtime
[INFO] |  |     \- org.jetbrains.kotlin:kotlin-stdlib:jar:1.8.22:runtime
[INFO] |  |        \- org.jetbrains:annotations:jar:13.0:runtime
[INFO] |  \- io.opentelemetry:opentelemetry-sdk-extension-autoconfigure-spi:jar:1.25.0:runtime

Steps to reproduce
Run a vulnerability scanner, such as dependency-check, sonarqube, blackduck, checkmarx, on io.opentelemetry:opentelemetry-exporter-otlp:jar:1.25.0

What did you expect to see?
I hope to not see any vulnerabilities

What did you see instead?
A CVE

What version and what artifacts are you using?
1.25.0

Environment
Compiler: (e.g., "Temurin 17.0.7") GraalVM 17

Additional context
This is my first issue here, hope this is not a trouble.
I wanted to raise the issue on opentelemetry-exporter-otlp but could not find the repo.
If not anything, thank you for your time

@patpatpat123 patpatpat123 added the Bug Something isn't working label Jul 19, 2023
@mateuszrzeszutek
Copy link
Member

Hey @patpatpat123 ,

OkHttp version has been updated to 4.11.0 in this commit, and it's been present since release 1.26.0. We recommend updating to the latest version (1.28.0).

@mateuszrzeszutek mateuszrzeszutek added the needs author feedback Waiting for additional feedback from the author label Jul 19, 2023
@trask
Copy link
Member

trask commented Jul 19, 2023

it looks like the CVE was just recently fixed in com.squareup.okio:okio:3.4.0 (square/okio#1280)

while com.squareup.okhttp3:okhttp:4.11.0 still pulls in com.squareup.okio:okio:3.2.0

@trask trask removed the needs author feedback Waiting for additional feedback from the author label Jul 19, 2023
@trask
Copy link
Member

trask commented Jul 24, 2023

I checked this again, and okio-bom was updated in #5613, which will be in next month's 1.29.0 release.

double checking against main, ./gradlew :exporters:otlp:all:dependencies shows all okio dependencies aligned to 3.4.0

@patpatpat123
Copy link
Author

Apologies, I do use opentelemetry-exporter-otlp, which has this vulnerable dependencies.
Does it means my action item is to find a opentelemetry-exporter-otlp version, which uses com.squareup.okio:okio:3.4.0?
May I ask if you know which version of opentelemetry-exporter-otlp uses com.squareup.okio:okio:3.4.0?

@trask
Copy link
Member

trask commented Jul 31, 2023

May I ask if you know which version of opentelemetry-exporter-otlp uses com.squareup.okio:okio:3.4.0?

the upcoming release, v1.29.0, currently scheduled for Aug 11

@patpatpat123
Copy link
Author

Understood, preparing ourselves for this day

@patpatpat123
Copy link
Author

Hello, it is now August 11th (at least in my current city). Still not able to get version 1.29.0.

Is it still expected?

@mateuszrzeszutek
Copy link
Member

Yes, the 1.29.0 will still be released today (during US day/EU eveninig)

@jack-berg
Copy link
Member

1.29.0 has been released!

If you're on java 11+, you might also consider the strategy discussed here which avoids the okhttp dependency altogether.

@patpatpat123
Copy link
Author

INFO] +- io.opentelemetry:opentelemetry-exporter-otlp:jar:1.29.0:compile
[INFO] |  +- io.opentelemetry:opentelemetry-sdk-metrics:jar:1.28.0:compile
[INFO] |  +- io.opentelemetry:opentelemetry-sdk-logs:jar:1.28.0:compile
[INFO] |  |  \- io.opentelemetry:opentelemetry-api-events:jar:1.28.0-alpha:runtime
[INFO] |  +- io.opentelemetry:opentelemetry-exporter-otlp-common:jar:1.28.0:runtime
[INFO] |  |  \- io.opentelemetry:opentelemetry-exporter-common:jar:1.28.0:runtime
[INFO] |  +- io.opentelemetry:opentelemetry-exporter-sender-okhttp:jar:1.28.0:runtime
[INFO] |  |  \- com.squareup.okhttp3:okhttp:jar:4.11.0:runtime
[INFO] |  |     +- com.squareup.okio:okio:jar:3.2.0:runtime
[INFO] |  |     |  \- com.squareup.okio:okio-jvm:jar:3.2.0:runtime
[INFO] |  |     +- org.jetbrains.kotlin:kotlin-stdlib:jar:1.9.0:runtime
[INFO] |  |     |  +- org.jetbrains.kotlin:kotlin-stdlib-common:jar:1.9.0:runtime
[INFO] |  |     |  \- org.jetbrains:annotations:jar:13.0:runtime
[INFO] |  |     \- org.jetbrains.kotlin:kotlin-stdlib-jdk8:jar:1.9.0:runtime
[INFO] |  |        \- org.jetbrains.kotlin:kotlin-stdlib-jdk7:jar:1.9.0:runtime

The issue still persists. What did I do wrong please?

@bestbeforetoday
Copy link

bestbeforetoday commented Aug 12, 2023

Something went wrong for you since you've got a mix of v1.29.0 and v1.28.0 OpenTelemetry dependencies. Even when you get things right the vulnerable dependency is not resolved though:

[INFO] +- io.opentelemetry:opentelemetry-exporter-otlp:jar:1.29.0:compile
[INFO] |  +- io.opentelemetry:opentelemetry-exporter-otlp-common:jar:1.29.0:runtime
[INFO] |  |  \- io.opentelemetry:opentelemetry-exporter-common:jar:1.29.0:runtime
[INFO] |  +- io.opentelemetry:opentelemetry-exporter-sender-okhttp:jar:1.29.0:runtime
[INFO] |  |  \- com.squareup.okhttp3:okhttp:jar:4.11.0:runtime
[INFO] |  |     +- com.squareup.okio:okio:jar:3.2.0:runtime
[INFO] |  |     |  \- com.squareup.okio:okio-jvm:jar:3.2.0:runtime
[INFO] |  |     +- org.jetbrains.kotlin:kotlin-stdlib:jar:1.6.20:runtime
[INFO] |  |     |  +- org.jetbrains.kotlin:kotlin-stdlib-common:jar:1.6.20:runtime
[INFO] |  |     |  \- org.jetbrains:annotations:jar:13.0:runtime
[INFO] |  |     \- org.jetbrains.kotlin:kotlin-stdlib-jdk8:jar:1.6.20:runtime
[INFO] |  |        \- org.jetbrains.kotlin:kotlin-stdlib-jdk7:jar:1.6.20:runtime
[INFO] |  \- io.opentelemetry:opentelemetry-sdk-extension-autoconfigure-spi:jar:1.29.0:compile

OpenTelemetry depends on okhttp:4.11.0, but this still has a dependency on the vulnerable okio:3.2.0. See the okhttp POM file published to Maven Central. CVE-2023-3635 reports that the vulnerability is fixed in okio:3.4.0, and this version is not the one resolved due to the okhttp dependency declarations in their POM file.

This comment on a related okhttp issue suggests it will be resolved with the okhttp:4.12.0 release, but they do not seem to be in any rush to release.

@jack-berg
Copy link
Member

I see something different @bestbeforetoday. When I run ./gradlew :exporters:otlp:all:dependencies I get:

runtimeClasspath - Runtime classpath of source set 'main'.
+--- project :sdk:trace
|    +--- project :api:all
|    |    \--- project :context
|    +--- project :sdk:common
|    |    +--- project :api:all (*)
|    |    \--- project :semconv
|    |         \--- project :api:all (*)
|    \--- project :semconv (*)
+--- project :sdk:metrics
|    +--- project :api:all (*)
|    +--- project :sdk:common (*)
|    \--- project :extensions:incubator
|         \--- project :api:all (*)
+--- project :sdk:logs
|    +--- project :api:all (*)
|    +--- project :sdk:common (*)
|    \--- project :api:events
|         \--- project :api:all (*)
+--- project :dependencyManagement
|    +--- com.fasterxml.jackson:jackson-bom:{strictly 2.15.2} -> 2.15.2
|    +--- com.google.guava:guava-bom:{strictly 32.1.2-jre} -> 32.1.2-jre
|    +--- com.google.protobuf:protobuf-bom:{strictly 3.24.0} -> 3.24.0
|    +--- com.linecorp.armeria:armeria-bom:{strictly 1.24.3} -> 1.24.3
|    +--- com.squareup.okhttp3:okhttp-bom:{strictly 4.11.0} -> 4.11.0
|    |    \--- com.squareup.okhttp3:okhttp:4.11.0 (c)
|    +--- com.squareup.okio:okio-bom:{strictly 3.5.0} -> 3.5.0
|    |    +--- com.squareup.okio:okio:3.5.0 (c)
|    |    \--- com.squareup.okio:okio-jvm:3.5.0 (c)
|    +--- io.grpc:grpc-bom:{strictly 1.57.1} -> 1.57.1
|    +--- io.netty:netty-bom:{strictly 4.1.96.Final} -> 4.1.96.Final
|    +--- io.zipkin.brave:brave-bom:{strictly 5.16.0} -> 5.16.0
|    +--- io.zipkin.reporter2:zipkin-reporter-bom:{strictly 2.16.4} -> 2.16.4
|    +--- org.assertj:assertj-bom:{strictly 3.24.2} -> 3.24.2
|    +--- org.junit:junit-bom:{strictly 5.10.0} -> 5.10.0
|    +--- org.testcontainers:testcontainers-bom:{strictly 1.18.3} -> 1.18.3
|    \--- org.snakeyaml:snakeyaml-engine:{strictly 2.6} -> 2.6
+--- project :exporters:otlp:common
|    \--- project :exporters:common
|         \--- project :api:all (*)
+--- project :exporters:sender:okhttp
|    +--- project :exporters:common (*)
|    +--- project :sdk:common (*)
|    \--- com.squareup.okhttp3:okhttp -> 4.11.0
|         +--- com.squareup.okio:okio:3.2.0 -> 3.5.0
|         |    \--- com.squareup.okio:okio-jvm:3.5.0
|         |         +--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.9.0
|         |         |    +--- org.jetbrains.kotlin:kotlin-stdlib:1.9.0
|         |         |    |    +--- org.jetbrains.kotlin:kotlin-stdlib-common:1.9.0
|         |         |    |    \--- org.jetbrains:annotations:13.0
|         |         |    \--- org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.9.0
|         |         |         \--- org.jetbrains.kotlin:kotlin-stdlib:1.9.0 (*)
|         |         \--- org.jetbrains.kotlin:kotlin-stdlib-common:1.9.0
|         +--- org.jetbrains.kotlin:kotlin-stdlib:1.6.20 -> 1.9.0 (*)
|         \--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.6.20 -> 1.9.0 (*)
\--- project :sdk-extensions:autoconfigure-spi
     \--- project :sdk:all
          +--- project :api:all (*)
          +--- project :sdk:common (*)
          +--- project :sdk:trace (*)
          +--- project :sdk:metrics (*)
          \--- project :sdk:logs (*)

But when I run ./gradlew dependencies from a project that depends on opentelemetry-exporter-otlp, I do indeed see the 3.2.0 dependency:

+--- io.opentelemetry:opentelemetry-exporter-otlp -> 1.29.0
|    +--- io.opentelemetry:opentelemetry-exporter-otlp-common:1.29.0
|    |    \--- io.opentelemetry:opentelemetry-exporter-common:1.29.0
|    |         \--- io.opentelemetry:opentelemetry-api:1.29.0 (*)
|    +--- io.opentelemetry:opentelemetry-exporter-sender-okhttp:1.29.0
|    |    +--- io.opentelemetry:opentelemetry-exporter-common:1.29.0 (*)
|    |    +--- io.opentelemetry:opentelemetry-sdk-common:1.29.0 (*)
|    |    \--- com.squareup.okhttp3:okhttp:4.11.0
|    |         +--- com.squareup.okio:okio:3.2.0
|    |         |    \--- com.squareup.okio:okio-jvm:3.2.0

What's happening is that opentelemetry-java depends on okio-bom:3.5.0, but only uses this to resolve the version of okio dependencies internally where com.squareup.okhttp3:okhttp is not used. The result is that we resolve okio:3.5.0 internally, but projects depending on opentelemetry-exporter-otlp will resolve okio:3.2.0.

The easy solution is to add a dependency on com.squareup.okio:okio-bom:3.5.0 like we do internally:

implementation platform("com.squareup.okio:okio-bom:3.5.0")

I'm inclined to recommend this solution rather than publishing a patch with an updated transitive dependency version.

@bestbeforetoday
Copy link

The problem with that approach is that is requires all projects that depend on OpenTelemetry to explicitly implement this override (rather than just you doing it), and for all projects that depend on OpenTelemetry to have good enough test coverage of OpenTelemetry functionality that they can be confident it doesn't break anything in their environment (rather than just you testing your own project). It's a common approach amongst open source packages, but it's not a great one for consumers (including other open source packages).

@jack-berg
Copy link
Member

jack-berg commented Aug 13, 2023

Just did a little more investigation and determined that this CVE is not relevant to opentelemetry-exporter-otlp users. The description reads:

GzipSource does not handle an exception that might be raised when parsing a malformed gzip buffer. This may lead to denial of service of the Okio client when handling a crafted GZIP archive, by using the GzipSource class.

opentelemetry-exporter-otlp never uses GzipSource because we never try to open payloads from the server. opentelemetry-sdk-extension-jaeger-remote-sampler does need to process payloads from the server and does use GzipSource.

@trask
Copy link
Member

trask commented Aug 13, 2023

I think we're in a tough position here because okhttp itself hasn't made a new release that relies on the non-vulnerable okio yet (square/okhttp#7944 (comment)).

We've verified that the non-vulnerable okio version works within opentelemetry's usage patterns of okhttp (since the opentelemetry-java tests themselves are running against the non-vulnerable okio version via the okio-bom usage).

But if we pass along these non-aligned versions of okhttp and okio as transitive dependencies, we could potentially break people downstream who are using okhttp in other ways.

@bestbeforetoday
Copy link

bestbeforetoday commented Aug 14, 2023

I agree. You are in the same boat as all your dependent projects getting flagged up for this vulnerability. What is really needed is for okhttp to update their dependency on okio and release okhttp:4.12.0. Then you can safely base on that new okhttp version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working needs author feedback Waiting for additional feedback from the author
Projects
None yet
Development

No branches or pull requests

5 participants