Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(2726)

#13703: Hash collision security issue

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 9 months ago by barry
Modified:
5 years, 8 months ago
Reviewers:
lists, zbyszek, greg, martin, benjamin, pitrou
CC:
lemburg, gvanrossum, tim.peters, loewis, barry, Georg, terry.reedy, gregory.p.smith, jcea, mark.dickinson, AntoinePitrou, haypo, christian.heimes, Benjamin Peterson, serwy, eric.araujo, Graham.Dumpleton_gmail.com, Arfrever, v+python, alex, cvrebert, Z. Jędrzejewski-Szmek, skrah, dmalcolm, gzlist_googlemail.com, Charles-François Natali, pavel.labushev_runbox.no, Mark.Shannon, devnull_psf.upfronthosting.co.za, eric.snow, kofreestyler_gmail.com, sidhpurwala.huzaifa_gmail.com, Jim.J.Jewett, paul_mcmillan.ws, python_sievertsen.de, skorgu_gmail.com, jsvaughan_gmail.com, devnull_psf.upfronthosting.co.za, paul_mcmillan.ws, Mark.Shannon, haypo
Visibility:
Public.

Patch Set 1 #

Patch Set 2 #

Total comments: 1

Patch Set 3 #

Patch Set 4 #

Patch Set 5 #

Patch Set 6 #

Total comments: 5

Patch Set 7 #

Patch Set 8 #

Patch Set 9 #

Total comments: 1

Patch Set 10 #

Patch Set 11 #

Patch Set 12 #

Patch Set 13 #

Patch Set 14 #

Patch Set 15 #

Patch Set 16 #

Total comments: 2

Patch Set 17 #

Patch Set 18 #

Patch Set 19 #

Patch Set 20 #

Patch Set 21 #

Patch Set 22 #

Patch Set 23 #

Patch Set 24 #

Patch Set 25 #

Patch Set 26 #

Patch Set 27 #

Patch Set 28 #

Patch Set 29 #

Total comments: 7

Patch Set 30 #

Patch Set 31 #

Patch Set 32 #

Total comments: 10

Patch Set 33 #

Total comments: 1

Patch Set 34 #

Total comments: 6

Patch Set 35 #

Patch Set 36 #

Patch Set 37 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Doc/library/sys.rst View 1 chunk +4 lines, -0 lines 0 comments Download
Doc/reference/datamodel.rst View 17 18 20 21 22 23 25 27 29 31 32 33 34 35 36 1 chunk +2 lines, -0 lines 0 comments Download
Doc/using/cmdline.rst View 3 4 5 6 7 9 13 14 15 16 17 18 20 21 22 23 25 27 29 31 32 33 34 35 36 5 chunks +47 lines, -1 line 0 comments Download
Include/object.h View 13 14 15 16 17 18 20 21 22 23 25 27 29 31 32 33 34 35 36 1 chunk +6 lines, -0 lines 0 comments Download
Include/pydebug.h View 17 18 20 21 22 23 25 27 29 31 32 33 34 35 36 1 chunk +1 line, -0 lines 0 comments Download
Include/pythonrun.h View 1 2 3 4 5 6 7 9 13 14 15 16 17 18 20 21 22 23 25 27 29 31 32 33 34 35 36 1 chunk +2 lines, -0 lines 0 comments Download
Lib/os.py View 1 2 3 4 5 6 7 9 13 14 15 16 17 18 20 21 22 23 25 27 29 31 32 33 34 35 36 1 chunk +0 lines, -17 lines 0 comments Download
Lib/test/regrtest.py View 6 7 9 13 14 15 16 17 18 20 21 25 29 32 34 35 36 1 chunk +5 lines, -0 lines 0 comments Download
Lib/test/script_helper.py View 2 chunks +5 lines, -2 lines 0 comments Download
Lib/test/test_cmd_line.py View 17 18 20 21 22 23 25 27 29 31 32 33 34 35 36 2 chunks +16 lines, -1 line 0 comments Download
Lib/test/test_hash.py View 14 15 16 17 18 20 21 22 23 25 27 29 31 32 33 34 35 36 2 chunks +90 lines, -2 lines 0 comments Download
Lib/test/test_os.py View 3 4 5 6 8 9 13 14 15 16 17 18 20 21 22 23 25 27 29 31 32 33 34 35 36 2 chunks +28 lines, -8 lines 0 comments Download
Lib/test/test_sys.py View 18 20 21 23 25 27 29 31 32 33 34 35 36 2 chunks +5 lines, -3 lines 0 comments Download
Makefile.pre.in View 1 2 3 4 5 6 7 9 13 14 15 16 17 18 20 21 22 23 25 27 29 31 32 33 34 35 36 1 chunk +1 line, -0 lines 0 comments Download
Misc/NEWS View 17 18 20 21 22 23 25 27 29 31 32 33 34 35 36 1 chunk +5 lines, -0 lines 0 comments Download
Misc/python.man View 13 16 17 18 20 21 22 23 25 27 29 31 32 33 34 35 36 3 chunks +29 lines, -0 lines 0 comments Download
Modules/datetimemodule.c View 16 17 18 20 21 25 29 32 34 35 36 1 chunk +3 lines, -1 line 0 comments Download
Modules/main.c View 17 18 20 21 22 23 25 27 29 31 32 33 34 35 36 5 chunks +15 lines, -1 line 0 comments Download
Modules/posixmodule.c View 1 2 3 4 5 6 7 9 13 14 15 16 17 18 20 21 22 23 25 27 29 31 32 33 34 35 36 4 chunks +23 lines, -110 lines 0 comments Download
Objects/bytesobject.c View 14 15 16 17 18 20 21 25 29 32 34 35 36 1 chunk +11 lines, -1 line 0 comments Download
Objects/object.c View 13 14 15 16 17 18 20 21 22 23 25 27 29 31 32 33 34 35 36 1 chunk +2 lines, -0 lines 0 comments Download
Objects/unicodeobject.c View 1 2 3 4 5 6 7 9 13 14 15 16 17 18 20 21 22 23 25 27 29 31 32 33 34 35 36 1 chunk +11 lines, -1 line 0 comments Download
PCbuild/pythoncore.vcproj View 6 7 9 13 14 15 16 17 18 20 21 22 23 25 27 29 31 32 33 34 35 36 1 chunk +4 lines, -0 lines 0 comments Download
Python/pythonrun.c View 1 2 3 4 5 6 7 9 13 14 15 16 17 18 20 21 22 23 25 27 29 31 32 33 34 35 36 3 chunks +8 lines, -0 lines 0 comments Download
Python/random.c View 1 2 4 5 6 7 9 13 14 15 16 17 18 20 21 22 23 25 27 29 31 32 33 34 35 36 1 chunk +302 lines, -0 lines 0 comments Download
Python/sysmodule.c View 10 14 15 18 20 21 22 23 25 27 29 31 32 33 34 35 36 2 chunks +2 lines, -0 lines 0 comments Download

Messages

Total messages: 11
christian.heimes
http://bugs.python.org/review/13703/diff/3932/12992 File Python/random.c (right): http://bugs.python.org/review/13703/diff/3932/12992#newcode47 Python/random.c:47: return length; length isn't defined, how about "return size;"?
5 years, 9 months ago #1
Z. Jędrzejewski-Szmek
Some small comments. http://bugs.python.org/review/13703/diff/3988/13266 File Include/pythonrun.h (right): http://bugs.python.org/review/13703/diff/3988/13266#newcode250 Include/pythonrun.h:250: PyAPI_FUNC(int PyOS_URandom) (void *buffer, Py_ssize_t size); ...
5 years, 9 months ago #2
gregory.p.smith
I think what you've done in issue 13704 is better than this change.
5 years, 9 months ago #3
gregory.p.smith
I still like what you've done in http://bugs.python.org/review/13704/show better than what you have done in ...
5 years, 9 months ago #4
loewis
http://bugs.python.org/review/13703/diff/4069/13656 File Makefile.pre.in (right): http://bugs.python.org/review/13703/diff/4069/13656#newcode715 Makefile.pre.in:715: test: all platform There should be a test target ...
5 years, 8 months ago #5
Benjamin Peterson
All doc problems I think. http://bugs.python.org/review/13703/diff/4129/14145 File Doc/using/cmdline.rst (right): http://bugs.python.org/review/13703/diff/4129/14145#newcode220 Doc/using/cmdline.rst:220: Turn on "hash randomization, ...
5 years, 8 months ago #6
gregory.p.smith
http://bugs.python.org/review/13703/diff/4172/14373 File Doc/library/sys.rst (right): http://bugs.python.org/review/13703/diff/4172/14373#newcode223 Doc/library/sys.rst:223: :const:`hash_randomization` :option:`-R` mention the 3.1.X version this was added ...
5 years, 8 months ago #7
gregory.p.smith
http://bugs.python.org/review/13703/diff/4172/14396 File Python/random.c (right): http://bugs.python.org/review/13703/diff/4172/14396#newcode277 Python/random.c:277: "in range [0; 4294967295]"); nit picking here, i think ...
5 years, 8 months ago #8
gregory.p.smith
looking good. I do agree with Marc-Andre Lemburg's comment in the bug based on previous ...
5 years, 8 months ago #9
gregory.p.smith
http://bugs.python.org/review/13703/diff/4191/14547 File Lib/test/test_cmd_line.py (right): http://bugs.python.org/review/13703/diff/4191/14547#newcode107 Lib/test/test_cmd_line.py:107: # Verify that -R enables hash randomization: To be ...
5 years, 8 months ago #10
AntoinePitrou
5 years, 8 months ago #11
http://bugs.python.org/review/13703/diff/4192/14568
File Doc/using/cmdline.rst (right):

http://bugs.python.org/review/13703/diff/4192/14568#newcode227
Doc/using/cmdline.rst:227: of a dict lookup, O(n^2) complexity.  See:
The worst case performance of a dict lookup is O(n). O(n^2) is the worst case
performance of dict construction.

http://bugs.python.org/review/13703/diff/4192/14575
File Lib/test/test_hash.py (right):

http://bugs.python.org/review/13703/diff/4192/14575#newcode14
Lib/test/test_hash.py:14: IS_64BIT = (struct.calcsize('l') == 8)
That's a bit misleading. This will get you the size of a C long, not whether the
machine is 64-bit. Under Windows for Python 3.1 it still does the right thing
(because hash() returns a C long). However, starting from 3.2, hash() returns a
Py_hash_t, which is 64-bit under 64-bit Windows: you'll have to use sys.maxsize.

http://bugs.python.org/review/13703/diff/4192/14575#newcode138
Lib/test/test_hash.py:138: env['PYTHONHASHRANDOMIZATION'] = str(randomization)
If it's None, you should remove it from the environment, no?

http://bugs.python.org/review/13703/diff/4192/14589
File Python/random.c (right):

http://bugs.python.org/review/13703/diff/4192/14589#newcode236
Python/random.c:236: return dev_urandom_python((char*)buffer, size);
No VMS here?

http://bugs.python.org/review/13703/diff/4192/14590
File Python/sysmodule.c (right):

http://bugs.python.org/review/13703/diff/4192/14590#newcode1138
Python/sysmodule.c:1138: 13
If you change this value, it threatens to break compatibility (think someone
using tuple-unpacking on sys.flags). I think we should only change it in 3.3.
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7