presto icon indicating copy to clipboard operation
presto copied to clipboard
verified publisher icon prestodb

Fix PRISMA-2023-0067 upgrade jackson core to 2.15.4

Open Mariamalmesfer opened this issue 1 year ago • 12 comments

Description

fixes the jackson-core security vulnerability PRISMA-2023-0067 by upgrading from version 2.11.0 to 2.15.4 PRISMA-2023-0067: Details

Motivation and Context

Reasons for upgradation: This PR upgrades jackson-core to address a Denial of Service (DoS) vulnerability caused by improper input validation during numeric type conversions. This upgrade prevents resource exhaustion by ensuring proper input validation.

Impact

Image Scan showed the vulnerability have been removed. Image scan report : Image-scan-report-9 Sep.csv

Test Plan

Contributor checklist

  • [x] Please make sure your submission complies with our development, formatting, commit message, and attribution guidelines.
  • [x] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
  • [ ] Documented new properties (with its default value), SQL syntax, functions, or other functionality.
  • [ ] If release notes are required, they follow the release notes guidelines.
  • [ ] Adequate tests were added if applicable.
  • [ ] CI passed.

Release Notes

 == RELEASE NOTES == =

Security Changes
* Upgrade Jackson-core to 2.15.4 :pr:`#23753`

Mariamalmesfer avatar Oct 01 '24 09:10 Mariamalmesfer

CLA Signed

The committers listed above are authorized under a signed CLA.

  • :white_check_mark: login: Mariamalmesfer / name: Mariam AlMesfer (fd9be630bee80ab650a440ac6a102f8730c63093)

linux-foundation-easycla[bot] avatar Oct 01 '24 09:10 linux-foundation-easycla[bot]

I know @steveburnett disagrees with me about this, but I really don't think we should be including dependency upgrades in the release notes.

elharo avatar Oct 01 '24 13:10 elharo

Looks like the most recent version is 2.18.0

elharo avatar Oct 01 '24 19:10 elharo

Looks like the most recent version is 2.18.0

The CVS scan suggests upgrading to version 2.15.0, as that's where the vulnerability was fixed. But if we want the latest updates, we can go with 2.18.0. Let me know your preference

Mariamalmesfer avatar Oct 03 '24 17:10 Mariamalmesfer

@Mariamalmesfer @elharo Maybe we can update to the one version before the very latest which is 2.17.2 what do you think?

agrawalreetika avatar Oct 07 '24 09:10 agrawalreetika

@Mariamalmesfer @elharo Maybe we can update to the one version before the very latest which is 2.17.2 what do you think?

Sounds good to me. We can go with 2.17.2 if that works for everyone. cc: @elharo

Mariamalmesfer avatar Oct 07 '24 10:10 Mariamalmesfer

Please update the release note entry to the version being updated to. For example

 == RELEASE NOTES == =

Security Changes
* Upgrade Jackson-core to 2.17.2 :pr:`#23753`

steveburnett avatar Oct 07 '24 13:10 steveburnett

I prefer 2.18.0 but I'm not sure others agree with this. Don't overthink it. Either 2.17.2 or 2.18 is better than what we have now.

elharo avatar Oct 10 '24 15:10 elharo

I prefer 2.18.0 but I'm not sure others agree with this. Don't overthink it. Either 2.17.2 or 2.18 is better than what we have now.

It seems we've all agreed on upgrading to Jackson-core version 2.17.2. Could you please give your approval so we can proceed?

CC: @tdcmeehan @agrawalreetika @elharo

Mariamalmesfer avatar Oct 14 '24 12:10 Mariamalmesfer

We need to figure out if the test failures are related or just flaky. I've rerun a couple of times so they might be related. Either way, we need to get them passing.

elharo avatar Oct 14 '24 13:10 elharo

Please squash commits.

tdcmeehan avatar Oct 16 '24 16:10 tdcmeehan

We need to figure out if the test failures are related or just flaky. I've rerun a couple of times so they might be related. Either way, we need to get them passing.

I took a look at why the test was failing, and it turned out that the higher Jackson version was causing the issue. I downgraded to Jackson version 2.15.4 to resolve the test failures caused by the newer version. The issue seemed to stem from incompatibilities with the higher versions, but this downgrade allowed the test to pass successfully while still addressing the relevant CVE.

Mariamalmesfer avatar Oct 16 '24 18:10 Mariamalmesfer

How and why did the higher jackson version cause the failure? That might be something we should fix, possibly in this PR.

The upgrade to Jackson 2.17.2 caused issues during testing. The main problem occurred with the TestDriver class not running due to a serialization error involving the CanonicalPlan. A similar issue also happened when using Jackson 2.16.0.

There were additional warnings related to configuration problems during server startup, which contributed to the test failures. The tests couldn't run properly as a result.

By downgrading to Jackson 2.15.4, the tests passed successfully, and the warnings were resolved. This version fixed the test failures and compatibility problems while still addressing the security vulnerability (PRISMA-2023-0067). The remaining test failure in Presto-Hive, related to materialized views, seems flaky to me.

Mariamalmesfer avatar Oct 21 '24 16:10 Mariamalmesfer

LGTM but please fix this typo in the commit message: [Upgraded jakcson-core to 2.15.4](https://github.com/prestodb/presto/pull/23753/commits/7b94b0459c7024b8e3880a06dbb3dbcecc49d8a0). jakcson->jackson

tdcmeehan avatar Oct 28 '24 19:10 tdcmeehan

Nit I didn't notice earlier, please remove the # from the release note entry

 == RELEASE NOTES == =

Security Changes
* Upgrade Jackson-core to 2.15.4. :pr:`23753`

steveburnett avatar Nov 04 '24 15:11 steveburnett

LGTM I think the entry in release note should be changed since you are not just upgrading jackson-core here cc @steveburnett

I agree @agrawalreetika, thank you!

@Mariamalmesfer , please update the release note entry to describe the other changes you are making. Also, please include at least one of the CVEs addressed by this PR. For formatting of the link to the CVE, see the example in Phrasing in the Release Note Guidelines. Thanks!

steveburnett avatar Dec 02 '24 14:12 steveburnett

two nits in the release note entry, looks good otherwise. Suggested edits below:

== RELEASE NOTES == =

Security Changes

  • Upgrade Jackson dependencies to 2.15.4 in response to PRISMA-2023-0067 <https://www.ibm.com/support/pages/security-bulletin-vulnerability-jackson-core-might-affect-ibm-business-automation-workflow-prisma-2023-0067>_. :pr:23753

steveburnett avatar Dec 03 '24 17:12 steveburnett

Thanks for the release note entry update! Some minor nits of formatting in the release note entry - my bad, I should have put ``` before and after the draft in my previous comment to display the single `s in the draft.

 == RELEASE NOTES == =

Security Changes
* Upgrade Jackson dependencies to 2.15.4 in response to `PRISMA-2023-0067 <https://www.ibm.com/support/pages/security-bulletin-vulnerability-jackson-core-might-affect-ibm-business-automation-workflow-prisma-2023-0067>`_. :pr:`23753`

steveburnett avatar Dec 04 '24 15:12 steveburnett

Thanks for the release note entry update! Some minor nits of formatting in the release note entry - my bad, I should have put ``` before and after the draft in my previous comment to display the single `s in the draft.

 == RELEASE NOTES == =

Security Changes
* Upgrade Jackson dependencies to 2.15.4 in response to `PRISMA-2023-0067 <https://www.ibm.com/support/pages/security-bulletin-vulnerability-jackson-core-might-affect-ibm-business-automation-workflow-prisma-2023-0067>`_. :pr:`23753`

Thank you @steveburnett!

Mariamalmesfer avatar Dec 04 '24 16:12 Mariamalmesfer