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

HIVE-28040: Upgrade netty to 4.1.116.Final due to CVEs #5592

Merged
merged 7 commits into from
Jan 7, 2025

Conversation

tanishq-chugh
Copy link
Contributor

@tanishq-chugh tanishq-chugh commented Dec 29, 2024

What changes were proposed in this pull request?

Upgrade Netty due to CVEs

Why are the changes needed?

Fixing CVEs

Does this PR introduce any user-facing change?

No

Is the change a dependency upgrade?

Yes
Dependency tree:
dpn_netty_116.txt

How was this patch tested?

Manual Testing by running few queries after local compilation

pom.xml Outdated
@@ -110,7 +110,7 @@
<antlr4.version>4.9.3</antlr4.version>
<apache-directory-server.version>1.5.7</apache-directory-server.version>
<!-- Include arrow for LlapOutputFormatService -->
<arrow.version>12.0.0</arrow.version>
<arrow.version>16.1.0</arrow.version>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this Apache Arrow is compiled by a different JDK version

[2024-12-29T18:31:23.075Z] Caused by: java.lang.NoSuchMethodError: java.nio.ByteBuffer.position(I)Ljava/nio/ByteBuffer;
[2024-12-29T18:31:23.075Z] 	at org.apache.arrow.memory.ArrowBuf.setBytes(ArrowBuf.java:850)
[2024-12-29T18:31:23.075Z] 	at io.netty.buffer.NettyArrowBuf.setBytes(NettyArrowBuf.java:278)
[2024-12-29T18:31:23.075Z] 	at io.netty.buffer.MutableWrappedByteBuf.setBytes(MutableWrappedByteBuf.java:341)
[2024-12-29T18:31:23.075Z] 	at io.netty.buffer.ExpandableByteBuf.setBytes(ExpandableByteBuf.java:27)
[2024-12-29T18:31:23.075Z] 	at io.netty.buffer.AbstractByteBuf.writeBytes(AbstractByteBuf.java:1113)

Copy link
Contributor Author

@tanishq-chugh tanishq-chugh Dec 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@okumin yes, you are right. I was looking into the same and it seems arrow v16.1.0+ is compiled with jdk9.
Have changed the arrow target version to 15.0.0 in cd3eb8f which seems compatible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my bad, arrow v15.0.0 was producing some license-related issues causing compilation failure.
Instead, have upgraded to v16.0.0

This reverts commit 5de7b98.
pom.xml Outdated
<netty.version>4.1.77.Final</netty.version>
<netty.version>4.1.108.Final</netty.version>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

netty 4.1.116 is around, we should upgrade to latest IMO
https://mvnrepository.com/artifact/io.netty/netty-all/4.1.116.Final

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CVE-2024-47535
this CVE is fixed in 4.1.115, It will be good that if we moved it to 4.1.116 as @ayushtkn suggested. Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for checking this out.

@basapuram-kumar
Copy link

@tanishq-chugh
Could you please also change version in the title of the PR to "4.1.116.Final"

Copy link

sonarqubecloud bot commented Jan 6, 2025

Copy link
Member

@ayushtkn ayushtkn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ayushtkn ayushtkn changed the title HIVE-28040: Upgrade netty to 4.1.108 due to CVEs HIVE-28040: Upgrade netty to 4.1.116.Final due to CVEs Jan 7, 2025
@basapuram-kumar
Copy link

LGTM

@ayushtkn ayushtkn merged commit cf21731 into apache:master Jan 7, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants