[1.3.x] Added a default limit to the maximum number of forms in a formset.

aaugustin · aaugustin · commit d7094bbce8cb · 2013-02-12T12:13:42.000+01:00
This is a security fix. Disclosure and advisory coming shortly.
diff --git a/django/forms/formsets.py b/django/forms/formsets.py
@@ -16,6 +16,9 @@
 ORDERING_FIELD_NAME = 'ORDER'
 DELETION_FIELD_NAME = 'DELETE'
 
+# default maximum number of forms in a formset, to prevent memory exhaustion
+DEFAULT_MAX_NUM = 1000
+
 class ManagementForm(Form):
     """
     ``ManagementForm`` is used to keep track of how many form instances
@@ -104,7 +107,7 @@ def initial_form_count(self):
     def _construct_forms(self):
         # instantiate all the forms and put them in self.forms
         self.forms = []
-        for i in xrange(self.total_form_count()):
+        for i in xrange(min(self.total_form_count(), self.absolute_max)):
             self.forms.append(self._construct_form(i))
 
     def _construct_form(self, i, **kwargs):
@@ -348,9 +351,14 @@ def as_ul(self):
 def formset_factory(form, formset=BaseFormSet, extra=1, can_order=False,
                     can_delete=False, max_num=None):
     """Return a FormSet for the given form class."""
+    if max_num is None:
+        max_num = DEFAULT_MAX_NUM
+    # hard limit on forms instantiated, to prevent memory-exhaustion attacks
+    # limit defaults to DEFAULT_MAX_NUM, but developer can increase it via max_num
+    absolute_max = max(DEFAULT_MAX_NUM, max_num)
     attrs = {'form': form, 'extra': extra,
              'can_order': can_order, 'can_delete': can_delete,
-             'max_num': max_num}
+             'max_num': max_num, 'absolute_max': absolute_max}
     return type(form.__name__ + 'FormSet', (formset,), attrs)
 
 def all_valid(formsets):
diff --git a/docs/topics/forms/formsets.txt b/docs/topics/forms/formsets.txt
@@ -102,8 +102,10 @@ If the value of ``max_num`` is greater than the number of existing
 objects, up to ``extra`` additional blank forms will be added to the formset,
 so long as the total number of forms does not exceed ``max_num``.
 
-A ``max_num`` value of ``None`` (the default) puts no limit on the number of
-forms displayed. Please note that the default value of ``max_num`` was changed
+A ``max_num`` value of ``None`` (the default) puts a high limit on the number
+of forms displayed (1000). In practice this is equivalent to no limit.
+
+Please note that the default value of ``max_num`` was changed
 from ``0`` to ``None`` in version 1.2 to allow ``0`` as a valid value.
 
 Formset validation
diff --git a/docs/topics/forms/modelforms.txt b/docs/topics/forms/modelforms.txt
@@ -691,8 +691,8 @@ so long as the total number of forms does not exceed ``max_num``::
 
 .. versionchanged:: 1.2
 
-A ``max_num`` value of ``None`` (the default) puts no limit on the number of
-forms displayed.
+A ``max_num`` value of ``None`` (the default) puts a high limit on the number
+of forms displayed (1000). In practice this is equivalent to no limit.
 
 Using a model formset in a view
 -------------------------------
diff --git a/tests/regressiontests/forms/tests/formsets.py b/tests/regressiontests/forms/tests/formsets.py
@@ -1,5 +1,5 @@
 # -*- coding: utf-8 -*-
-from django.forms import Form, CharField, IntegerField, ValidationError, DateField
+from django.forms import Form, CharField, IntegerField, ValidationError, DateField, formsets
 from django.forms.formsets import formset_factory, BaseFormSet
 from django.utils.unittest import TestCase
 
@@ -47,7 +47,7 @@ def test_basic_formset(self):
         # for adding data. By default, it displays 1 blank form. It can display more,
         # but we'll look at how to do so later.
         formset = ChoiceFormSet(auto_id=False, prefix='choices')
-        self.assertEqual(str(formset), """<input type="hidden" name="choices-TOTAL_FORMS" value="1" /><input type="hidden" name="choices-INITIAL_FORMS" value="0" /><input type="hidden" name="choices-MAX_NUM_FORMS" />
+        self.assertEqual(str(formset), """<input type="hidden" name="choices-TOTAL_FORMS" value="1" /><input type="hidden" name="choices-INITIAL_FORMS" value="0" /><input type="hidden" name="choices-MAX_NUM_FORMS" value="1000" />
 <tr><th>Choice:</th><td><input type="text" name="choices-0-choice" /></td></tr>
 <tr><th>Votes:</th><td><input type="text" name="choices-0-votes" /></td></tr>""")
 
@@ -623,8 +623,8 @@ def test_limiting_max_forms(self):
         # Limiting the maximum number of forms ########################################
         # Base case for max_num.
 
-        # When not passed, max_num will take its default value of None, i.e. unlimited
-        # number of forms, only controlled by the value of the extra parameter.
+        # When not passed, max_num will take a high default value, leaving the
+        # number of forms only controlled by the value of the extra parameter.
 
         LimitedFavoriteDrinkFormSet = formset_factory(FavoriteDrinkForm, extra=3)
         formset = LimitedFavoriteDrinkFormSet()
@@ -671,8 +671,8 @@ def test_limiting_max_forms(self):
     def test_max_num_with_initial_data(self):
         # max_num with initial data
 
-        # When not passed, max_num will take its default value of None, i.e. unlimited
-        # number of forms, only controlled by the values of the initial and extra
+        # When not passed, max_num will take a high default value, leaving the
+        # number of forms only controlled by the value of the initial and extra
         # parameters.
 
         initial = [
@@ -805,6 +805,65 @@ def __iter__(self):
         self.assertEqual(str(reverse_formset[1]), str(forms[-2]))
         self.assertEqual(len(reverse_formset), len(forms))
 
+    def test_hard_limit_on_instantiated_forms(self):
+        """A formset has a hard limit on the number of forms instantiated."""
+        # reduce the default limit of 1000 temporarily for testing
+        _old_DEFAULT_MAX_NUM = formsets.DEFAULT_MAX_NUM
+        try:
+            formsets.DEFAULT_MAX_NUM = 3
+            ChoiceFormSet = formset_factory(Choice)
+            # someone fiddles with the mgmt form data...
+            formset = ChoiceFormSet(
+                {
+                    'choices-TOTAL_FORMS': '4',
+                    'choices-INITIAL_FORMS': '0',
+                    'choices-MAX_NUM_FORMS': '4',
+                    'choices-0-choice': 'Zero',
+                    'choices-0-votes': '0',
+                    'choices-1-choice': 'One',
+                    'choices-1-votes': '1',
+                    'choices-2-choice': 'Two',
+                    'choices-2-votes': '2',
+                    'choices-3-choice': 'Three',
+                    'choices-3-votes': '3',
+                    },
+                prefix='choices',
+                )
+            # But we still only instantiate 3 forms
+            self.assertEqual(len(formset.forms), 3)
+        finally:
+            formsets.DEFAULT_MAX_NUM = _old_DEFAULT_MAX_NUM
+
+    def test_increase_hard_limit(self):
+        """Can increase the built-in forms limit via a higher max_num."""
+        # reduce the default limit of 1000 temporarily for testing
+        _old_DEFAULT_MAX_NUM = formsets.DEFAULT_MAX_NUM
+        try:
+            formsets.DEFAULT_MAX_NUM = 3
+            # for this form, we want a limit of 4
+            ChoiceFormSet = formset_factory(Choice, max_num=4)
+            formset = ChoiceFormSet(
+                {
+                    'choices-TOTAL_FORMS': '4',
+                    'choices-INITIAL_FORMS': '0',
+                    'choices-MAX_NUM_FORMS': '4',
+                    'choices-0-choice': 'Zero',
+                    'choices-0-votes': '0',
+                    'choices-1-choice': 'One',
+                    'choices-1-votes': '1',
+                    'choices-2-choice': 'Two',
+                    'choices-2-votes': '2',
+                    'choices-3-choice': 'Three',
+                    'choices-3-votes': '3',
+                    },
+                prefix='choices',
+                )
+            # This time four forms are instantiated
+            self.assertEqual(len(formset.forms), 4)
+        finally:
+            formsets.DEFAULT_MAX_NUM = _old_DEFAULT_MAX_NUM
+
+
 data = {
     'choices-TOTAL_FORMS': '1', # the number of forms rendered
     'choices-INITIAL_FORMS': '0', # the number of forms with initial data
@@ -900,12 +959,12 @@ def test_empty_forms_are_unbound(self):
         # The empty forms should be equal.
         self.assertEqual(empty_forms[0].as_p(), empty_forms[1].as_p())
 
-class TestEmptyFormSet(TestCase): 
+class TestEmptyFormSet(TestCase):
     "Test that an empty formset still calls clean()"
-    def test_empty_formset_is_valid(self): 
-        EmptyFsetWontValidateFormset = formset_factory(FavoriteDrinkForm, extra=0, formset=EmptyFsetWontValidate) 
-        formset = EmptyFsetWontValidateFormset(data={'form-INITIAL_FORMS':'0', 'form-TOTAL_FORMS':'0'},prefix="form") 
-        formset2 = EmptyFsetWontValidateFormset(data={'form-INITIAL_FORMS':'0', 'form-TOTAL_FORMS':'1', 'form-0-name':'bah' },prefix="form") 
-        self.assertFalse(formset.is_valid()) 
-        self.assertFalse(formset2.is_valid()) 
+    def test_empty_formset_is_valid(self):
+        EmptyFsetWontValidateFormset = formset_factory(FavoriteDrinkForm, extra=0, formset=EmptyFsetWontValidate)
+        formset = EmptyFsetWontValidateFormset(data={'form-INITIAL_FORMS':'0', 'form-TOTAL_FORMS':'0'},prefix="form")
+        formset2 = EmptyFsetWontValidateFormset(data={'form-INITIAL_FORMS':'0', 'form-TOTAL_FORMS':'1', 'form-0-name':'bah' },prefix="form")
+        self.assertFalse(formset.is_valid())
+        self.assertFalse(formset2.is_valid())
 
