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: Bump axios version #40

Closed
wants to merge 2 commits into from
Closed

fix: Bump axios version #40

wants to merge 2 commits into from

Conversation

bryanhuhta
Copy link
Contributor

Fixes CVE-2023-45857. I also removed the "compatible with" operator to avoid blindly accepting more upstream changes that might break us.

@bryanhuhta bryanhuhta self-assigned this Feb 5, 2024
@bryanhuhta bryanhuhta marked this pull request as draft February 5, 2024 18:35
@bryanhuhta
Copy link
Contributor Author

After more research, there are two problems with simply bumping the axios version to address this CVE:

  1. v1 axios changes its module export from CJS modules to ESM
  2. This library may not even be impacted by this CVE

For 1, upgrading to axios v1 causes our Jest test to break, which can be fixed, but this signals we may be breaking users of the library. We should proceed with caution here.

For 2, CVE-2023-45857 has reproduction steps which include:

const instance = axios.create({
  withCredentials: true,
});

This library doesn't use the withCredentials: true option and thereby may not actually be susceptible to this vulnerability.

@simonswine
Copy link
Collaborator

Went with #56 #57 instead

@simonswine simonswine closed this Feb 26, 2024
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.

2 participants