Question: Safe to expose Smart Query?

[TheNoim]

Hello,

I started to use Mikro-orm and I really like it. I saw Mikro-orm supports a kind of query language which works with plain JSON objects: https://mikro-orm.io/docs/query-conditions

I asked my self, is it safe to expose this smart query? For example, allow to pass it via a JSON body from an HTTP request? Or is it to easy to inject SQL with this? I mean for my current application I will do it anyway because it is just a small side project and not public for the internet. However, this would be cool to know for future projects.

Greetings, Nils

[B4nan]

Nope, you should not let API users to be able to form the query, only values should be used. You could end up with attacker using some operator or negation, disabling some checks, using in/not in instead of =, etc...

For internal endpoints it's fine I guess, as long as you know the caller can be trusted (like if it's some script, another microservice of your own).

[TheNoim]

const res = await orm.em.find(Author, { $and: [
  { 'id:in': [1, 2, 7] },
  ...customFilter // Only add additional and filters
] });

But how could an attacker disable some checks with an approach like this? It could only add some checks. At least this is how I understand it. Or does this smart query builder combine this and remove duplicates? This would lead to behaviour like you described. I was a bit more concerned about SQL Injections.

[B4nan]

Yeah that approach should be safe, I was talking about using the query as value, so you would have query param that would be used instead of the whole query you have (or as a direct value, e.g. id: objectFromQuery).

Anyway, the approach with operator in key is deprecated and will be removed in v5, mainly because it can be dangerous when used without proper input validation.

[TheNoim]

Ok, so it is safe from SQL Injections and it doesn't collapse the whole query and removes duplicated things? This would be fine for me.

[B4nan]

Btw you don't even need $in operator here, as that is implicit if the value is array. id: [...] is same as id: { $in: [...] }.

Wait, so the whole Smart Query gets deprecated or only some parts?

No, I said operator in key approach, so 'id:in' etc.

[B4nan]

Not sure what you mean by duplicates and collapsing, do you have an example?

[TheNoim]

Btw you don't even need $in operator here, as that is implicit if the value is array. id: [...] is same as id: { $in: [...] }.

Yeah, I just copied this from the docs. I also like the approach with the $ operators more.

[TheNoim]

Not sure what you mean by duplicates and collapsing, do you have an example?

I meant something like this:

const res = await orm.em.find(Author, { $and: [
  { id: { $in: [1, 2, 7] }, }, // "duplicated"
  { id: { $nin: [1, 2, 7] }, } 
] });

// gets resolved to
const res = await orm.em.find(Author, { $and: [
  { id: { $nin: [1, 2, 7] }, } 
] });

But if this is not like in my example then I am fine.

[B4nan]

Yeah that is safe, nothing will be removed from the $and subqueries. On the other hand, if you use and object, it would:

const res = await orm.em.find(Author, {
  id: { $in: [1, 2, 7] },
  ...query, // query is `{ id: { $ne: 0 } }`
] });

Here it would overried the id key, but that is how JS works, nothing ORM related (and can be fixed by using the query param first).

[TheNoim]

Yeah that is safe, nothing will be removed from the $and subqueries. On the other hand, if you use and object, it would:

const res = await orm.em.find(Author, {
  id: { $in: [1, 2, 7] },
  ...query, // query is `{ id: { $ne: 0 } }`
] });

Here it would overried the id key, but that is how JS works, nothing ORM related (and can be fixed by using the query param first).

Yeah, that's true.

Thanks for the clarification.