Commit bcfb4778 authored by Tim Graham's avatar Tim Graham
Browse files

[1.7.x] Fixed DoS possibility in ModelMultipleChoiceField. 

This is a security fix. Disclosure following shortly.

Thanks Keryn Knight for the report and initial patch.
parent 818e59a3

django/forms/models.py

+23 −5
Original line number Diff line number Diff line
@@ -1221,8 +1221,7 @@ class ModelMultipleChoiceField(ModelChoiceField): 
    def to_python(self, value):

        if not value:

            return []

        to_py = super(ModelMultipleChoiceField, self).to_python

        return [to_py(val) for val in value]

        return list(self._check_values(value))



    def clean(self, value):

        if self.required and not value:
@@ -1231,7 +1230,29 @@ class ModelMultipleChoiceField(ModelChoiceField): 
            return self.queryset.none()

        if not isinstance(value, (list, tuple)):

            raise ValidationError(self.error_messages['list'], code='list')

        qs = self._check_values(value)

        # Since this overrides the inherited ModelChoiceField.clean

        # we run custom validators here

        self.run_validators(value)

        return qs



    def _check_values(self, value):

        """

        Given a list of possible PK values, returns a QuerySet of the

        corresponding objects. Raises a ValidationError if a given value is

        invalid (not a valid PK, not in the queryset, etc.)

        """

        key = self.to_field_name or 'pk'

        # deduplicate given values to avoid creating many querysets or

        # requiring the database backend deduplicate efficiently.

        try:

            value = frozenset(value)

        except TypeError:

            # list of lists isn't hashable, for example

            raise ValidationError(

                self.error_messages['list'],

                code='list',

            )

        for pk in value:

            try:

                self.queryset.filter(**{key: pk})
@@ -1250,9 +1271,6 @@ class ModelMultipleChoiceField(ModelChoiceField): 
                    code='invalid_choice',

                    params={'value': val},

                )

        # Since this overrides the inherited ModelChoiceField.clean

        # we run custom validators here

        self.run_validators(value)

        return qs



    def prepare_value(self, value):

docs/releases/1.6.10.txt

+9 −0
Original line number Diff line number Diff line
@@ -58,3 +58,12 @@ Note, however, that this view has always carried a warning that it is not 
hardened for production use and should be used only as a development aid. Now

may be a good time to audit your project and serve your files in production

using a real front-end web server if you are not doing so.



Database denial-of-service with ``ModelMultipleChoiceField``

============================================================



Given a form that uses ``ModelMultipleChoiceField`` and

``show_hidden_initial=True`` (not a documented API), it was possible for a user

to cause an unreasonable number of SQL queries by submitting duplicate values

for the field's data. The validation logic in ``ModelMultipleChoiceField`` now

deduplicates submitted values to address this issue.

docs/releases/1.7.3.txt

+9 −0
Original line number Diff line number Diff line
@@ -59,6 +59,15 @@ hardened for production use and should be used only as a development aid. Now 
may be a good time to audit your project and serve your files in production

using a real front-end web server if you are not doing so.



Database denial-of-service with ``ModelMultipleChoiceField``

============================================================



Given a form that uses ``ModelMultipleChoiceField`` and

``show_hidden_initial=True`` (not a documented API), it was possible for a user

to cause an unreasonable number of SQL queries by submitting duplicate values

for the field's data. The validation logic in ``ModelMultipleChoiceField`` now

deduplicates submitted values to address this issue.



Bugfixes

========

docs/spelling_wordlist

+1 −0
Original line number Diff line number Diff line
@@ -134,6 +134,7 @@ dbshell 
de

deconstruct

deconstructing

deduplicates

deepcopy

deserialization

deserialize

tests/model_forms/tests.py

+21 −0
Original line number Diff line number Diff line
@@ -1573,6 +1573,27 @@ class ModelMultipleChoiceFieldTests(TestCase): 
        self.assertTrue(form.is_valid())

        self.assertTrue(form.has_changed())



    def test_show_hidden_initial_changed_queries_efficiently(self):

        class WriterForm(forms.Form):

            persons = forms.ModelMultipleChoiceField(

                show_hidden_initial=True, queryset=Writer.objects.all())



        writers = (Writer.objects.create(name=str(x)) for x in range(0, 50))

        writer_pks = tuple(x.pk for x in writers)

        form = WriterForm(data={'initial-persons': writer_pks})

        with self.assertNumQueries(1):

            self.assertTrue(form.has_changed())



    def test_clean_does_deduplicate_values(self):

        class WriterForm(forms.Form):

            persons = forms.ModelMultipleChoiceField(queryset=Writer.objects.all())



        person1 = Writer.objects.create(name="Person 1")

        form = WriterForm(data={})

        queryset = form.fields['persons'].clean([str(person1.pk)] * 50)

        sql, params = queryset.query.sql_with_params()

        self.assertEqual(len(params), 1)





class ModelOneToOneFieldTests(TestCase):

    def test_modelform_onetoonefield(self):