Prevent contains to be executed on arbitrary fields (#3563)

* Prevent contains to be executed on arbitrary fields

* Fixing browser test and updating browser while we're here

---------

Co-authored-by: Alex Cottner <acottner@mozilla.com>

Author: leplatrem

kinto/core/resource/__init__.py

@@ -1135,6 +1135,14 @@ def is_valid_timestamp(value):
             if field == self.model.modified_field and not is_valid_timestamp(value):
                 raise_invalid(self.request, **error_details)
 
+            if field in (self.model.modified_field, self.model.id_field) and operator in (
+                COMPARISON.CONTAINS,
+                COMPARISON.CONTAINS_ANY,
+            ):
+                error_msg = f"Field '{field}' is not an array"
+                error_details["description"] = error_msg
+                raise_invalid(self.request, **error_details)
+
             filters.append(Filter(field, value, operator))
 
         # If a plural endpoint is reached, and if the user does not have the

kinto/core/storage/postgresql/__init__.py

@@ -884,6 +884,14 @@ def _format_conditions(self, filters, id_field, modified_field, prefix="filters"
                 operator = "IS NOT NULL" if filtr.value else "IS NULL"
                 cond = f"{sql_field} {operator}"
 
+            elif filtr.operator == COMPARISON.CONTAINS:
+                value_holder = f"{prefix}_value_{i}"
+                holders[value_holder] = value
+                # In case the field is not a sequence, we ignore the object.
+                is_json_sequence = f"jsonb_typeof({sql_field}) = 'array'"
+                sql_operator = operators[filtr.operator]
+                cond = f"{is_json_sequence} AND {sql_field} {sql_operator} :{value_holder}"
+
             elif filtr.operator == COMPARISON.CONTAINS_ANY:
                 value_holder = f"{prefix}_value_{i}"
                 holders[value_holder] = value

kinto/core/storage/testing.py

@@ -491,6 +491,14 @@ def test_list_all_can_filter_on_array_with_contains_and_unsupported_type(self):
         objects = self.storage.list_all(filters=filters, **self.storage_kw)
         self.assertEqual(len(objects), 0)
 
+    def test_list_all_contains_ignores_object_if_field_is_not_array(self):
+        self.create_object({"code": "black"})
+        self.create_object({"fib": ["a", "b", "c"]})
+
+        filters = [Filter("fib", ["a"], utils.COMPARISON.CONTAINS)]
+        objects = self.storage.list_all(filters=filters, **self.storage_kw)
+        self.assertEqual(len(objects), 1)
+
     def test_list_all_can_filter_on_array_with_contains_any_and_unsupported_type(self):
         self.create_object({"code": "black"})
         self.create_object({"fib": [2, 3, 5]})

tests/browser.ini

@@ -7,6 +7,8 @@ port = %(http_port)s
 [app:main]
 use = egg:kinto
 
+kinto.project_name = Kinto
+
 #
 # Backends.
 #

tests/core/resource/test_filter.py

@@ -339,6 +339,10 @@ def test_contains_any_fails_on_a_non_sequence_object_value(self):
         values = result["data"]
         assert len(values) == 0
 
+    def test_contains_returns_400_if_field_is_id_or_last_modified(self):
+        self.validated["querystring"] = {"contains_id": "foo"}
+        self.assertRaises(httpexceptions.HTTPBadRequest, self.resource.plural_get)
+
 
 class SubobjectFilteringTest(BaseTest):
     def setUp(self):