7 ms·

I haven't worked with GraphQL before, but looking at those code snippets and reading the description of the vulnerability, it seems like a mess. You're giving a

I haven't worked with GraphQL before, but looking at those code snippets and reading the description of the vulnerability, it seems like a mess. You're giving a client unfettered access to just... query your database? Of course you're going to get these kind of issues - that just seems obvious to me. Getting real off-topic, but the syntax is backwards too: ` Interface Votable implemented by Adventure, Comment, Post, Scenario ` The interface lists what implements it? Reminds me of COMEFROM[0]. I dunno. Modern front-end is wild. These live code Notebook things are chaos. Spaghetti begets spaghetti. [0]: https://en.wikipedia.org/wiki/COMEFROM https://en.wikipedia.org/wiki/COMEFROM

jcims 5y ago
That was my initial reaction to GraphQL as well, but just like a database you can do stored procedures or you can do SQL but both can still support access control if you need it. I think it's just a function of them not really thinking too much about the potential sensitivity of the data or building an access control model into the application.
colonwqbang 5y ago
GraphQL is just a standardised API model to access data, similar to REST. How you choose to implement the API is up to you. The fact that one developer was negligent in their implementation of a GraphQL type API does not really say anything about GraphQL itself. Many REST APIs have developed vulnerabilities over the years, but we should not draw the conclusion that REST itself is insecure or is a bad idea.
- duckerude 5y ago
  I think GraphQL might be unusually easy to accidentally get wrong. It handles very powerful untrusted input, by design. And it has enough features that many, maybe most users don't know about all of them. This vulnerability wouldn't have happened if GraphQL didn't have a certain feature. I've implemented a simple GraphQL API and I didn't know about all the features that were involved here. I thought carefully about security, and I think I ultimately got it right, but there were a few false starts. Good software is not just possible to use correctly, but easy to use correctly. Software that's easy to use correctly will be more secure in practice, even if it has the same theoretical properties. I don't know if GraphQL would be better if it were less powerful. I'm not an expert. But it's worth considering the possibility.
  - holtalanm 5y ago
    I honestly dont see any different between graphql and a REST api, in terms of what data is available where. if you have data you don't want publicly available, just.....don't include it in the model, and make sure your server implementation doesn't return it. It is possible I don't understand your comment, I suppose, but I really don't see what is so unique about graphql from a security standpoint.
    - duckerude 5y ago
      > don't include it in the model They tried that, but they failed because they didn't know that it's possible to downcast from an interface. It's really hard to have that kind of problem in a dumb REST API. `return {"name": record.name}` does what it says with hardly any magic. But if I write `return record` there's a whole extra layer that grabs information out of record, and I have to trust that it only grabs the information I want it to grab. This is not to say that dumb REST APIs are definitely better. Having to do things manually also introduces risks.
      - true_religion 5y ago
        If your return a record it only will have data for the fields that you have implemented. When you are implementing graphql, every resolver has to check if the requesting user has the proper permissions to see anything. Fields aren’t created automatically by the graphql engine itself. You have to write them yourself, or use another tool that generates them for you at run or compile time. Also it’s not often said, but you can have fields return a union such as AdminRecods and PublicRecord or Record WithoutPrivateData.
  - kbenson 5y ago
    Does GraphQL have a way to disable features? If so, it seems the sane way to go about implementing it for a set of data would be to enable the bare minimum features, and enable anything needed specifically after reviewing and researching what it allowed and how it interacted with other other features. If you can't, that seems like a very dangerous tool to use.
tmsbrg 5y ago
You can and should protect GraphQL on the backend. GraphQL will just forward requests to resolvers which actually get the data, these resolvers should see who's logged in and only return data this user has access to. I'd say the security model the AI dungeon GraphQL uses (depending on GraphQL interfaces to protect subfields of objects) is broken. These fields should never be returned to GraphQL by the backend in the first place, if you're not authorized.
- holtalanm 5y ago
  This. Your resolvers should still do access control. No different than a REST api would.

jcims 5y ago

That was my initial reaction to GraphQL as well, but just like a database you can do stored procedures or you can do SQL but both can still support access control if you need it. I think it's just a function of them not really thinking too much about the potential sensitivity of the data or building an access control model into the application.

colonwqbang 5y ago

GraphQL is just a standardised API model to access data, similar to REST. How you choose to implement the API is up to you. The fact that one developer was negligent in their implementation of a GraphQL type API does not really say anything about GraphQL itself. Many REST APIs have developed vulnerabilities over the years, but we should not draw the conclusion that REST itself is insecure or is a bad idea.

duckerude 5y ago
I think GraphQL might be unusually easy to accidentally get wrong. It handles very powerful untrusted input, by design. And it has enough features that many, maybe most users don't know about all of them. This vulnerability wouldn't have happened if GraphQL didn't have a certain feature. I've implemented a simple GraphQL API and I didn't know about all the features that were involved here. I thought carefully about security, and I think I ultimately got it right, but there were a few false starts. Good software is not just possible to use correctly, but easy to use correctly. Software that's easy to use correctly will be more secure in practice, even if it has the same theoretical properties. I don't know if GraphQL would be better if it were less powerful. I'm not an expert. But it's worth considering the possibility.
- holtalanm 5y ago
  I honestly dont see any different between graphql and a REST api, in terms of what data is available where. if you have data you don't want publicly available, just.....don't include it in the model, and make sure your server implementation doesn't return it. It is possible I don't understand your comment, I suppose, but I really don't see what is so unique about graphql from a security standpoint.
  - duckerude 5y ago
    > don't include it in the model They tried that, but they failed because they didn't know that it's possible to downcast from an interface. It's really hard to have that kind of problem in a dumb REST API. `return {"name": record.name}` does what it says with hardly any magic. But if I write `return record` there's a whole extra layer that grabs information out of record, and I have to trust that it only grabs the information I want it to grab. This is not to say that dumb REST APIs are definitely better. Having to do things manually also introduces risks.
    - true_religion 5y ago
      If your return a record it only will have data for the fields that you have implemented. When you are implementing graphql, every resolver has to check if the requesting user has the proper permissions to see anything. Fields aren’t created automatically by the graphql engine itself. You have to write them yourself, or use another tool that generates them for you at run or compile time. Also it’s not often said, but you can have fields return a union such as AdminRecods and PublicRecord or Record WithoutPrivateData.
- kbenson 5y ago
  Does GraphQL have a way to disable features? If so, it seems the sane way to go about implementing it for a set of data would be to enable the bare minimum features, and enable anything needed specifically after reviewing and researching what it allowed and how it interacted with other other features. If you can't, that seems like a very dangerous tool to use.

tmsbrg 5y ago

You can and should protect GraphQL on the backend. GraphQL will just forward requests to resolvers which actually get the data, these resolvers should see who's logged in and only return data this user has access to. I'd say the security model the AI dungeon GraphQL uses (depending on GraphQL interfaces to protect subfields of objects) is broken. These fields should never be returned to GraphQL by the backend in the first place, if you're not authorized.

holtalanm 5y ago
This. Your resolvers should still do access control. No different than a REST api would.