microsoft / spring-data-cosmosdb

Access data with Azure Cosmos DB
MIT License
93 stars 68 forks source link

escaping querying with IN clause #533

Closed chrisgolle closed 4 years ago

chrisgolle commented 4 years ago

having a method signature like the following: List<Object> findAllByPropIn(Collection<String> props) works good. However, inputs are not parameterized so an input such as bahhhh') OR (r.id > 'a will return all documents and use up all my RU's pretty fast. Am i responsible for escaping inputs, and if so, how are inputs escaped? It looks like other methods are susceptible to the same issue.

kushagraThapar commented 4 years ago

@chrisgolle can you please explain more on what you are trying to achieve with a sample may be?

chrisgolle commented 4 years ago

sry, my first comment wasnt very clear at all. im not trying to achieve anything really, more pointing out that I think the library has an issue where it is not using parameterized queries. I think users of this library expect inputs to be escaped/parameterized for them. Read more about it here. https://owasp.org/www-community/attacks/SQL_Injection

Throughout the library queries are constructed using String.format("some query %s some params %s", parama, paramb) or String.join ("," parameters). I dont think query parameters should be manually concatenated to the query strings.

In the example List<Object> findAllByPropIn(Collection<String> props)

if someone calls this method with the following argument:

findAllByPropIn(Collections.singleton("bahhhh') OR (r.id > 'a"))

the library will generate and execute the following query:

SELECT * FROM ROOT r WHERE r.prop IN ('bahhhh') OR (r.id > 'a')

Instead, I i think it should generate something like the following (or array_contains instead, but that isnt the issue.)

SELECT * FROM ROOT r WHERE r.prop IN @params and pass cosmos the parameter separately. https://docs.microsoft.com/en-us/azure/cosmos-db/sql-query-parameterized-queries

kushagraThapar commented 4 years ago

@chrisgolle Thats a valid point, I will investigate this and see if we can change the query construction to be parameterized query APIs instead of using string concatenation.

kushagraThapar commented 4 years ago

Closing it here as this is being tracked in a separate repo.