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: resolve CVE-2023-45857 in v0.x branch #6091

Merged
merged 4 commits into from Jan 19, 2024

Conversation

lnjbr
Copy link

@lnjbr lnjbr commented Nov 17, 2023

Fixes #6090

Used solutions from #6028 and #6046 to resolve the CVE-2023-45857 vulnerability in Axios 0.x

Breaking change:
axios('http://example.com/') will no longer set an XSRF token by default. To maintain old behavior, a truthy value for withXSRFToken must be passed. i.e. axios('http://example.com/') would need to be changed to something akin to:

axios('http://example.com/', {
   withXSRFToken: true
});

@levpachmanov
Copy link

Hey @lnjbr,
We're part of a startup called Seal Security that mitigates software vulnerabilities in older open source versions by backporting/creating standalone security patches - enabling more straightforward remediation in cases like this. We created an axios 0.27.2-sp1 that's vulnerability-free. As with all of our patches, it's open-source and available for free.

If relevant, check out our GitHub repo if you wish to learn more, or start using our app.

Please feel free to reach us at info@seal.security if you have any requests/questions.

@ashnewport
Copy link

Is there a release schedule which might indicate when this vulnerability fix will be released?

@bmuenzenmeyer
Copy link

@jasonsaayman @DigitalBrainJS sorry for the direct ping, but curious if this vulnerability will be addressed in 0.x?

@jasonsaayman
Copy link
Member

@jasonsaayman @DigitalBrainJS sorry for the direct ping, but curious if this vulnerability will be addressed in 0.x?

No probs yes I am very certain we should be fixing it in there too, will try get this out asap

@jasonsaayman
Copy link
Member

can we check the failing tests? @lnjbr

@lnjbr
Copy link
Author

lnjbr commented Dec 13, 2023

can we check the failing tests? @lnjbr

Yup! All set and ready for the workflows to be re-ran 🙇

@bmuenzenmeyer
Copy link

@jasonsaayman can the CI be re-ran here?

@ashnewport
Copy link

How do we trigger the CI here, and assuming green builds, what is the release cadence to main branch? Hoping to get some understand of times to assist with planning.

@bmuenzenmeyer
Copy link

bmuenzenmeyer commented Dec 22, 2023

How do we trigger the CI here

as a security feature, axios maintiainers have configured the repository to only run CI when they manually kick if off. It's a tactic to ensure forks do not introduce malicious code or try to steal secrets within GH Actions, for example

what is the release cadence to main branch

this is correctly targetting the 0.x branch, and is already on 1.X / main. meaning, it will never go to main, but presumably post-merge, an npm release for 0.27.3 or 0.28.0 will be cut.

@ashnewport
Copy link

Thanks for the info and running the CI! Much appreciated to get that feedback 👏

When I said main branch release cadence, what I probably should have asked is: "when will the changes be merged and released into their respective branch?", e.g. weekly release on a Monday to the targeted version, or ad hoc, or when certain thresholds are met

@lnjbr
Copy link
Author

lnjbr commented Dec 26, 2023

Is there a reason for the workflow to be running npm install rather than npm ci for the Node 12.x and 14.x checks when y'all are committing the package-lock.json file? I was only able to reproduce the error in CI locally after regenerating my lock file. All of the dependencies in y'all's package.json file are careted and, notably, deterministic builds as a default were not introduced until NPM 7 (i.e. Node 15+). I believe that's causing the discrepancy and, consequentially, the error within the node_modules directory.

TL;DR: Can I change the install command to npm ci for the 12.x and 14.x checks or are y'all intentionally avoiding deterministic builds on them?

@DigitalBrainJS
Copy link
Collaborator

@lnjbr Well, this is an old major branch, no one did CI backports from the 1.x branch...

@bmuenzenmeyer
Copy link

bmuenzenmeyer commented Dec 27, 2023

that looks like permission to change it if you ask me?
I say give it a shot and let the CI tell the story here

@bmuenzenmeyer
Copy link

@jasonsaayman can you please re-run the CI again?

@lnjbr
Copy link
Author

lnjbr commented Jan 8, 2024

Validated that CI for the branch is passing in my repo after replacing npm install with npm ci in the ci.yml file and adding the --localTs option to the dtslint command in the package.json's test script 🙇

Screenshot of the GH actions page in my repo showing successful builds for the 12.x, 14.x, 16.x, and 18.x build checks in the ci.yml workflow

@bmuenzenmeyer
Copy link

@DigitalBrainJS can you please re-run the CI here?

@TerkaPrzemyslaw1897
Copy link

bump!

@DigitalBrainJS DigitalBrainJS merged commit 2755df5 into axios:v0.x Jan 19, 2024
7 checks passed
@DigitalBrainJS
Copy link
Collaborator

The PR will be released a bit later, because in the v0.x branch there is no CI/CD, which means we need to manually release it, like in the wild old days. Backporting CI/CD is unlikely to be advisable for releasing rare security patches.
@jasonsaayman can you look into this?

@lagatchell
Copy link

The PR will be released a bit later, because in the v0.x branch there is no CI/CD, which means we need to manually release it, like in the wild old days. Backporting CI/CD is unlikely to be advisable for releasing rare security patches. @jasonsaayman can you look into this?

@DigitalBrainJS any update on the release of this?

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.

None yet

8 participants