티스토리 수익 글 보기

티스토리 수익 글 보기

[1.5.x] Added a default limit to the maximum number of forms in a for… · django/django@3ef4bbf · GitHub
Skip to content

Commit 3ef4bbf

Browse files
aaugustincarljm
aaugustin
authored and
carljm
committed
[1.5.x] Added a default limit to the maximum number of forms in a formset.
This is a security fix. Disclosure and advisory coming shortly.
1 parent 0e46c7f commit 3ef4bbf

File tree

5 files changed

+85
–19
lines changed

5 files changed

+85
–19
lines changed

‎django/forms/formsets.py‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@
2121
ORDERING_FIELD_NAME = 'ORDER'
2222
DELETION_FIELD_NAME = 'DELETE'
2323

24+
# default maximum number of forms in a formset, to prevent memory exhaustion
25+
DEFAULT_MAX_NUM = 1000
26+
2427
class ManagementForm(Form):
2528
"""
2629
``ManagementForm`` is used to keep track of how many form instances
@@ -97,11 +100,10 @@ def total_form_count(self):
97100
total_forms = initial_forms + self.extra
98101
# Allow all existing related objects/inlines to be displayed,
99102
# but don't allow extra beyond max_num.
100-
if self.max_num is not None:
101-
if initial_forms > self.max_num >= 0:
102-
total_forms = initial_forms
103-
elif total_forms > self.max_num >= 0:
104-
total_forms = self.max_num
103+
if initial_forms > self.max_num >= 0:
104+
total_forms = initial_forms
105+
elif total_forms > self.max_num >= 0:
106+
total_forms = self.max_num
105107
return total_forms
106108

107109
def initial_form_count(self):
@@ -111,14 +113,14 @@ def initial_form_count(self):
111113
else:
112114
# Use the length of the inital data if it's there, 0 otherwise.
113115
initial_forms = self.initial and len(self.initial) or 0
114-
if self.max_num is not None and (initial_forms > self.max_num >= 0):
116+
if initial_forms > self.max_num >= 0:
115117
initial_forms = self.max_num
116118
return initial_forms
117119

118120
def _construct_forms(self):
119121
# instantiate all the forms and put them in self.forms
120122
self.forms = []
121-
for i in xrange(self.total_form_count()):
123+
for i in xrange(min(self.total_form_count(), self.absolute_max)):
122124
self.forms.append(self._construct_form(i))
123125

124126
def _construct_form(self, i, **kwargs):
@@ -368,9 +370,14 @@ def as_ul(self):
368370
def formset_factory(form, formset=BaseFormSet, extra=1, can_order=False,
369371
can_delete=False, max_num=None):
370372
"""Return a FormSet for the given form class."""
373+
if max_num is None:
374+
max_num = DEFAULT_MAX_NUM
375+
# hard limit on forms instantiated, to prevent memory-exhaustion attacks
376+
# limit defaults to DEFAULT_MAX_NUM, but developer can increase it via max_num
377+
absolute_max = max(DEFAULT_MAX_NUM, max_num)
371378
attrs = {'form': form, 'extra': extra,
372379
'can_order': can_order, 'can_delete': can_delete,
373-
'max_num': max_num}
380+
'max_num': max_num, 'absolute_max': absolute_max}
374381
return type(form.__name__ + str('FormSet'), (formset,), attrs)
375382

376383
def all_valid(formsets):

‎docs/topics/forms/formsets.txt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,8 +98,8 @@ If the value of ``max_num`` is greater than the number of existing
9898
objects, up to ``extra`` additional blank forms will be added to the formset,
9999
so long as the total number of forms does not exceed ``max_num``.
100100

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

104104
Formset validation
105105
------------------

‎docs/topics/forms/modelforms.txt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -727,8 +727,8 @@ so long as the total number of forms does not exceed ``max_num``::
727727
<tr><th><label for="id_form-2-name">Name:</label></th><td><input id="id_form-2-name" type="text" name="form-2-name" value="Walt Whitman" maxlength="100" /><input type="hidden" name="form-2-id" value="2" id="id_form-2-id" /></td></tr>
728728
<tr><th><label for="id_form-3-name">Name:</label></th><td><input id="id_form-3-name" type="text" name="form-3-name" maxlength="100" /><input type="hidden" name="form-3-id" id="id_form-3-id" /></td></tr>
729729

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

733733
Using a model formset in a view
734734
-------------------------------

‎tests/regressiontests/forms/tests/formsets.py‎

Lines changed: 64 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
from __future__ import unicode_literals
33

44
from django.forms import (CharField, DateField, FileField, Form, IntegerField,
5-
ValidationError)
5+
ValidationError, formsets)
66
from django.forms.formsets import BaseFormSet, formset_factory
77
from django.forms.util import ErrorList
88
from django.test import TestCase
@@ -51,7 +51,7 @@ def test_basic_formset(self):
5151
# for adding data. By default, it displays 1 blank form. It can display more,
5252
# but we'll look at how to do so later.
5353
formset = ChoiceFormSet(auto_id=False, prefix='choices')
54-
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" />
54+
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" />
5555
<tr><th>Choice:</th><td><input type="text" name="choices-0-choice" /></td></tr>
5656
<tr><th>Votes:</th><td><input type="text" name="choices-0-votes" /></td></tr>""")
5757

@@ -654,8 +654,8 @@ def test_limiting_max_forms(self):
654654
# Limiting the maximum number of forms ########################################
655655
# Base case for max_num.
656656

657-
# When not passed, max_num will take its default value of None, i.e. unlimited
658-
# number of forms, only controlled by the value of the extra parameter.
657+
# When not passed, max_num will take a high default value, leaving the
658+
# number of forms only controlled by the value of the extra parameter.
659659

660660
LimitedFavoriteDrinkFormSet = formset_factory(FavoriteDrinkForm, extra=3)
661661
formset = LimitedFavoriteDrinkFormSet()
@@ -702,8 +702,8 @@ def test_limiting_max_forms(self):
702702
def test_max_num_with_initial_data(self):
703703
# max_num with initial data
704704

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

709709
initial = [
@@ -878,6 +878,64 @@ def is_valid(self):
878878
self.assertTrue(formset.is_valid())
879879
self.assertTrue(all([form.is_valid_called for form in formset.forms]))
880880

881+
def test_hard_limit_on_instantiated_forms(self):
882+
"""A formset has a hard limit on the number of forms instantiated."""
883+
# reduce the default limit of 1000 temporarily for testing
884+
_old_DEFAULT_MAX_NUM = formsets.DEFAULT_MAX_NUM
885+
try:
886+
formsets.DEFAULT_MAX_NUM = 3
887+
ChoiceFormSet = formset_factory(Choice)
888+
# someone fiddles with the mgmt form data...
889+
formset = ChoiceFormSet(
890+
{
891+
'choices-TOTAL_FORMS': '4',
892+
'choices-INITIAL_FORMS': '0',
893+
'choices-MAX_NUM_FORMS': '4',
894+
'choices-0-choice': 'Zero',
895+
'choices-0-votes': '0',
896+
'choices-1-choice': 'One',
897+
'choices-1-votes': '1',
898+
'choices-2-choice': 'Two',
899+
'choices-2-votes': '2',
900+
'choices-3-choice': 'Three',
901+
'choices-3-votes': '3',
902+
},
903+
prefix='choices',
904+
)
905+
# But we still only instantiate 3 forms
906+
self.assertEqual(len(formset.forms), 3)
907+
finally:
908+
formsets.DEFAULT_MAX_NUM = _old_DEFAULT_MAX_NUM
909+
910+
def test_increase_hard_limit(self):
911+
"""Can increase the built-in forms limit via a higher max_num."""
912+
# reduce the default limit of 1000 temporarily for testing
913+
_old_DEFAULT_MAX_NUM = formsets.DEFAULT_MAX_NUM
914+
try:
915+
formsets.DEFAULT_MAX_NUM = 3
916+
# for this form, we want a limit of 4
917+
ChoiceFormSet = formset_factory(Choice, max_num=4)
918+
formset = ChoiceFormSet(
919+
{
920+
'choices-TOTAL_FORMS': '4',
921+
'choices-INITIAL_FORMS': '0',
922+
'choices-MAX_NUM_FORMS': '4',
923+
'choices-0-choice': 'Zero',
924+
'choices-0-votes': '0',
925+
'choices-1-choice': 'One',
926+
'choices-1-votes': '1',
927+
'choices-2-choice': 'Two',
928+
'choices-2-votes': '2',
929+
'choices-3-choice': 'Three',
930+
'choices-3-votes': '3',
931+
},
932+
prefix='choices',
933+
)
934+
# This time four forms are instantiated
935+
self.assertEqual(len(formset.forms), 4)
936+
finally:
937+
formsets.DEFAULT_MAX_NUM = _old_DEFAULT_MAX_NUM
938+
881939

882940
data = {
883941
'choices-TOTAL_FORMS': '1', # the number of forms rendered

‎tests/regressiontests/generic_inline_admin/tests.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
from django.contrib.admin.sites import AdminSite
77
from django.contrib.contenttypes.generic import (
88
generic_inlineformset_factory, GenericTabularInline)
9+
from django.forms.formsets import DEFAULT_MAX_NUM
910
from django.forms.models import ModelForm
1011
from django.test import TestCase
1112
from django.test.utils import override_settings
@@ -244,7 +245,7 @@ def test_get_formset_kwargs(self):
244245

245246
# Create a formset with default arguments
246247
formset = media_inline.get_formset(request)
247-
self.assertEqual(formset.max_num, None)
248+
self.assertEqual(formset.max_num, DEFAULT_MAX_NUM)
248249
self.assertEqual(formset.can_order, False)
249250

250251
# Create a formset with custom keyword arguments

0 commit comments

Comments
 (0)