Skip to content

[Low] Patch reaper for CVE-2024-6484 #13636

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

[Low] Patch reaper for CVE-2024-6484 #13636

wants to merge 2 commits into from

Conversation

kevin-b-lockwood
Copy link
Contributor

Merge Checklist

All boxes should be checked before merging the PR (just tick any boxes which don't apply to this PR)

  • The toolchain has been rebuilt successfully (or no changes were made to it)
  • The toolchain/worker package manifests are up-to-date
  • Any updated packages successfully build (or no packages were changed)
  • Packages depending on static components modified in this PR (Golang, *-static subpackages, etc.) have had their Release tag incremented.
  • Package tests (%check section) have been verified with RUN_CHECK=y for existing SPEC files, or added to new SPEC files
  • All package sources are available
  • cgmanifest files are up-to-date and sorted (./cgmanifest.json, ./toolkit/scripts/toolchain/cgmanifest.json, .github/workflows/cgmanifest.json)
  • LICENSE-MAP files are up-to-date (./SPECS/LICENSES-AND-NOTICES/data/licenses.json, ./SPECS/LICENSES-AND-NOTICES/LICENSES-MAP.md, ./SPECS/LICENSES-AND-NOTICES/LICENSE-EXCEPTIONS.PHOTON)
  • All source files have up-to-date hashes in the *.signatures.json files
  • sudo make go-tidy-all and sudo make go-test-coverage pass
  • Documentation has been updated to match any changes to the build system
  • If you are adding/removing a .spec file that has multiple-versions supported, please add @microsoft/cbl-mariner-multi-package-reviewers team as reviewer (Eg. golang has 2 versions 1.18, 1.21+)
  • Ready to merge
Summary

Patch CVE-2024-6484 in bootstrap dependency of reaper

Change Log
  • Add patch for CVE-2024-6484
  • Remove some unused but vulnerable files in bootstrap dependency.
Does this affect the toolchain?

NO

Links to CVEs
Test Methodology
  • Local build

@kevin-b-lockwood kevin-b-lockwood requested a review from a team as a code owner April 29, 2025 22:35
kgodara912
kgodara912 reviewed May 2, 2025
View reviewed changes
Copy link
Contributor

@kgodara912 kgodara912 left a comment

Choose a reason for hiding this comment

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

Buddy Build

@Kanishk-Bansal Kanishk-Bansal requested a review from kgodara912 May 14, 2025 19:59
kgodara912
kgodara912 approved these changes May 21, 2025
View reviewed changes
Copy link
Contributor

@kgodara912 kgodara912 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Buddy build passes except in arm installation as package is exclusive for x86: spec file has ExclusiveArch: x86_64

clean up
+++ kill running container - 834914d328f955216dcc91a7481597497eb43ae37c35f433df841916ec7c6e5b
Error response from daemon: Cannot kill container: 834914d328f955216dcc91a7481597497eb43ae37c35f433df841916ec7c6e5b: Container 834914d328f955216dcc91a7481597497eb43ae37c35f433df841916ec7c6e5b is not running

which is killing a not running container which should be fine. Patch matches with reference except map files which are not part of CVE affected files. These two files

rm src/ui/bower_components/bootstrap/dist/js/bootstrap.min.js
rm src/ui/node_modules/bootstrap/dist/js/bootstrap.min.js

do not seem to be used as package compiles fine. LGTM.

0xba1a
0xba1a requested changes May 30, 2025
View reviewed changes
SPECS/reaper/reaper.spec Outdated
@@ -49,6 +49,8 @@ Patch13: CVE-2024-52798.patch
Patch14: CVE-2020-24025.patch
Patch15: CVE-2024-28863.patch
Patch16: CVE-2024-12905.patch
# CVE-2024-6484 is fixed in bootstrap version 3.4.2 by https://github.com/odinserj/bootstrap/commit/0ea568be7ff0c1f72a693f5d782277a9e9872077
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to capture the upstream reference in the SPEC file

SPECS/reaper/CVE-2024-6484.patch Outdated
Subject: [PATCH] Fix CVE-2024-6484 vulnerability by disabling further event
handling

Link: https://github.com/odinserj/bootstrap/commit/0ea568be7ff0c1f72a693f5d782277a9e9872077
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use "Upstream" keyword to specify that it is the upstream reference of the backported patch

SPECS/reaper/reaper.spec
@@ -114,7 +116,12 @@ popd
pushd $tmp_local_dir/n/versions/node/14.18.0/lib/node_modules/
%autopatch -p1 15
popd
%autopatch -p1 16
%autopatch -p1 -m 16
Copy link
Contributor

Choose a reason for hiding this comment

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

Good usage of -m option

@kevin-b-lockwood kevin-b-lockwood requested a review from 0xba1a June 3, 2025 23:30
@kevin-b-lockwood
Copy link
Contributor Author

I believe I have addressed all points raised, thank you for the feedback.

@kevin-b-lockwood
Copy link
Contributor Author

@0xba1a Please let me know if there's something more I must do on this PR.

@Kanishk-Bansal Kanishk-Bansal mentioned this pull request Jun 9, 2025
Merged
12 tasks
@jykanase
Copy link
Contributor

jykanase commented Jun 10, 2025 •
edited
Loading

Adding this PR changes to #13965 to avoid conflict.

@kevin-b-lockwood
Copy link
Contributor Author

This work has been incorporated into #13965, so there's no need for this PR anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kgodara912 kgodara912

@0xba1a 0xba1a

Assignees
No one assigned
Labels
main PR Destined for main Packaging security
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@kevin-b-lockwood @jykanase @0xba1a @kgodara912 @Kanishk-Bansal