Improve the atack generation of graphql queries and mutations

doyensec / inql

InQL is a robust, open-source Burp Suite extension for advanced GraphQL testing, offering intuitive vulnerability detection, customizable scans, and seamless Burp integration.

https://doyensec.com/

Apache License 2.0

1.51k stars 156 forks source link

Improve the atack generation of graphql queries and mutations #122

Open bandronic opened 1 year ago

bandronic commented 1 year ago

I have fixed some issues in the generation of the graphql queries. The issues I faced where that the extension did not create corresponding separate operations and did not remove the comments from the code.

What I basically did was:

stripped all the comments
set the query type to query or mutation based on the action type
used the already existing logic to create multiple operations with different name

execveat commented 1 year ago

Hey @bandronic and thanks for contributing! I'm a little puzzled about the commit c99e6763705c97cfb94ac0570432b0f9e51ea161, as nothing seems to be using this new library. Can you explain the reason for adding it?

Also, I have to warn you that we'll attempt rewriting the bulk of the extension in Kotlin soon. The GraphQL parsing will still be done in Python (for a while), using the https://github.com/doyensec/gqlspection library. So, if you plan to add more improvements to the batching attacks and want to write Python, gqlspection might be a better target.

bandronic commented 1 year ago

Hey, thanks for getting back to me. Yes, I will remove that library as it was a test for a different parser. In this case, should I continue improving this or should I just forget it? Thanks!

bandronic commented 1 year ago

I believe that this implementation will fix issue #97 if I'm not mistaking

execveat commented 1 year ago

We'll start the rewrite in August and we'll try to keep it 1:1 or as close as possible to the existing Python code. So, I'd say if you see other improvement opportunities that are few lines long - go for it (with the understanding that it will likely get translated to Kotlin before the next release). On the other hand, if you have larger features in mind, it's probably best to wait for the rewrite, to make merging things easier.

On the topic of external libraries - once we're in Kotlin-land, I'm fine with bringing external dependencies, so if you have suitable GraphQL parsing libraries (Java/Kotlin) in mind, please share. We could either import them directly to replace GQLSpection or rewrite GQLSpection to Kotlin (possibly replacing core with some upstream libraries). We're focusing on rewriting GUI stuff right now, so I haven't researched this and any help / proof of concepts would be very helpful.

bandronic commented 1 year ago

I understand. Thanks for the explanation. I was starting to write an improvement for value generation for different data types and expansion of the input arguments as in if you have an input type, expand it to it's most basic scalar types, but in this case I'll wait for the rewrite. Regarding this pull request, in this case is it still valid?

execveat commented 1 year ago

Oh, that's a great feature! Might be relevant: we have a plan to generate queries with variables (#77), although I'm still not sure about UI.

The pull request is good! I want to take another look at it over the weekend as the Attacker was left behind for a while and there might be more bugs lurking there.

bandronic commented 1 year ago

Ah yes, I remember I saw a ticket regarding this but I didn't remember which one. Please let me know if I can help in any way. Thanks!