Skip to content

Commit

Permalink
[1.4.x] Added a default limit to the maximum number of forms in a for…
Browse files Browse the repository at this point in the history
…mset.

This is a security fix. Disclosure and advisory coming shortly.
  • Loading branch information
aaugustin authored and carljm committed Feb 19, 2013
1 parent 0e7861a commit 0cc350a
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 13 deletions.
12 changes: 10 additions & 2 deletions django/forms/formsets.py
Expand Up @@ -19,6 +19,9 @@
ORDERING_FIELD_NAME = 'ORDER' ORDERING_FIELD_NAME = 'ORDER'
DELETION_FIELD_NAME = 'DELETE' DELETION_FIELD_NAME = 'DELETE'


# default maximum number of forms in a formset, to prevent memory exhaustion
DEFAULT_MAX_NUM = 1000

class ManagementForm(Form): class ManagementForm(Form):
""" """
``ManagementForm`` is used to keep track of how many form instances ``ManagementForm`` is used to keep track of how many form instances
Expand Down Expand Up @@ -111,7 +114,7 @@ def initial_form_count(self):
def _construct_forms(self): def _construct_forms(self):
# instantiate all the forms and put them in self.forms # instantiate all the forms and put them in self.forms
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)) self.forms.append(self._construct_form(i))


def _construct_form(self, i, **kwargs): def _construct_form(self, i, **kwargs):
Expand Down Expand Up @@ -360,9 +363,14 @@ def as_ul(self):
def formset_factory(form, formset=BaseFormSet, extra=1, can_order=False, def formset_factory(form, formset=BaseFormSet, extra=1, can_order=False,
can_delete=False, max_num=None): can_delete=False, max_num=None):
"""Return a FormSet for the given form class.""" """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, attrs = {'form': form, 'extra': extra,
'can_order': can_order, 'can_delete': can_delete, '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) return type(form.__name__ + 'FormSet', (formset,), attrs)


def all_valid(formsets): def all_valid(formsets):
Expand Down
6 changes: 4 additions & 2 deletions docs/topics/forms/formsets.txt
Expand Up @@ -108,8 +108,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, 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``. 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 A ``max_num`` value of ``None`` (the default) puts a high limit on the number
forms displayed. Please note that the default value of ``max_num`` was changed 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. from ``0`` to ``None`` in version 1.2 to allow ``0`` as a valid value.


Formset validation Formset validation
Expand Down
4 changes: 2 additions & 2 deletions docs/topics/forms/modelforms.txt
Expand Up @@ -703,8 +703,8 @@ so long as the total number of forms does not exceed ``max_num``::


.. versionchanged:: 1.2 .. versionchanged:: 1.2


A ``max_num`` value of ``None`` (the default) puts no limit on the number of A ``max_num`` value of ``None`` (the default) puts a high limit on the number
forms displayed. of forms displayed (1000). In practice this is equivalent to no limit.


Using a model formset in a view Using a model formset in a view
------------------------------- -------------------------------
Expand Down
70 changes: 64 additions & 6 deletions tests/regressiontests/forms/tests/formsets.py
@@ -1,5 +1,5 @@
# -*- coding: utf-8 -*- # -*- 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.forms.formsets import formset_factory, BaseFormSet
from django.test import TestCase from django.test import TestCase


Expand Down Expand Up @@ -47,7 +47,7 @@ def test_basic_formset(self):
# for adding data. By default, it displays 1 blank form. It can display more, # for adding data. By default, it displays 1 blank form. It can display more,
# but we'll look at how to do so later. # but we'll look at how to do so later.
formset = ChoiceFormSet(auto_id=False, prefix='choices') formset = ChoiceFormSet(auto_id=False, prefix='choices')
self.assertHTMLEqual(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.assertHTMLEqual(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>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>""") <tr><th>Votes:</th><td><input type="text" name="choices-0-votes" /></td></tr>""")


Expand Down Expand Up @@ -650,8 +650,8 @@ def test_limiting_max_forms(self):
# Limiting the maximum number of forms ######################################## # Limiting the maximum number of forms ########################################
# Base case for max_num. # Base case for max_num.


# When not passed, max_num will take its default value of None, i.e. unlimited # 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. # number of forms only controlled by the value of the extra parameter.


LimitedFavoriteDrinkFormSet = formset_factory(FavoriteDrinkForm, extra=3) LimitedFavoriteDrinkFormSet = formset_factory(FavoriteDrinkForm, extra=3)
formset = LimitedFavoriteDrinkFormSet() formset = LimitedFavoriteDrinkFormSet()
Expand Down Expand Up @@ -698,8 +698,8 @@ def test_limiting_max_forms(self):
def test_max_num_with_initial_data(self): def test_max_num_with_initial_data(self):
# max_num with initial data # max_num with initial data


# When not passed, max_num will take its default value of None, i.e. unlimited # When not passed, max_num will take a high default value, leaving the
# number of forms, only controlled by the values of the initial and extra # number of forms only controlled by the value of the initial and extra
# parameters. # parameters.


initial = [ initial = [
Expand Down Expand Up @@ -844,6 +844,64 @@ def test_formset_nonzero(self):
self.assertEqual(len(formset.forms), 0) self.assertEqual(len(formset.forms), 0)
self.assertTrue(formset) self.assertTrue(formset)


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 = { data = {
'choices-TOTAL_FORMS': '1', # the number of forms rendered 'choices-TOTAL_FORMS': '1', # the number of forms rendered
Expand Down
3 changes: 2 additions & 1 deletion tests/regressiontests/generic_inline_admin/tests.py
Expand Up @@ -7,6 +7,7 @@
from django.contrib.admin.sites import AdminSite from django.contrib.admin.sites import AdminSite
from django.contrib.contenttypes.generic import ( from django.contrib.contenttypes.generic import (
generic_inlineformset_factory, GenericTabularInline) generic_inlineformset_factory, GenericTabularInline)
from django.forms.formsets import DEFAULT_MAX_NUM
from django.forms.models import ModelForm from django.forms.models import ModelForm
from django.test import TestCase from django.test import TestCase


Expand Down Expand Up @@ -241,7 +242,7 @@ def test_get_formset_kwargs(self):


# Create a formset with default arguments # Create a formset with default arguments
formset = media_inline.get_formset(request) formset = media_inline.get_formset(request)
self.assertEqual(formset.max_num, None) self.assertEqual(formset.max_num, DEFAULT_MAX_NUM)
self.assertEqual(formset.can_order, False) self.assertEqual(formset.can_order, False)


# Create a formset with custom keyword arguments # Create a formset with custom keyword arguments
Expand Down

0 comments on commit 0cc350a

Please sign in to comment.