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

Latest version of cli is pulling in insecure packages that have available patches #11543

Open
3 of 7 tasks
G-Rath opened this issue May 4, 2024 · 17 comments
Open
3 of 7 tasks

Comments

@G-Rath
Copy link
Contributor

G-Rath commented May 4, 2024

The latest version of the vercel cli is pulling in packages with known vulnerabilities that have available patches.

Current vulnerabilities:

While I don't believe any of these are exploitable in the context of this cli, they are a nuisance since non-breaking patches are available and security policies can make these expensive to ignore.

`npm audit` output as of 2025-02-11
# npm audit report

path-to-regexp  4.0.0 - 6.2.2
Severity: high
path-to-regexp outputs backtracking regular expressions - https://github.com/advisories/GHSA-9wv6-86v2-598j
fix available via `npm audit fix --force`
Will install [email protected], which is a breaking change
node_modules/path-to-regexp
  @vercel/remix-builder  <=2.0.3 || >=5.2.4
  Depends on vulnerable versions of path-to-regexp
  node_modules/@vercel/remix-builder
    vercel  >=24.2.5-canary.0
    Depends on vulnerable versions of @vercel/fun
    Depends on vulnerable versions of @vercel/node
    Depends on vulnerable versions of @vercel/redwood
    Depends on vulnerable versions of @vercel/remix-builder
    node_modules/vercel
  @vercel/routing-utils  <=3.1.0 || >=5.0.0
  Depends on vulnerable versions of path-to-regexp
  node_modules/@vercel/routing-utils
    @vercel/redwood  >=0.6.1-canary.0
    Depends on vulnerable versions of @vercel/routing-utils
    node_modules/@vercel/redwood

tar  <6.2.1
Severity: moderate
Denial of service while parsing a tar file due to lack of folders count validation - https://github.com/advisories/GHSA-f5x3-32g6-xq36
fix available via `npm audit fix --force`
Will install [email protected], which is a breaking change
node_modules/tar
  @vercel/fun  *
  Depends on vulnerable versions of tar
  node_modules/@vercel/fun

undici  4.5.0 - 5.28.4
Severity: moderate
Use of Insufficiently Random Values in undici - https://github.com/advisories/GHSA-c76h-2ccp-4975
fix available via `npm audit fix --force`
Will install [email protected], which is a breaking change
node_modules/undici
  @vercel/node  2.14.0 || >=3.0.2
  Depends on vulnerable versions of undici
  node_modules/@vercel/node

9 vulnerabilities (4 moderate, 5 high)

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

Related issues and pull requests:

zcbntr added a commit to zcbntr/rt-trainer that referenced this issue May 9, 2024
Hoping that vercel/vercel#11543 gets resolved eventually
@soulofmischief
Copy link

Thanks for staying on top of this @G-Rath

@G-Rath
Copy link
Contributor Author

G-Rath commented Jun 10, 2024

Thanks, but unfortunately I'm not confident that they'll get addressed anytime soon - we're currently in the process of reviewing moving our Vercel apps to Netlify since they setup a whole internal workflow to stay on top of these updates in response to my equivalent issue about their cli (which is even more impressive given they use a shrink-wrap.json where Vercel just pins their top level dependences).

I will continue to update this issue for the foreseeable future though

@justinoboyle
Copy link

This is very important for organizations that have security policies that require packages to be updated with all security patches.

@G-Rath
Copy link
Contributor Author

G-Rath commented Sep 12, 2024

Added new GHSA-9wv6-86v2-598j vulnerability, and marked micromatch as patched

@mrmckeb
Copy link
Contributor

mrmckeb commented Oct 10, 2024

I realise that these are probably low priority for the team as these are probably not issues for the CLI, but it would help teams like ours if these were cleaned up. Dependabot raises these as security issues, and we need to dig in and assess if they really are issues or not.

CC @trek to perhaps raise with the team.

@chrisdenton-ct
Copy link

Hello,

I've got all of these vulnerabilities showing up in GitHub - including undici, btw - though I'm mainly concerned about path-to-regexp and semver as they are both rated as high risk.

Now I appreciate that the vulnerabilities may not be exploitable and therefore the risk level is arguably wrong, but even so, we've still got to convince our external auditors of this, which is going to take time and if we fail to do so then that's our security certification up in smoke. I'm sure many other vercel users will have similar problems.

So I would like to respectfully request an update, and reiterate that whilst this may be low priority for some people it is not low priority for everyone. Far from it.

@gaelperon
Copy link

+1 @chrisdenton-ct

It's important for us too

@omonk
Copy link

omonk commented Nov 12, 2024

@chrisdenton-ct - I emailed vercel security directly about this a few months ago.

Essentially they said they're always keeping track of potential security threats and they're aware of these reports but they don't consider them issues and they're not going to fix it

@burtek
Copy link

burtek commented Nov 12, 2024

Apparently it's not an issue for Vercel if companies can't use Vercel because the nested dependencies are flagged. Strange business decision

@chrisdenton-ct
Copy link

@chrisdenton-ct - I emailed vercel security directly about this a few months ago.

Essentially they said they're always keeping track of potential security threats and they're aware of these reports but they don't consider them issues and they're not going to fix it

Thanks, that's useful to know, though obviously I find that response from vercel to be disappointing.

@omonk
Copy link

omonk commented Nov 12, 2024

I'll try and dig out the email tomorrow

@gaelperon
Copy link

I have sent an email to the Vercel security team

@omonk
Copy link

omonk commented Nov 13, 2024

I emailed the security team requesting they fix the issues listed in this specific issue

Their reply on 23 Sept 2024:


Thank you for contacting the Vercel security team. Our software is patched in alignment with our internal vulnerability management process timelines, which relies heavily on contextualisation of vulnerabilities (not just CVSS rating).

We'll patch this vulnerability in due course, but please understand this may not be immediate based on the context of the vulnerability. If you would like to see this patched sooner and to contribute to the open source software ecosystem, the team would welcome any PRs to the repository.

Please feel free to reach out if you have any further questions or concerns.


My thoughts

All of the reported vulnerabilities have PRs against them. Some are still failing CI but it does feel a bit spicy to ask for paying customers to fix the product they're paying to use.

@joey-ma
Copy link

joey-ma commented Dec 19, 2024

For those who still wanted to resolve this, I was able to patch this by adding the following to my package.json:

  "overrides": {
    "@vercel/node": {
      "path-to-regexp": "8.2.0"
    }
  },

Can't promise I'll be able to open a PR 🙃, but definitely appreciate the open-source community!

@joey-ma
Copy link

joey-ma commented Dec 19, 2024

Tried following the Contributing Guidelines to open a PR, but it seems like there's quite a bit going on here. I'd imagine it'll take a while just to get past "Make sure all the tests pass before making changes."

Some things I've noticed: packages/functions/ had several no-undef eslint errors.
image

Theoretically this should be fixable by changing the eslintConfig's parserOptions.ecmaVersion to 2020 and env.es2020 to true, but it didn't work for me here. Noticed there were also tons of packages in .eslintignore... but not packages/functions.

In addition, there were several failing unit tests as well. 🙃 Might be better for someone internal who's more familiar with the overall app + testing to take a look?

@G-Rath
Copy link
Contributor Author

G-Rath commented Dec 19, 2024

@joey-ma if you're trying to do a PR patching path-to-regexp, I've already done that (#12131) and the team have been trying to get it landed looks like it's now been landed! 🎉 (#12734)

If you'd like to help out, I would recommend exploring upgrading tar in @vercel/fun (vercel/fun#104) as that's had no attention and requires a major version bump through from my initial research it seemed like it should be safe to just do because the only breaking changes were in Node versions that @vercel/fun already has dropped support for.

edit: ok for path-to-regexp it seems it was patched for a short while but then the team decided they wanted to rollout some monitoring so they're reverted the upgrade: #12746

@G-Rath
Copy link
Contributor Author

G-Rath commented Feb 10, 2025

Happy new year everyone!

I've updated this to reflect changes - namely:

  • path-to-regexp has changed: @vercel/node seems to no longer use it, but it is now being used by @vercel/remix-builder though at least it's the same version of @vercel/routing-utils so technically one instance of the vulnerability went away
  • debug and semver have been addressed: the cli is now finally using the updated version of @vercel/fun
  • esbuild has been added
  • undici has been added

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

No branches or pull requests

9 participants