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

Review security of the Strapi endpoints #7

Open
franckc opened this issue Oct 5, 2022 · 4 comments
Open

Review security of the Strapi endpoints #7

franckc opened this issue Oct 5, 2022 · 4 comments
Assignees
Labels
P0 Blocking an imminent release, "everything is broken", all hands on deck, etc

Comments

@franckc
Copy link

franckc commented Oct 5, 2022

No description provided.

@franckc franckc added the P0 Blocking an imminent release, "everything is broken", all hands on deck, etc label Oct 5, 2022
@franckc franckc self-assigned this Oct 13, 2022
@shahthepro
Copy link
Contributor

So far, I have disabled all update/delete/create routes for all the schemas we added. Some routes added by plugins like the i18n and menus still the have those non-read routes public. But we have disabled access to that through the admin panel (using the Roles and Permissions plugin).

Other than the sitemap, all the read routes of the schemas we added also require a API token. Also added a few simple sanitizations to make sure we are exposing any data that we don't want through the API. So, I'd say we are good for the launch. I'll keep exploring to see if we can solidify it more

@franckc franckc assigned mikeshultz and unassigned franckc Oct 21, 2022
@mikeshultz
Copy link

mikeshultz commented Nov 30, 2022

Just starting to go through this and these are my primary questions and initial notes. Feel free to add context to it but don't feel like it's required since this is just raw notes as I'm getting started.

Are these routes consumed by clients and/or servers?

I see an API token in prod labeled "client-token" that is consistently used, however I haven't yet seen a site that made any XHR calls to an API. It may only be a next.js backend fetching this data but will need to confirm.

Is any sensitive data stored in this API?

Looking at the content-types it looks like it does not. It doesn't look like we have any staged unpublished content that might reveal plans or allow inside information leaks but I suspect it may happen since we did that with Medium posts.

Does this API access any sensitive data sources?

My initial impressions were that it does not, but there is a dependency on pg and heroku is configured with a postgres DB. I haven't come across where it's implemented yet but I assume it's there for a reason. My local install is working and I have not configured DB access so it's not immediately clear where this comes into play.

Looks like it's primary storage is Sqlite for local dev and Postres for prod. I don't see any references to external databases in the codebase or config.

What's the worst case scenario if this API is compromised?

External links or impersonation content would make us look pretty bad but maybe not critical to the business. Good chance of an XSS injection but that's likely only through the admin which I think is out of scope for this review.


TODO

  • Answer primary questions better, and review data in prod for better understanding of risks
  • Catalog routes, look for writers
  • Review AWS access key permissions (creds are configured as env vars) [Only has permissions for bucket cmsmediastaging]
  • Review AWS resources (s3 bucket in config), how they're used, and permissions [Appears the only relevant resources are the IAM user and the cmsmediastaging bucket, which contains image uploads. It is publicly accessible from https://cmsmediastaging.s3.us-east-1.amazonaws.com/]
  • Review use of JWT secrets
  • Check on what can trigger E-mails (Sendgrid is configured on Heroku)
  • Look for STRAPI best practices wrt the admin
  • Look at STRAPI's history for security issues and if there's anything we can keep an eye on

Will update this post as I go. I'll add new replies if I find anything important or have any questions.

@shahthepro
Copy link
Contributor

Thanks for looking into this 🙏🏻

I see an API token in prod labeled "client-token" that is consistently used, however I haven't yet seen a site that made any XHR calls to an API. It may only be a next.js backend fetching this data but will need to confirm.

All API are consumed by the Next.js server. We don't make XHR calls directly from client, it's a pass-through (from Client <> Next.js Server <> Strapi server) to not expose the API key on the client side

External links or impersonation content would make us look pretty bad but maybe not critical to the business. Good chance of an XSS injection but that's likely only through the admin which I think is out of scope for this review.

I'd say we may have to worry a bit more about impersonation of content. Someone can link Story or OUSD to a phishing site or something that can steal funds from their web3 wallet (if they connect it).

@mikeshultz
Copy link

Having trouble understanding their magic route generation. For instance, the Author content type generates routes with this factory function:

module.exports = createCoreRouter('api::blog.author', {
only: [],
});

But none of these return:

  • /api/blog/author
  • /api/blog/authors
  • /api/author
  • /api/authors

I got lost in their abstractions in the source on how singular/plural names are defined so that was no help. Though I suspect singular/plural names will equal what's defined in the content-type here:

"singularName": "author",
"pluralName": "authors",

Their docs suggest it should just be /api/authors as a collection type. There doesn't appear to be any way to configure routes in the admin, and they aren't explicitly set like the non-factory definitions...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P0 Blocking an imminent release, "everything is broken", all hands on deck, etc
Projects
None yet
Development

No branches or pull requests

3 participants