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

This repo (and all forks) have major security vulnerabilities #892

Open
bfischer1121 opened this issue Mar 27, 2025 · 11 comments
Open

This repo (and all forks) have major security vulnerabilities #892

bfischer1121 opened this issue Mar 27, 2025 · 11 comments

Comments

@bfischer1121
Copy link

All products based on this repo have the following vulnerabilities:

  1. Any user can add messages to any other user's chat
  2. Any user can replace any other user's documents

I responsibly disclosed these to the Vercel team, and they addressed the first vulnerability with this commit, but will not communicate that to you.

The 2nd vulnerability is still active in this repo, and all forks.

They refuse to communicate these security issues with the community, so I'm doing so here.

Here is their viewpoint on the matter:

Thanks for following up. While we publish security advisories for our open source software in alignment with industry standards (e.g for Next.js, Turbo, Hyper), it's a little trickier in this scenario because this repository is a simple template, without having the concept of any semantic versioning or "safe" versions that we recommend users upgrade to.

Additionally, as users modify the template for their needs, it becomes extremely complicated to offer straightforward remediation advice, due to lack of familiarity with each individual codebase.

It's generally best practice to thoroughly audit any third party code a user chooses to implement (whether from a template, StackOverflow, or an LLM), and ensuring that the security is in alignment with individual risk appetite.

We're therefore intending to treat your report as a broader improvement request for the template. We hope this makes sense.

Please let me know if you have any further questions, comments, or concerns.

If you are building a product using this repo as a basis, know that their official viewpoint is that there is no reasonable expectation of security and, when issues are found, they will not be communicated to you.

Happy to disclose more about these vulnerabilities in this thread if needed. Cheers!

@The-1818
Copy link

This is very important. Appreciate it. Would disabling "Share Chat" functionality temporarily fix it?

@bfischer1121
Copy link
Author

@The-1818 yea that seems like a great approach -- don't think the chat/document ids are in any other endpoints

@JackFinnegan
Copy link

Thanks @bfischer1121 .

I'm disappointed to read Vercel's stance.

I run an AI chatbot based startup, and I pay for Vercel.

I've downloaded this repo to investigate what 'best-in-class' looks like and re-assess my own repo accordingly.

Seeing that security is not a priority is extremely concerning!

@jeremyphilemon , hello!
I'm sorry to tag you specifically, as I've just picked your name as the most recent Vercel employee to make a commit.
Is it possible you can make security a priority for this repo please?

The response that @bfischer1121 posted from Vercel (I appreciate probably not you!) seems peculiar and inappropriate.

@JackFinnegan
Copy link

@bfischer1121 can you please explain the second vulnerability that still exists, here?

@yamz8
Copy link

yamz8 commented Mar 28, 2025

Really surprised they that didn't chose to close both issues and move on, this is one of their most popular repos.

@jeremyphilemon
Copy link
Collaborator

jeremyphilemon commented Mar 29, 2025

Hey @bfischer1121, thanks so much for flagging this!

As mentioned in the response you had received earlier from Vercel, it does seem like it's tricky to communicate these changes to the community when there is no concept of versioning.

Although tricky, I don't think it's an impossible problem to solve. Since this template has received an unprecedented amount of forks and has been deployed by many developers, it’s fair to set the expectation that the template meets a baseline level of security and transparency around changes.

As a result, I'll be shipping the following improvements to help build more trust and security:

  • Add an ownership check to api/document to patch the 2nd vulnerability
  • Add changeset to version the template and follow semver for updates
  • Create a changelog page to help with communicating these changes to the community and improving visibility
  • Add tests specifically for auth/security
  • Document all security-related touchpoints in the template

Let me know what you think, and I appreciate you for nudging us to do better!

@yamz8
Copy link

yamz8 commented Mar 29, 2025

Jeremy for the win

@bfischer1121
Copy link
Author

@jeremyphilemon that's awesome, thanks so much for these improvements and the quality of your response!

@jeremyphilemon
Copy link
Collaborator

@bfischer1121 can you share more details around the second vulnerability? I checked /api/document and it looks like it already has an ownership check setup?

  if (document.userId !== session.user.id) {
    return new Response('Unauthorized', { status: 401 });
  }

@bfischer1121
Copy link
Author

@jeremyphilemon yea for this one the starting point is on the POST method with general flow being:
POST doc with the same doc id as other user (passes check if logged in) -> other user calls GET doc (passes check that first version of doc matches their user id) -> system includes all the docs with injected one as the latest

@yamz8
Copy link

yamz8 commented Apr 6, 2025

hey @jeremyphilemon I would like to add some of the suggestions from #898 — can you share your thoughts? Are some of the suggestions things you plan to implement as part of the improvements you mentioned?

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

5 participants