Skip to content

Commit

Permalink
Prevented newlines from being accepted in some validators.
Browse files Browse the repository at this point in the history
This is a security fix; disclosure to follow shortly.

Thanks to Sjoerd Job Postmus for the report and draft patch.
  • Loading branch information
timgraham committed Jul 8, 2015
1 parent df049ed commit 014247a
Show file tree
Hide file tree
Showing 5 changed files with 111 additions and 13 deletions.
27 changes: 15 additions & 12 deletions django/core/validators.py
Expand Up @@ -83,7 +83,7 @@ class URLValidator(RegexValidator):
r'(?:' + ipv4_re + '|' + ipv6_re + '|' + host_re + ')'
r'(?::\d{2,5})?' # port
r'(?:[/?#][^\s]*)?' # resource path
r'$', re.IGNORECASE)
r'\Z', re.IGNORECASE)
message = _('Enter a valid URL.')
schemes = ['http', 'https', 'ftp', 'ftps']

Expand Down Expand Up @@ -125,29 +125,32 @@ def __call__(self, value):
raise ValidationError(self.message, code=self.code)
url = value

integer_validator = RegexValidator(
re.compile('^-?\d+\Z'),
message=_('Enter a valid integer.'),
code='invalid',
)


def validate_integer(value):
try:
int(value)
except (ValueError, TypeError):
raise ValidationError(_('Enter a valid integer.'), code='invalid')
return integer_validator(value)


@deconstructible
class EmailValidator(object):
message = _('Enter a valid email address.')
code = 'invalid'
user_regex = re.compile(
r"(^[-!#$%&'*+/=?^_`{}|~0-9A-Z]+(\.[-!#$%&'*+/=?^_`{}|~0-9A-Z]+)*$" # dot-atom
r'|^"([\001-\010\013\014\016-\037!#-\[\]-\177]|\\[\001-\011\013\014\016-\177])*"$)', # quoted-string
r"(^[-!#$%&'*+/=?^_`{}|~0-9A-Z]+(\.[-!#$%&'*+/=?^_`{}|~0-9A-Z]+)*\Z" # dot-atom
r'|^"([\001-\010\013\014\016-\037!#-\[\]-\177]|\\[\001-\011\013\014\016-\177])*"\Z)', # quoted-string
re.IGNORECASE)
domain_regex = re.compile(
# max length for domain name labels is 63 characters per RFC 1034
r'((?:[A-Z0-9](?:[A-Z0-9-]{0,61}[A-Z0-9])?\.)+)(?:[A-Z0-9-]{2,63}(?<!-))$',
r'((?:[A-Z0-9](?:[A-Z0-9-]{0,61}[A-Z0-9])?\.)+)(?:[A-Z0-9-]{2,63}(?<!-))\Z',
re.IGNORECASE)
literal_regex = re.compile(
# literal form, ipv4 or ipv6 address (SMTP 4.1.3)
r'\[([A-f0-9:\.]+)\]$',
r'\[([A-f0-9:\.]+)\]\Z',
re.IGNORECASE)
domain_whitelist = ['localhost']

Expand Down Expand Up @@ -205,14 +208,14 @@ def __eq__(self, other):

validate_email = EmailValidator()

slug_re = re.compile(r'^[-a-zA-Z0-9_]+$')
slug_re = re.compile(r'^[-a-zA-Z0-9_]+\Z')
validate_slug = RegexValidator(
slug_re,
_("Enter a valid 'slug' consisting of letters, numbers, underscores or hyphens."),
'invalid'
)

ipv4_re = re.compile(r'^(25[0-5]|2[0-4]\d|[0-1]?\d?\d)(\.(25[0-5]|2[0-4]\d|[0-1]?\d?\d)){3}$')
ipv4_re = re.compile(r'^(25[0-5]|2[0-4]\d|[0-1]?\d?\d)(\.(25[0-5]|2[0-4]\d|[0-1]?\d?\d)){3}\Z')
validate_ipv4_address = RegexValidator(ipv4_re, _('Enter a valid IPv4 address.'), 'invalid')


Expand Down Expand Up @@ -255,7 +258,7 @@ def ip_address_validators(protocol, unpack_ipv4):


def int_list_validator(sep=',', message=None, code='invalid'):
regexp = re.compile('^\d+(?:%s\d+)*$' % re.escape(sep))
regexp = re.compile('^\d+(?:%s\d+)*\Z' % re.escape(sep))
return RegexValidator(regexp, message=message, code=code)


Expand Down
26 changes: 26 additions & 0 deletions docs/releases/1.4.21.txt
Expand Up @@ -26,3 +26,29 @@ As each built-in session backend was fixed separately (rather than a fix in the
core sessions framework), maintainers of third-party session backends should
check whether the same vulnerability is present in their backend and correct
it if so.

Header injection possibility since validators accept newlines in input
======================================================================

Some of Django's built-in validators
(:class:`~django.core.validators.EmailValidator`, most seriously) didn't
prohibit newline characters (due to the usage of ``$`` instead of ``\Z`` in the
regular expressions). If you use values with newlines in HTTP response or email
headers, you can suffer from header injection attacks. Django itself isn't
vulnerable because :class:`~django.http.HttpResponse` and the mail sending
utilities in :mod:`django.core.mail` prohibit newlines in HTTP and SMTP
headers, respectively. While the validators have been fixed in Django, if
you're creating HTTP responses or email messages in other ways, it's a good
idea to ensure that those methods prohibit newlines as well. You might also
want to validate that any existing data in your application doesn't contain
unexpected newlines.

:func:`~django.core.validators.validate_ipv4_address`,
:func:`~django.core.validators.validate_slug`, and
:class:`~django.core.validators.URLValidator` and their usage in the
corresponding form fields ``GenericIPAddresseField``, ``IPAddressField``,
``SlugField``, and ``URLField`` are also affected.

The undocumented, internally unused ``validate_integer()`` function is now
stricter as it validates using a regular expression instead of simply casting
the value using ``int()`` and checking if an exception was raised.
28 changes: 28 additions & 0 deletions docs/releases/1.7.9.txt
Expand Up @@ -27,6 +27,34 @@ core sessions framework), maintainers of third-party session backends should
check whether the same vulnerability is present in their backend and correct
it if so.

Header injection possibility since validators accept newlines in input
======================================================================

Some of Django's built-in validators
(:class:`~django.core.validators.EmailValidator`, most seriously) didn't
prohibit newline characters (due to the usage of ``$`` instead of ``\Z`` in the
regular expressions). If you use values with newlines in HTTP response or email
headers, you can suffer from header injection attacks. Django itself isn't
vulnerable because :class:`~django.http.HttpResponse` and the mail sending
utilities in :mod:`django.core.mail` prohibit newlines in HTTP and SMTP
headers, respectively. While the validators have been fixed in Django, if
you're creating HTTP responses or email messages in other ways, it's a good
idea to ensure that those methods prohibit newlines as well. You might also
want to validate that any existing data in your application doesn't contain
unexpected newlines.

:func:`~django.core.validators.validate_ipv4_address`,
:func:`~django.core.validators.validate_slug`, and
:class:`~django.core.validators.URLValidator` are also affected, however, as
of Django 1.6 the ``GenericIPAddresseField``, ``IPAddressField``, ``SlugField``,
and ``URLField`` form fields which use these validators all strip the input, so
the possibility of newlines entering your data only exists if you are using
these validators outside of the form fields.

The undocumented, internally unused ``validate_integer()`` function is now
stricter as it validates using a regular expression instead of simply casting
the value using ``int()`` and checking if an exception was raised.

Bugfixes
========

Expand Down
28 changes: 28 additions & 0 deletions docs/releases/1.8.3.txt
Expand Up @@ -32,6 +32,34 @@ core sessions framework), maintainers of third-party session backends should
check whether the same vulnerability is present in their backend and correct
it if so.

Header injection possibility since validators accept newlines in input
======================================================================

Some of Django's built-in validators
(:class:`~django.core.validators.EmailValidator`, most seriously) didn't
prohibit newline characters (due to the usage of ``$`` instead of ``\Z`` in the
regular expressions). If you use values with newlines in HTTP response or email
headers, you can suffer from header injection attacks. Django itself isn't
vulnerable because :class:`~django.http.HttpResponse` and the mail sending
utilities in :mod:`django.core.mail` prohibit newlines in HTTP and SMTP
headers, respectively. While the validators have been fixed in Django, if
you're creating HTTP responses or email messages in other ways, it's a good
idea to ensure that those methods prohibit newlines as well. You might also
want to validate that any existing data in your application doesn't contain
unexpected newlines.

:func:`~django.core.validators.validate_ipv4_address`,
:func:`~django.core.validators.validate_slug`, and
:class:`~django.core.validators.URLValidator` are also affected, however, as
of Django 1.6 the ``GenericIPAddresseField``, ``IPAddressField``, ``SlugField``,
and ``URLField`` form fields which use these validators all strip the input, so
the possibility of newlines entering your data only exists if you are using
these validators outside of the form fields.

The undocumented, internally unused ``validate_integer()`` function is now
stricter as it validates using a regular expression instead of simply casting
the value using ``int()`` and checking if an exception was raised.

Bugfixes
========

Expand Down
15 changes: 14 additions & 1 deletion tests/validators/tests.py
Expand Up @@ -28,10 +28,12 @@
(validate_integer, '42', None),
(validate_integer, '-42', None),
(validate_integer, -42, None),
(validate_integer, -42.5, None),

(validate_integer, -42.5, ValidationError),
(validate_integer, None, ValidationError),
(validate_integer, 'a', ValidationError),
(validate_integer, '\n42', ValidationError),
(validate_integer, '42\n', ValidationError),

(validate_email, 'email@here.com', None),
(validate_email, 'weirder-email@here.and.there.com', None),
Expand Down Expand Up @@ -77,6 +79,11 @@
# Max length of domain name labels is 63 characters per RFC 1034.
(validate_email, 'a@%s.us' % ('a' * 63), None),
(validate_email, 'a@%s.us' % ('a' * 64), ValidationError),
# Trailing newlines in username or domain not allowed
(validate_email, 'a@b.com\n', ValidationError),
(validate_email, 'a\n@b.com', ValidationError),
(validate_email, '"test@test"\n@example.com', ValidationError),
(validate_email, 'a@[127.0.0.1]\n', ValidationError),

(validate_slug, 'slug-ok', None),
(validate_slug, 'longer-slug-still-ok', None),
Expand All @@ -89,6 +96,7 @@
(validate_slug, 'some@mail.com', ValidationError),
(validate_slug, '你好', ValidationError),
(validate_slug, '\n', ValidationError),
(validate_slug, 'trailing-newline\n', ValidationError),

(validate_ipv4_address, '1.1.1.1', None),
(validate_ipv4_address, '255.0.0.0', None),
Expand All @@ -98,6 +106,7 @@
(validate_ipv4_address, '25.1.1.', ValidationError),
(validate_ipv4_address, '25,1,1,1', ValidationError),
(validate_ipv4_address, '25.1 .1.1', ValidationError),
(validate_ipv4_address, '1.1.1.1\n', ValidationError),

# validate_ipv6_address uses django.utils.ipv6, which
# is tested in much greater detail in its own testcase
Expand Down Expand Up @@ -142,6 +151,7 @@

(int_list_validator(sep='.'), '1.2.3', None),
(int_list_validator(sep='.'), '1,2,3', ValidationError),
(int_list_validator(sep='.'), '1.2.3\n', ValidationError),

(MaxValueValidator(10), 10, None),
(MaxValueValidator(10), -10, None),
Expand Down Expand Up @@ -175,6 +185,9 @@
(URLValidator(EXTENDED_SCHEMES), 'git://example.com/', None),

(URLValidator(EXTENDED_SCHEMES), 'git://-invalid.com', ValidationError),
# Trailing newlines not accepted
(URLValidator(), 'http://www.djangoproject.com/\n', ValidationError),
(URLValidator(), 'http://[::ffff:192.9.5.5]\n', ValidationError),

(BaseValidator(True), True, None),
(BaseValidator(True), False, ValidationError),
Expand Down

3 comments on commit 014247a

@marfire
Copy link
Contributor

@marfire marfire commented on 014247a Jul 8, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is surely a good change, but is it really true that there was a "header injection possibility"? In the absence of the re.MULTILINE flag, the only newline that could have passed through would have been one at the last position in the string, so I don't see how that could be used to construct a malicious header.

Or am I missing something?

@aaugustin
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have a closer look at the email examples.

@marfire
Copy link
Contributor

@marfire marfire commented on 014247a Jul 9, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aaugustin: Thanks, I missed the fact that the validator was splitting the email address and applying separate regexes. Makes sense now...

Please sign in to comment.