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

fix: mitigate CVE-2023-28155, CVE-2023-26136 #25385

Merged
merged 1 commit into from
Oct 28, 2024

Conversation

coreydaley
Copy link
Contributor

@coreydaley coreydaley commented Jun 25, 2024

Hey, I just made a Pull Request!

This pull request builds off of #23320 and supersedes it, fixing the conflicts that it had with the main branch when it was abandoned.

This pull request updates the @kubernetes/client-node dependency from version 0.20.0 to 1.0.0-rc7. This update is critical as it addresses two known security vulnerabilities related to the request package, a dependency of @kubernetes/client-node, and its transitive dependency tough-cookie:

  1. CVE-2023-28155 - A vulnerability in the request package, which could potentially allow attackers to perform various attacks due to improper input validation.
  2. CVE-2023-26136 - A vulnerability in the tough-cookie package, a transitive dependency of request, which could allow attackers to craft cookies that may bypass intended security restrictions, leading to potential information disclosure or session hijacking.

By updating to @kubernetes/client-node version 1.0.0-rc7, these vulnerabilities are mitigated through the inclusion of patched versions of the affected packages. It is highly recommended to merge this pull request at the earliest to ensure the security of our application and protect against potential exploits leveraging these vulnerabilities.

Fixes #18742

Changes:

  • Updated @kubernetes/client-node from 0.20.0 to 1.0.0-rc7 in package.json.

Impact:

  • Addresses critical security vulnerabilities CVE-2023-28155 and CVE-2023-26136 related to the request and tough-cookie packages.
  • Enhances the security and stability of the application by using the latest, patched versions of dependencies.

Please review the changes and merge this pull request to enhance our application's security posture.

Thank you.

Built upon a contribution by Daniel Meyer (external expert on behalf of DB InfraGO) daniel.di.meyer-extern@deutschebahn.com

✔️ Checklist

  • A changeset describing the change and affected packages. (more info)
  • Added or updated documentation
  • Tests for new functionality and regression tests for bug fixes
  • Screenshots attached (for UI changes)
  • All your commits have a Signed-off-by line in the message. (more info)

@coreydaley coreydaley requested review from a team as code owners June 25, 2024 01:43
@github-actions github-actions bot added the area:kubernetes Related to the Kubernetes Project Area - not deploying Backstage with k8s. label Jun 25, 2024
@backstage-goalie
Copy link
Contributor

backstage-goalie bot commented Jun 25, 2024

Changed Packages

Package Name Package Path Changeset Bump Current Version
@backstage/plugin-kubernetes-backend plugins/kubernetes-backend minor v0.18.8-next.0
@backstage/plugin-kubernetes-common plugins/kubernetes-common minor v0.8.3
@backstage/plugin-kubernetes-node plugins/kubernetes-node minor v0.1.21-next.0
@backstage/plugin-kubernetes-react plugins/kubernetes-react minor v0.4.5-next.0
@backstage/plugin-kubernetes plugins/kubernetes minor v0.11.17-next.0

@backstage-goalie
Copy link
Contributor

Thanks for the contribution!
All commits need to be DCO signed before they are reviewed. Please refer to the the DCO section in CONTRIBUTING.md or the DCO status for more info.

@coreydaley coreydaley force-pushed the 2024-06-24-fix-cves branch from 4e8c0f9 to 3315285 Compare June 25, 2024 01:51
@coreydaley coreydaley changed the title Closing CVE-2023-28155, CVE-2023-26136 fix: mitigate CVE-2023-28155, CVE-2023-26136 Jun 25, 2024
@benjdlambert
Copy link
Member

@mclarke47 I know you're pretty slammed at the min, but is there any chance you could take a look and test this out? 🙏

@efenner-cambia
Copy link
Contributor

@coreydaley thanks for taking this on!

@mclarke47
Copy link
Collaborator

There seem to be some test failures in CI. I'll run this locally and do some basic manual testing

@coreydaley
Copy link
Contributor Author

Thank you @mclarke47 , if you figure it out please let me know and I can update my pull request.

@coreydaley coreydaley force-pushed the 2024-06-24-fix-cves branch from 014631f to 648e935 Compare June 26, 2024 03:42
@coreydaley coreydaley force-pushed the 2024-06-24-fix-cves branch 3 times, most recently from 6e37cd7 to 6f8e5e6 Compare July 10, 2024 21:08
@freben
Copy link
Member

freben commented Jul 17, 2024

This PR has a conflict now.

@joaquin386
Copy link

Any traction on the vulnerability fix?

@coreydaley coreydaley force-pushed the 2024-06-24-fix-cves branch from 179f4b2 to a103251 Compare August 21, 2024 00:27
@coreydaley
Copy link
Contributor Author

I am back working on this, the only part I have left is what was discussed in #25385 (review)
I am not a TypeScript guru so trying to reason my way through it :)

@coreydaley coreydaley force-pushed the 2024-06-24-fix-cves branch from a103251 to 3187b77 Compare August 21, 2024 01:13
@coreydaley
Copy link
Contributor Author

@mclarke47 I think I will just stick with this approach then. Looks like I need one more approving review before I can merge. Thanks!

@coreydaley
Copy link
Contributor Author

Verify Docs Quality failing due to #27154

@coreydaley
Copy link
Contributor Author

I have rebased (again) and all tests are passing if anyone has time for a second approving review. Thanks!

@mclarke47
Copy link
Collaborator

It needs another rebase 😞

@mclarke47
Copy link
Collaborator

Thank you for this amazing contribution btw! 🙏 Sorry it's been such a slog

@coreydaley
Copy link
Contributor Author

Thank you for this amazing contribution btw! 🙏 Sorry it's been such a slog

No problem, it's been a good learning experience and I appreciate all of the help along the way :)

I would like to get it merged soon though if possible ...

@mclarke47 mclarke47 enabled auto-merge (squash) October 21, 2024 18:57
@mclarke47
Copy link
Collaborator

I'll try to get one of the maintainers to look, when they come online STO time tomorrow.

@coreydaley
Copy link
Contributor Author

I'll try to get one of the maintainers to look, when they come online STO time tomorrow.

I would appreciate that, thank you!

Copy link
Member

@Rugvip Rugvip left a comment

Choose a reason for hiding this comment

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

Awesome, thank you! 😁

Just need to avoid mock-fs

plugins/kubernetes-backend/package.json Outdated Show resolved Hide resolved
auto-merge was automatically disabled October 28, 2024 01:29

Head branch was pushed to by a user without write access

@coreydaley coreydaley force-pushed the 2024-06-24-fix-cves branch 4 times, most recently from 485376c to bba7855 Compare October 28, 2024 02:02
Copy link
Member

@Rugvip Rugvip left a comment

Choose a reason for hiding this comment

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

Nice! 🎉

Just a small nit left

plugins/kubernetes-common/report.api.md Outdated Show resolved Hide resolved
Signed-off-by: Corey Daley <cdaley@redhat.com>
Copy link
Member

@Rugvip Rugvip left a comment

Choose a reason for hiding this comment

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

Awesome, let's :shipit:, thank you! 🎉

@Rugvip Rugvip enabled auto-merge October 28, 2024 12:45
@Rugvip Rugvip merged commit a096e9f into backstage:master Oct 28, 2024
30 checks passed
Copy link
Contributor

Thank you for contributing to Backstage! The changes in this pull request will be part of the 1.33.0 release, scheduled for Tue, 19 Nov 2024.

@meda1028
Copy link

meda1028 commented Nov 13, 2024

Hi coreydaley and team,
First of all, a huge huge thank you for getting this over the line.

Since you superseded the PR Description from PR(#23320), can you remove the "Contribution is Signed-off-by: Daniel Meyer (external expert on behalf of DB InfraGO) daniel.di.meyer-extern@deutschebahn.com" statement as this is not true anymore.
Mentioning it would be appreciated still.

@coreydaley
Copy link
Contributor Author

@meda1028 Updated, please let me know if that is ok or you would like further modifications.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:kubernetes Related to the Kubernetes Project Area - not deploying Backstage with k8s.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Snyk vulnerability [SNYK-JS-TOUGHCOOKIE-5672873]
9 participants