Graphql-engine: BUG: null value in where expression leads to unexpected result

[TAnas0]

Hey Hasura team,

I am writing this to ask about a behavior I encountered with Hasura.

Basically, I came across this trying to update a single specific entity referenced by its ID, using a mutation with a where filter. If I pass that mutation the value null, it updates all of the entities in that table, which I believe might be a problem for some use cases, or even an attack vector.

To sum it all up, there is nothing better than a screenshot:

The expected behavior in this case, at least IMO, is to make no changes at all, since no entity has an ID equal to null.

I may be just hallucinating, but I think I saw this as a bug fix in the changelog someday, I just can't find anymore

Cheers

Hasura version: v1.0.0-beta.10
Postgres version: Postgres 11 (managed)

[rakeshkky]

Hi @TAnas0, Thank you for opening an issue.

If you want to filter based on whether a column value is null, you can make use of is_null operator. See docs.

[TAnas0]

Hey @rakeshkky,

Thanks for answering but that is not my issue. I am not trying to filter based on a value being null.

The problem I have is that the screenshoted mutation should not execute on any entity, because none of them have a null ID (my primary key).

[ecthiender]

@TAnas0 the problem is the expression {id: {_eq: null}} will roughly translate to the expression not being there. So in your case, the entire where expression evaluates to true, hence it updates all the rows.

You should take a variable for your mutation and make it mandatory. Like:

mutation updateFirstAudit ($id: Int!) {
  update_first_audit(
    where: {id: {_eq: $id}}
   .....

See this https://github.com/hasura/graphql-engine/issues/2648#issuecomment-517137522 for more details.

[TAnas0]

@rikinsk Thank you, but again, I am not trying to filter based on an attribute being null.

@ecthiender Thank you
Do you think it makes sense that the expression {id: {_eq: null}} translates to "the expression not being there"? What do you mean exactly by that? I think it should simply mean that the id is null.

Anyway, I will use your tip on requiring that parameter to not be null on my frontend mutation.
If you have an idea about Hasura's internals, and how it translates that mutation to an SQL statement, please make sure that there is no misunderstanding in the translation.

Also note that your tip on requiring that parameter only works for me because that ID is not nullable. If we were to be dealing with a filter value that is nullable, then we would be in the same problem again. For example, the mutation below, filtered on the nullable attribute exit_code, applies to all the entities, even the ones with an exit_code not null (See the first element returned by the mutation):

P.S: and please I insist on this being more than a question. Maybe not a bug technically, but serious nonetheless.

[yasinarik]

@TAnas0 If I don't misunderstand, you are saying that this behavior where you try to filter by
_eq: null is not valid and shouldn't be accepted, right?

I agree with you on that and I think that should turn into an error instead.

[leoalves]

@yasinarik

Check this issue:
https://github.com/hasura/graphql-engine/issues/704

[TAnas0]

@yasinarik No, I am afraid we are not on the same page.

My point is that, if I specify a where filter of this type {myAttribute: {_eq: null}}, then my changes should happen to all, and only all the entities that have null as value for the attribute myAttribute. Which is currently not the case, as seen in both the screenshots above, where entities that do not have the attribute exit_code or id as null, get updated too. In fact, these mutations update ALL the entities of the table, which I find very weird and even a security problem.

I would like to see someone trying this out on their Hasura instance too.

And much much thanks to you @yasinarik ! That's what I was referring to in the original message. I thought I saw that as fixed in the changelog. I spent much more time than I can admit looking for that :smile:

[yasinarik]

@TAnas0 @leoalves I've checked #704 .

I'm very confused at the moment.

query getUsers {
  users(where: {id: {_eq: null}}) {
    id
    username
  }
}

I have tested this on my own db and this returns all the users. Why? I always thought that this should return an error or an empty array:

{
  "data": {
    "users": []
  }
}

If somehow, a null parameter is passed instead of an actual UUID into the (where: {id: {_eq: null}} clause from my front-end, it will cause serious problems including security.

I don't think that I understand the correct implementation.

[ecthiender]

@yasinarik as others have mentioned, the implementation is explained in #704 and https://github.com/hasura/graphql-engine/issues/2648#issuecomment-517137522

[TAnas0]

@yasinarik Yeah, that's exactly my point.

@ecthiender I know that this could be a huge breaking change, but can we at least have that documented somewhere/somehow?

EDIT: Closing since the Hasura team seems to be aware of this and working to resolve it