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

Upgrade node-fetch #33462

Closed
juliocarneiro opened this issue Jan 19, 2022 · 7 comments · Fixed by #33466
Closed

Upgrade node-fetch #33462

juliocarneiro opened this issue Jan 19, 2022 · 7 comments · Fixed by #33466
Labels
bug Issue was opened via the bug report template.

Comments

@juliocarneiro
Copy link

Run next info (available from version 12.0.8 and up)

Operating System:
Platform: win32
Arch: x64
Version: Windows 10 Pro
Binaries:
Node: 14.18.2
npm: 6.14.15
Yarn: 1.22.17
pnpm: N/A
Relevant packages:
next: 12.0.8
react: 17.0.2
react-dom: 17.0.2

What version of Next.js are you using?

12.0.8

What version of Node.js are you using?

14.18.2

What browser are you using?

Chrome

What operating system are you using?

Windows 10

How are you deploying your application?

Vercel

Describe the Bug

Vulnerability in Next.js 12.0.8, in node-fetch package

Expected Behavior

λ snyk test next

Testing next...

✗ Medium severity vulnerability found in node-fetch
Description: Information Exposure
Info: https://snyk.io/vuln/SNYK-JS-NODEFETCH-2342118
Introduced through: node-fetch@2.6.1
From: node-fetch@2.6.1

Organization: xxx
Package manager: npm
Open source: yes
Project path: next

Tested next for known vulnerabilities, found 1 vulnerability, 1 vulnerable path.

To Reproduce

install nextjs project
install snyk global
run snyk test

@juliocarneiro juliocarneiro added the bug Issue was opened via the bug report template. label Jan 19, 2022
@juliocarneiro juliocarneiro changed the title Vulnerabilities in Next.js 12.0.8 Vulnerability in Next.js 12.0.8 / node-fetch Jan 19, 2022
@balazsorban44 balazsorban44 changed the title Vulnerability in Next.js 12.0.8 / node-fetch Upgrade node-fetch Jan 19, 2022
@kodiakhq kodiakhq bot closed this as completed in #33466 Jan 19, 2022
kodiakhq bot pushed a commit that referenced this issue Jan 19, 2022
Fixes #33462 

Tried upgrading to v3, but we rely on the `timeout` property:

https://github.com/vercel/next.js/blob/e5dee17f776dcc79ebb269f7b7341fa6e2b6c3f1/packages/next/telemetry/post-payload.ts#L12

Which was removed since it's non-standard:

https://github.com/node-fetch/node-fetch/blob/main/docs/v3-UPGRADE-GUIDE.md#the-timeout-option-was-removed

I wanted to keep this PR minimal, so I did not try to work around the above.

## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes by running `yarn lint`
ecchochan added a commit to ecchochan/ts-nextjs-tailwind-pwa-starter that referenced this issue Jan 23, 2022
@nikhilgupta16
Copy link

nikhilgupta16 commented Jan 25, 2022

@balazsorban44 / @kodiakhq did we fix this for next 11.x version as well? Is that in the plan?

@balazsorban44
Copy link
Member

We likely won't backport it since you can update node-fetch in v11 with overrides for npm or resolutions for yarn. node-fetch wasn't precompiled in 11.1.3

@RishikeshDarandale
Copy link

RishikeshDarandale commented Jan 25, 2022

We likely won't backport it since you can update node-fetch in v11 with overrides for npm or resolutions for yarn. node-fetch wasn't precompiled in 11.1.3

@balazsorban44 , overrides is introduced with npm 8.3.0. What about if npm < 8.3.0 ? As this security vulnerability, why can't it get patched in 11.x? Is 11.x will not be maintained further?

@balazsorban44
Copy link
Member

On reconsideration, we might do it after all, thank you for pointing it out.

@guidorice
Copy link

@balazsorban44 Use npm overrides huh? I hope do you reconsider about patching next v11: Just consider any new users of next v11 today:

$ npm install next@11
npm WARN deprecated querystring@0.2.1: The querystring API is considered Legacy. new code should use the URLSearchParams API instead.
npm WARN deprecated querystring@0.2.0: The querystring API is considered Legacy. new code should use the URLSearchParams API instead.

added 320 packages, and audited 321 packages in 9s

51 packages are looking for funding
  run `npm fund` for details

2 high severity vulnerabilities

To address all issues (including breaking changes), run:
  npm audit fix --force

Run `npm audit` for details.

$ npm audit
# npm audit report

node-fetch  <2.6.7
Severity: high
node-fetch is vulnerable to Exposure of Sensitive Information to an Unauthorized Actor - https://github.com/advisories/GHSA-r683-j2x4-v87g
fix available via `npm audit fix --force`
Will install next@10.0.1, which is a breaking change
node_modules/node-fetch
  next  9.0.6-canary.0 - 9.3.4-canary.0 || 10.0.2-canary.0 - 12.0.8
  Depends on vulnerable versions of node-fetch
  node_modules/next

2 high severity vulnerabilities

To address all issues (including breaking changes), run:
  npm audit fix --force

@balazsorban44
Copy link
Member

11.1.4 and 12.0.9 is out, please upgrade!

natew pushed a commit to natew/next.js that referenced this issue Feb 16, 2022
Fixes vercel#33462 

Tried upgrading to v3, but we rely on the `timeout` property:

https://github.com/vercel/next.js/blob/e5dee17f776dcc79ebb269f7b7341fa6e2b6c3f1/packages/next/telemetry/post-payload.ts#L12

Which was removed since it's non-standard:

https://github.com/node-fetch/node-fetch/blob/main/docs/v3-UPGRADE-GUIDE.md#the-timeout-option-was-removed

I wanted to keep this PR minimal, so I did not try to work around the above.

## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes by running `yarn lint`
@github-actions
Copy link
Contributor

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 26, 2022
nevilm-lt pushed a commit to nevilm-lt/next.js that referenced this issue Apr 22, 2022
Fixes vercel#33462 

Tried upgrading to v3, but we rely on the `timeout` property:

https://github.com/vercel/next.js/blob/e5dee17f776dcc79ebb269f7b7341fa6e2b6c3f1/packages/next/telemetry/post-payload.ts#L12

Which was removed since it's non-standard:

https://github.com/node-fetch/node-fetch/blob/main/docs/v3-UPGRADE-GUIDE.md#the-timeout-option-was-removed

I wanted to keep this PR minimal, so I did not try to work around the above.

## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes by running `yarn lint`
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue was opened via the bug report template.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants