New CS: GraphQL Security Cheat Sheet

[ThunderSon]

After various discussion on Slack, it is important to create a GraphQL Cheat Sheet in order to guide developers to writing secure GraphQL endpoints.

@bigshebang to provide us with a draft on work already done from their side.

@PauloASilva FYI. This should be moving properly with the API security project's line as well.

Let's see this effort through. After Luke's draft, we can modify the template and see what's missing and could be improved 😄

[mackowski]

Awesome! ;-)

[ErezYalon]

@bigshebang, is there an ETA for the draft? We (together with @PauloASilva) are starting to work on the subject as well.

[bigshebang]

@ErezYalon the ETA is now :D #434

[ThunderSon]

What @bigshebang presented is an effort already made on creating a GraphQL knowledge base. Next up is growing that to cover the defenses spectrum and tidying it up to have a sheet feeling.

[ErezYalon]

We are a small team working on several API Security projects.
We will start our GraphQL research soon and add our results to the sheet. Most will probably come from @PauloASilva.

[PauloASilva]

Hi guys,
Sorry for the late reply.

My initial comments and thoughts on how to improve the GraphQL Cheat Sheet proposal were left in the PR.

On an initial stage I think we can work on our general suggestion: make the CS more actionable, but only after hearing your thoughts about our comments.

Cheers,
Paulo A. Silva

[ThunderSon]

Thanks @PauloASilva ! We didn't provide our full feedback as well. Thanks for tackling this!
I'll add some more comments and try to get some improvements in hopefully in the weekend.

[mackowski]

I will try to review it over the weekend

[jmanico]

What is the stats? Can I help here?

[nikitastupin]

Hi there! I can describe some common vulnerabilities related to GraphQL APIs that I've found during my bug bounty journey. Also methodology of researching GraphQL APIs from bug hunter perspective.

However as far as I understand this cheatsheet intended for developers. From your perspective, is that kind of information valuable to the cheatsheet?

[mackowski]

@jmanico @nikitastupin please review this work #434 and share your thoughts.
@nikitastupin perspective of a bug hunter is very important - if you can enumerate common vulnerabilities related to GraphQL and see if we covered all of them with the mitigations/fixes in the #434 it will be awesome, if we are missing some vuln please add a comment about it.

[nikitastupin]

I've took a look at #434 and I agree with most comments that other reviewers left. Also from my perspective proposed cheat sheet provides too general information or information that can be found in other OWASP resources 😃

It was easier for me to make cheat sheet from scratch than integrate my thoughts in existing one. Thus I've decided to make #469 which reflects my vision on how GraphQL cheat sheet should look like. It's not complete and definitely not perfect. However I've tried to keep it small, practical and clean for faster review and merge.

We can add more information to it in the future in small steps.

[bigshebang]

I've been working on incorporating everyone's feedback and will have at least one new commit up tomorrow. I'm about halfway through so we'll see if I can finish the rest tomorrow.

@nikitastupin I will also add some of your content into the CS where it makes sense.

[nikitastupin]

@bigshebang it's great news! However I think that it's easier to create some minimal cheat sheet done and then add other things to it. So if you'll feel that adding content form #469 to new version of #434 is slowing down the process you may leave this work to me 😃

[mackowski]

I like the content from both @nikitastupin and @bigshebang CS is awesome, we need to merge them.
We will proceed with @bigshebang version of the CS and add content from #469 to it.

[bigshebang]

@mackowski I have added content from @nikitastupin 's PR that I thought was relevant as well as incorporated all other existing feedback. There are a couple things I would appreciate some help with from someone who has more GraphQL experience or a testing setup handy; I left comments for these things. If nobody else gets to them I will at some set up my own test environment (just not sure when).

[mackowski]

@bigshebang awesome thanks for update!

[bigshebang]

Close to being done, gonna push some commits in the next couple days. But I just wanted to leave a few items that need improvement in this CS in the future:

• The content under the "OS/Container Resource Management" section should basically be replaced by a link to the "Availability" section of the Web Services CS when it is improved to have better information. The content for this section in this CS also does not have vetted Windows information, so that needs a second look from a Windows person.
• Some code examples are not tested to make sure they work.
• A closer/better look at the "Secure Configurations" section. Most of the remediations there aren't tested and just thrown in from some relatively quick googling and educated guessing. And the suggestions to fix could be improved in general for this section.

[bigshebang]

@mackowski the CS is all done! I would just like Paulo to give a quick review on one new section (I tagged him on the section I want them to review). Would be great if you or @ThunderSon could also give the whole thing a once over to make sure it's all nice and tidy and ready to go.

[PauloASilva]

I've reviewed the new section and left a few suggestions based on what I have found in the wild.
I think the overall result is pretty good and it's ready to be merged.

@ErezYalon Maybe you'll be able to help reviewing the whole CS and provide some feedback.

@bigshebang Congrats for the hard and great work!

Cheers,
Paulo A. Silva

[ErezYalon]

@PauloASilva I'm on it.

[ErezYalon]

After review, there are two comments:

• Introspection should have its own subsection that explains what it is and why it should be removed.
• What about Prototype Pollution (under Injection) for GraphQL implementations in JS that allow mutation?

[bigshebang]

@ErezYalon

I have a one-liner that explains why Introspection is bad. I don't think this CS needs to go deeper than this:

Many implementations of GraphQL have Introspection and GraphiQL enabled by default and leave them accessible without requiring authentication. This is problematic because introspection allows the requester to learn all about supported schema and queries (see a real-world example abusing this). Introspection might be how the API owner wants to educate consumers about how to use the API. However, the preferred way to educate consumers about a service is through a separate documentation channel such as a wiki, Git Readme, or readthedocs.

I'll defer to @ThunderSon on that.

Is JS Prototype Pollution more common in GraphQL than other APIs? If so we can add a note about it. Erez and @PauloASilva would love your thoughts here as well as the actual content we can add.

[ThunderSon]

1. This is a CS for best practices. The reader's outcome should be: "To improve my security stance, I need to do X because Y can happen. Why Y can happen? There is this Z reference for it to dig into it". The one liner presented mentions what the issue is.
   For me, reading the bits about introspection gave me enough insight on what it is on a high level, the need for it, and that it shouldn't be available on production. Abusing it, if that's part of what you want to expand on, shouldn't be in this CS.
   Would you be able to expand on your point? Maybe we're missing on something.

2. For prototype pollution, wouldn't that be more on library implementations? If yes, that shouldn't be part of this CS. Otherwise, can you please try and provide a cleaner example? I know this is happening everywhere now, trying to model with you the way it could happen on an implementation, and what exactly will be the weak point or layer.
   If the CS was to give a solution or a fix, what would you recommend?

[PauloASilva]

Is JS Prototype Pollution more common in GraphQL than other APIs? If so we can add a note about it. Erez and @PauloASilva would love your thoughts here as well as the actual content we can add.

I share the same vision than @ThunderSon: prototype pollution is more a library-thing.
In fact there are reported GraphQL issues related to prototype pollution but all I came up with were due to some vulnerable dependency (e.g. lodash).

Regarding Introspection, it is a controversial subject: some people argue that it can be used as "documentation". I don't share such vision and I am comfortable with what we have now.

[bigshebang]

Perfect! @ThunderSon I think we're all set here then. I'm eager to get this merged in 😄

[ThunderSon]

Fixed in #434