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

Secure file uploads when NODE_ENV=dev #487

Merged
merged 1 commit into from Aug 29, 2022
Merged

Conversation

jdorn
Copy link
Member

@jdorn jdorn commented Aug 29, 2022

If a user fails to set NODE_ENV=production, our built-in security checks are disabled and it's possible to misconfigure GrowthBook and leave the file upload API endpoint open to attack.

To be affected, ALL of the following must be true

  1. Self-hosted (GrowthBook Cloud is unaffected)
  2. GrowthBook API exposed to the internet
  3. Using local file uploads (as opposed to S3 or Google Cloud Storage)
  4. NODE_ENV set to a non-production value
  5. JWT_SECRET set to the default "dev" string

To solve these security vulnerabilities, this PR makes 3 changes:

  1. Add more protection around the fileUpload API endpoint (protect against directory traversal, etc.)
  2. Only disable built-in security checks if the domain is still localhost (in case someone forgets to update NODE_ENV)
  3. Add an extra sanity check to the first-time register API endpoint

What should you do if you are potentially affected:

  • Update to the latest GrowthBook version
  • Make sure NODE_ENV is set to "production"
  • Set the JWT_SECRET environment variable to a long random string

If a user fails to set NODE_ENV=production, our built-in security checks are disabled and it's possible to misconfigure GrowthBook and leave the file upload API endpoint open to attack.

To be affected, ALL of the following must be true
1. Self-hosted (GrowthBook Cloud is unaffected)
2. GrowthBook API exposed to the internet
3. Using local file uploads (as opposed to S3 or Google Cloud Storage)
4. NODE_ENV set to a non-production value
5. JWT_SECRET set to the default "dev" string

To solve these security vulnerabilities, this PR makes 3 changes:
1. Add more protection around the fileUpload API endpoint (protect against directory traversal, etc.)
2. Only disable built-in security checks if the domain is still `localhost` (if someone forgets to set NODE_ENV)
3. Add an extra sanity check to the first-time register API endpoint
@vercel
Copy link

vercel bot commented Aug 29, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Updated
docs ⬜️ Ignored (Inspect) Aug 29, 2022 at 9:27PM (UTC)

@jdorn jdorn merged commit 1a5edff into main Aug 29, 2022
2 checks passed
@jdorn jdorn deleted the secure-dev-file-uploads branch August 29, 2022 21:30
@Auz Auz added the security An issue relating to security label Sep 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
security An issue relating to security
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants