Skip to content

Commit

Permalink
[1.6.x] Fixed queries that may return unexpected results on MySQL due…
Browse files Browse the repository at this point in the history
… to typecasting.

This is a security fix. Disclosure will follow shortly.

Backport of 75c0d4e from master
  • Loading branch information
mxsasha authored and timgraham committed Apr 21, 2014
1 parent d63e209 commit 5f0829a
Show file tree
Hide file tree
Showing 6 changed files with 157 additions and 2 deletions.
16 changes: 15 additions & 1 deletion django/db/models/fields/__init__.py
Expand Up @@ -1013,6 +1013,12 @@ def __init__(self, verbose_name=None, name=None, path='', match=None,
kwargs['max_length'] = kwargs.get('max_length', 100)
Field.__init__(self, verbose_name, name, **kwargs)

def get_prep_value(self, value):
value = super(FilePathField, self).get_prep_value(value)
if value is None:
return None
return six.text_type(value)

def formfield(self, **kwargs):
defaults = {
'path': self.path,
Expand Down Expand Up @@ -1120,6 +1126,12 @@ def __init__(self, *args, **kwargs):
kwargs['max_length'] = 15
Field.__init__(self, *args, **kwargs)

def get_prep_value(self, value):
value = super(IPAddressField, self).get_prep_value(value)
if value is None:
return None
return six.text_type(value)

def get_internal_type(self):
return "IPAddressField"

Expand Down Expand Up @@ -1158,12 +1170,14 @@ def get_db_prep_value(self, value, connection, prepared=False):
return value or None

def get_prep_value(self, value):
if value is None:
return value
if value and ':' in value:
try:
return clean_ipv6_address(value, self.unpack_ipv4)
except exceptions.ValidationError:
pass
return value
return six.text_type(value)

def formfield(self, **kwargs):
defaults = {
Expand Down
10 changes: 10 additions & 0 deletions docs/howto/custom-model-fields.txt
Expand Up @@ -501,6 +501,16 @@ For example::
return ''.join([''.join(l) for l in (value.north,
value.east, value.south, value.west)])

.. warning::

If your custom field uses the ``CHAR``, ``VARCHAR`` or ``TEXT``
types for MySQL, you must make sure that :meth:`.get_prep_value`
always returns a string type. MySQL performs flexible and unexpected
matching when a query is performed on these types and the provided
value is an integer, which can cause queries to include unexpected
objects in their results. This problem cannot occur if you always
return a string type from :meth:`.get_prep_value`.

Converting query values to database values
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Expand Down
16 changes: 16 additions & 0 deletions docs/ref/databases.txt
Expand Up @@ -504,6 +504,22 @@ MySQL does not support the ``NOWAIT`` option to the ``SELECT ... FOR UPDATE``
statement. If ``select_for_update()`` is used with ``nowait=True`` then a
``DatabaseError`` will be raised.

Automatic typecasting can cause unexpected results
--------------------------------------------------

When performing a query on a string type, but with an integer value, MySQL will
coerce the types of all values in the table to an integer before performing the
comparison. If your table contains the values ``'abc'``, ``'def'`` and you
query for ``WHERE mycolumn=0``, both rows will match. Similarly, ``WHERE mycolumn=1``
will match the value ``'abc1'``. Therefore, string type fields included in Django
will always cast the value to a string before using it in a query.

If you implement custom model fields that inherit from :class:`~django.db.models.Field`
directly, are overriding :meth:`~django.db.models.Field.get_prep_value`, or use
:meth:`extra() <django.db.models.query.QuerySet.extra>` or
:meth:`raw() <django.db.models.Manager.raw>`, you should ensure that you
perform the appropriate typecasting.

.. _sqlite-notes:

SQLite notes
Expand Down
10 changes: 10 additions & 0 deletions docs/ref/models/querysets.txt
Expand Up @@ -1132,6 +1132,16 @@ of the arguments is required, but you should use at least one of them.

Entry.objects.extra(where=['headline=%s'], params=['Lennon'])

.. warning::

If you are performing queries on MySQL, note that MySQL's silent type coercion
may cause unexpected results when mixing types. If you query on a string
type column, but with an integer value, MySQL will coerce the types of all values
in the table to an integer before performing the comparison. For example, if your
table contains the values ``'abc'``, ``'def'`` and you query for ``WHERE mycolumn=0``,
both rows will match. To prevent this, perform the correct typecasting
before using the value in a query.

defer
~~~~~

Expand Down
10 changes: 10 additions & 0 deletions docs/topics/db/sql.txt
Expand Up @@ -66,6 +66,16 @@ options that make it very powerful.
database, but does nothing to enforce that. If the query does not
return rows, a (possibly cryptic) error will result.

.. warning::

If you are performing queries on MySQL, note that MySQL's silent type coercion
may cause unexpected results when mixing types. If you query on a string
type column, but with an integer value, MySQL will coerce the types of all values
in the table to an integer before performing the comparison. For example, if your
table contains the values ``'abc'``, ``'def'`` and you query for ``WHERE mycolumn=0``,
both rows will match. To prevent this, perform the correct typecasting
before using the value in a query.

Mapping query fields to model fields
------------------------------------

Expand Down
97 changes: 96 additions & 1 deletion tests/model_fields/tests.py
Expand Up @@ -7,7 +7,14 @@
from django import forms
from django.core.exceptions import ValidationError
from django.db import connection, models, IntegrityError
from django.db.models.fields.files import FieldFile
from django.db.models.fields import (
AutoField, BigIntegerField, BinaryField, BooleanField, CharField,
CommaSeparatedIntegerField, DateField, DateTimeField, DecimalField,
EmailField, FilePathField, FloatField, IntegerField, IPAddressField,
GenericIPAddressField, NullBooleanField, PositiveIntegerField,
PositiveSmallIntegerField, SlugField, SmallIntegerField, TextField,
TimeField, URLField)
from django.db.models.fields.files import FileField, ImageField
from django.utils import six
from django.utils import unittest

Expand Down Expand Up @@ -494,6 +501,94 @@ def test_genericipaddressfield_formfield_protocol(self):
self.assertRaises(ValidationError, form_field.clean, '127.0.0.1')


class PrepValueTest(test.TestCase):
def test_AutoField(self):
self.assertIsInstance(AutoField(primary_key=True).get_prep_value(1), int)

@unittest.skipIf(six.PY3, "Python 3 has no `long` type.")
def test_BigIntegerField(self):
self.assertIsInstance(BigIntegerField().get_prep_value(long(9999999999999999999)), long)

def test_BinaryField(self):
self.assertIsInstance(BinaryField().get_prep_value(b''), bytes)

def test_BooleanField(self):
self.assertIsInstance(BooleanField().get_prep_value(True), bool)

def test_CharField(self):
self.assertIsInstance(CharField().get_prep_value(''), six.text_type)
self.assertIsInstance(CharField().get_prep_value(0), six.text_type)

def test_CommaSeparatedIntegerField(self):
self.assertIsInstance(CommaSeparatedIntegerField().get_prep_value('1,2'), six.text_type)
self.assertIsInstance(CommaSeparatedIntegerField().get_prep_value(0), six.text_type)

def test_DateField(self):
self.assertIsInstance(DateField().get_prep_value(datetime.date.today()), datetime.date)

def test_DateTimeField(self):
self.assertIsInstance(DateTimeField().get_prep_value(datetime.datetime.now()), datetime.datetime)

def test_DecimalField(self):
self.assertIsInstance(DecimalField().get_prep_value(Decimal('1.2')), Decimal)

def test_EmailField(self):
self.assertIsInstance(EmailField().get_prep_value('mailbox@domain.com'), six.text_type)

def test_FileField(self):
self.assertIsInstance(FileField().get_prep_value('filename.ext'), six.text_type)
self.assertIsInstance(FileField().get_prep_value(0), six.text_type)

def test_FilePathField(self):
self.assertIsInstance(FilePathField().get_prep_value('tests.py'), six.text_type)
self.assertIsInstance(FilePathField().get_prep_value(0), six.text_type)

def test_FloatField(self):
self.assertIsInstance(FloatField().get_prep_value(1.2), float)

def test_ImageField(self):
self.assertIsInstance(ImageField().get_prep_value('filename.ext'), six.text_type)

def test_IntegerField(self):
self.assertIsInstance(IntegerField().get_prep_value(1), int)

def test_IPAddressField(self):
self.assertIsInstance(IPAddressField().get_prep_value('127.0.0.1'), six.text_type)
self.assertIsInstance(IPAddressField().get_prep_value(0), six.text_type)

def test_GenericIPAddressField(self):
self.assertIsInstance(GenericIPAddressField().get_prep_value('127.0.0.1'), six.text_type)
self.assertIsInstance(GenericIPAddressField().get_prep_value(0), six.text_type)

def test_NullBooleanField(self):
self.assertIsInstance(NullBooleanField().get_prep_value(True), bool)

def test_PositiveIntegerField(self):
self.assertIsInstance(PositiveIntegerField().get_prep_value(1), int)

def test_PositiveSmallIntegerField(self):
self.assertIsInstance(PositiveSmallIntegerField().get_prep_value(1), int)

def test_SlugField(self):
self.assertIsInstance(SlugField().get_prep_value('slug'), six.text_type)
self.assertIsInstance(SlugField().get_prep_value(0), six.text_type)

def test_SmallIntegerField(self):
self.assertIsInstance(SmallIntegerField().get_prep_value(1), int)

def test_TextField(self):
self.assertIsInstance(TextField().get_prep_value('Abc'), six.text_type)
self.assertIsInstance(TextField().get_prep_value(0), six.text_type)

def test_TimeField(self):
self.assertIsInstance(
TimeField().get_prep_value(datetime.datetime.now().time()),
datetime.time)

def test_URLField(self):
self.assertIsInstance(URLField().get_prep_value('http://domain.com'), six.text_type)


class CustomFieldTests(unittest.TestCase):

def test_14786(self):
Expand Down

0 comments on commit 5f0829a

Please sign in to comment.