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

Upgrade jsch from 0.1.52 to 0.1.55 to fix CVE-2016-5725 #281

Merged
merged 1 commit into from
Dec 13, 2024

Conversation

Dilli-Babu-Godari
Copy link
Contributor

Description

Upgrade jsch from 0.1.52 to 0.1.55 to fix CVE-2016-5725

Upgrading the jsch library from version 0.1.52 to 0.1.55 addresses CVE-2016-5725, which is a directory traversal vulnerability in JCraft JSch before version 0.1.54. This vulnerability allowed remote SFTP servers to write to arbitrary files on Windows when using ChannelSftp.OVERWRITE mode, via a ..\ (dot dot backslash) in a recursive GET command.

Build Successful.
Screenshot 2024-12-05 at 2 11 34 PM

Copy link

linux-foundation-easycla bot commented Dec 5, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: Dilli-Babu-Godari (95ca2c5)

@Dilli-Babu-Godari Dilli-Babu-Godari force-pushed the CVE_OSS_Jsch branch 4 times, most recently from 4ddfaef to 73684e1 Compare December 9, 2024 09:43
@aaneja
Copy link
Contributor

aaneja commented Dec 10, 2024

I could pull this locally and build & run tests. We need to have a release pipeline to publish updated artifacts.
I'm not sure how we've done this in the past, but I see there were signing + upload steps added in #3

If we cannot dig through history to figure out how we did releases, I do have a branch which uses a more up-to-date gradle that I think we should switch to for releases

@Dilli-Babu-Godari
Copy link
Contributor Author

Dilli-Babu-Godari commented Dec 10, 2024

I could pull this locally and build & run tests. We need to have a release pipeline to publish updated artifacts. I'm not sure how we've done this in the past, but I see there were signing + upload steps added in #3

If we cannot dig through history to figure out how we did releases, I do have a branch which uses a more up-to-date gradle that I think we should switch to for releases

For Reference: Dilli-Babu-Godari#2

Screenshot 2024-12-10 at 12 48 37 PM

I tried adding build scripts on GitHub following this PR: https://github.com/prestodb/airlift/pull/77/files in my forked repository, as suggested by Tim Meehan. However, I'm encountering a build failure. The build is failing at the signingArchives task for the Java 8 build, and I'm currently investigating the issue.

Screenshot 2024-12-10 at 12 38 52 PM

I also noticed a lot of warnings while using Java 8, and during the Java 11 compilation, it failed at the 'compileJava' task with a couple of errors.

Screenshot 2024-12-10 at 12 45 57 PM If you have any insights on this, please let me know.

@ZacBlanco
Copy link

@Dilli-Babu-Godari for both CI in the matrix you should set skipSigning to false in the gradle properties. The example property is defined in the sample gradle.properties in the root of the repository. You might need an extra step in the CI script to generate the file, or you might be able to pass it on the command line with something like -PskipSigning=true or -DskipSigning=true

@ZacBlanco
Copy link

ZacBlanco commented Dec 10, 2024

Also regarding the Java 11 compatibility. I don't think we need to compile to Java 11 bytecode. We probably just need it to compile to java 8 and for the tests to pass if executed with Java 11. The gradle build may need to be updated for that, but I'm not sure.

@Dilli-Babu-Godari
Copy link
Contributor Author

Also regarding the Java 11 compatibility. I don't think we need to compile to Java 11 bytecode. We probably just need it to compile to java 8 and for the tests to pass if executed with Java 11. The gradle build may need to be updated for that, but I'm not sure.

I will try and check with Java 8 for now.

@Dilli-Babu-Godari
Copy link
Contributor Author

@Dilli-Babu-Godari for both CI in the matrix you should set skipSigning to false in the gradle properties. The example property is defined in the sample gradle.properties in the root of the repository. You might need an extra step in the CI script to generate the file, or you might be able to pass it on the command line with something like -PskipSigning=true or -DskipSigning=true

@ZacBlanco I've implemented the changes based on your feedback and have raised a separate PR for them. You can find the PR here: #282. Could you please take a look when you have a moment?

@ZacBlanco
Copy link

@Dilli-Babu-Godari can you force-push again to this branch to trigger the new GH action

@Dilli-Babu-Godari Dilli-Babu-Godari force-pushed the CVE_OSS_Jsch branch 2 times, most recently from 49398ad to 3ee9480 Compare December 13, 2024 18:41
@Dilli-Babu-Godari
Copy link
Contributor Author

@Dilli-Babu-Godari can you force-push again to this branch to trigger the new GH action

I've rebased with master and force-pushed the changes to the branch. Everything is working as expected, and the check has successfully passed.

Copy link

@ZacBlanco ZacBlanco left a comment

Choose a reason for hiding this comment

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

last thing, otherwise lgtm

.gitignore Outdated
@@ -1,6 +1,6 @@
# Build files
.gradle/
gradle.properties
!gradle.properties

Choose a reason for hiding this comment

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

looks like this got added in again, please remove

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed this. Could you please merge the change if everything looks fine ?

Upgrading the jsch library from version 0.1.52 to 0.1.55 addresses
CVE-2016-5725, which is a directory traversal vulnerability in
JCraft JSch before version 0.1.54. This vulnerability allowed remote
SFTP servers to write to arbitrary files on Windows when using
ChannelSftp.OVERWRITE mode, via a ..\ (dot dot backslash) in a
recursive GET command.
@ZacBlanco
Copy link

I don't have permissions. @tdcmeehan should be able to merge this

@tdcmeehan tdcmeehan merged commit eb9d16c into prestodb:master Dec 13, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants