[issue13703] Hash collision security issue

Dave Malcolm report at bugs.python.org
Mon Jan 23 22:32:00 CET 2012


Dave Malcolm <dmalcolm at redhat.com> added the comment:

I'm attaching an attempt at backporting haypo's random-8.patch to 2.7

Changes relative to random-8.patch:

   * The randomization is off by default, and must be enabled by setting
     a new environment variable PYTHONHASHRANDOMIZATION to a non-empty string.
     (if so then, PYTHONHASHSEED also still works, if provided, in the same
     way as in haypo's patch)

   * All of the various "Py_hash_t" become "long" again (Py_hash_t was
     added in 3.2: issue9778)

   * I expanded the randomization from just PyUnicodeObject to also cover
     these types:

     * PyStringObject

     * PyBufferObject

     The randomization does not cover numeric types: if we change the hash of
     int so that hash(i) no longer equals i, we also have to change it
     consistently for long, float, complex, decimal.Decimal and
     fractions.Fraction; however, there are 3rd-party numeric types that
     have their own __hash__ implementation that mimics int.__hash__ (see
     e.g. numpy)

     As noted in http://bugs.python.org/issue13703#msg151063 and
     http://bugs.python.org/issue13703#msg151064, it's not possible
     to directly create a dict with integer keys via JSON or XML-RPC.

     This seems like a tradeoff between the risk of attack via other means
     vs breakage induced by not having hash() == hash() for the various
     equivalent numerical representations in pre-existing code.

   * To support my expanded usage of the random secret, I moved:
       
       PyAPI_DATA(_Py_unicode_hash_secret_t) _Py_unicode_hash_secret

     from unicodeobject.h to object.h and renamed it to:

       PyAPI_DATA(_Py_HashSecret_t) _Py_HashSecret;

     This also exposes it for usage by C extension modules, just in case
     they need it (Murphy's Law suggests we will need if we don't expose
     it).   This is an extension of the API, but warranted, I feel.  My
     plan for downstream RHEL is to add this explicitly to the RPM metadata
     as a "Provides" of the RPM providing libpython.so so that if something
     needs to use it, it can express a "Requires" on it; I assume that
     something similar is possible with .deb)

   * generalized test_unicode.HashTest to support the new env var and the
     additional types.  In my version, get_hash takes a _repr string rather
     than an object, so that I can test it with a buffer().  Arguably the
     tests should thus be moved from test_unicode to somewhere else, but this
     location keeps things consistent with haypo's patch.

     haypo: in random-8.patch, within test_unicode.HashTest.test_null_hash,
     "hash_empty" seems to be misnamed

   * dropped various selftest fixes where the corresponding selftests don't
     exist in 2.7

   * adds a description of the new environment variables to the manpage;
     arguably this should be done for the patch for the default branch also

Caveats:

   * only tested on Linux (Fedora 15 x86_64); not tested on Windows.  Tested
     via "make test" both with and without PYTHONHASHRANDOMIZATION=1

   * not yet benchmarked

 Doc/using/cmdline.rst                      |   28 ++
 Include/object.h                           |    7 
 Include/pythonrun.h                        |    2 
 Lib/lib-tk/test/test_ttk/test_functions.py |    2 
 Lib/os.py                                  |   19 -
 Lib/test/mapping_tests.py                  |    2 
 Lib/test/regrtest.py                       |    5 
 Lib/test/test_gdb.py                       |   15 +
 Lib/test/test_inspect.py                   |    1 
 Lib/test/test_os.py                        |   47 +++-
 Lib/test/test_unicode.py                   |   55 +++++
 Makefile.pre.in                            |    1 
 Misc/python.man                            |   22 ++
 Modules/posixmodule.c                      |  126 ++----------
 Objects/bufferobject.c                     |    8 
 Objects/object.c                           |    2 
 Objects/stringobject.c                     |    8 
 Objects/unicodeobject.c                    |   17 +
 PCbuild/pythoncore.vcproj                  |    4 
 Python/pythonrun.c                         |    2 
 b/Python/random.c                          |  284 +++++++++++++++++++++++++++++
 21 files changed, 510 insertions(+), 147 deletions(-)

----------
Added file: http://bugs.python.org/file24304/backport-of-hash-randomization-to-2.7-dmalcolm-2012-01-23-001.patch

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue13703>
_______________________________________


More information about the Python-bugs-list mailing list