Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

_md5 module crashes on large data #59093

Closed
pitrou opened this issue May 23, 2012 · 12 comments
Closed

_md5 module crashes on large data #59093

pitrou opened this issue May 23, 2012 · 12 comments
Labels
extension-modules C modules in the Modules dir type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@pitrou
Copy link
Member

pitrou commented May 23, 2012

BPO 14888
Nosy @gpshead, @jcea, @pitrou
Files
  • md5_huge.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2012-05-23.21:20:05.130>
    created_at = <Date 2012-05-23.13:20:13.646>
    labels = ['extension-modules', 'type-crash']
    title = '_md5 module crashes on large data'
    updated_at = <Date 2012-05-23.21:20:05.129>
    user = 'https://github.com/pitrou'

    bugs.python.org fields:

    activity = <Date 2012-05-23.21:20:05.129>
    actor = 'pitrou'
    assignee = 'none'
    closed = True
    closed_date = <Date 2012-05-23.21:20:05.130>
    closer = 'pitrou'
    components = ['Extension Modules']
    creation = <Date 2012-05-23.13:20:13.646>
    creator = 'pitrou'
    dependencies = []
    files = ['25679']
    hgrepos = []
    issue_num = 14888
    keywords = ['patch']
    message_count = 12.0
    messages = ['161406', '161407', '161411', '161412', '161426', '161427', '161438', '161439', '161440', '161441', '161456', '161457']
    nosy_count = 4.0
    nosy_names = ['gregory.p.smith', 'jcea', 'pitrou', 'python-dev']
    pr_nums = []
    priority = 'low'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'crash'
    url = 'https://bugs.python.org/issue14888'
    versions = ['Python 2.7']

    @pitrou
    Copy link
    Member Author

    pitrou commented May 23, 2012

    This appears to be 2.7-only:

    $ ./python -m test.regrtest -M5G -v test_hashlib
    == CPython 2.7.3+ (2.7:086afe7b61f5, May 23 2012, 15:15:34) [GCC 4.5.2]
    ==   Linux-2.6.38.8-desktop-10.mga-x86_64-with-mandrake-1-Official little-endian
    ==   /home/antoine/cpython/27/build/test_python_6042
    Testing with flags: sys.flags(debug=0, py3k_warning=0, division_warning=0, division_new=0, inspect=0, interactive=0, optimize=0, dont_write_bytecode=0, no_user_site=0, no_site=0, ignore_environment=0, tabcheck=0, verbose=0, unicode=0, bytes_warning=0, hash_randomization=0)
    test_hashlib
    test_algorithms_attribute (test.test_hashlib.HashLibTestCase) ... ok
    test_case_md5_0 (test.test_hashlib.HashLibTestCase) ... ok
    test_case_md5_1 (test.test_hashlib.HashLibTestCase) ... ok
    test_case_md5_2 (test.test_hashlib.HashLibTestCase) ... ok
    test_case_md5_huge (test.test_hashlib.HashLibTestCase) ... python: /home/antoine/cpython/27/Modules/md5module.c:276: MD5_new: Assertion `(Py_ssize_t)(unsigned int)(view.len) == (view.len)' failed.
    Abandon

    @pitrou pitrou added extension-modules C modules in the Modules dir type-crash A hard crash of the interpreter, possibly with a core dump labels May 23, 2012
    @pitrou
    Copy link
    Member Author

    pitrou commented May 23, 2012

    Here is a patch.

    @jcea
    Copy link
    Member

    jcea commented May 23, 2012

    Does this affect other hash modules?. Why is this not affecting python 3?

    Patch looks good.

    @pitrou
    Copy link
    Member Author

    pitrou commented May 23, 2012

    Does this affect other hash modules?

    I don't know, only md5 seems to have tests for large data.

    . Why is this not affecting python 3?

    The _md5 module was apparently rewritten in Python 3.

    @jcea
    Copy link
    Member

    jcea commented May 23, 2012

    I can't reproduce this issue in my 64 bit machines, neither in Solaris neither in Ubuntu. I guess the assertion can be fooled by compiler optimizacions.

    As a consequence, I can't check other hashes functions.

    Antoine, can you reproduce it doing "md5.new("A"*(2**32+5)).hexdigest()"?. That is what test HUGE does.

    @pitrou
    Copy link
    Member Author

    pitrou commented May 23, 2012

    I can't reproduce this issue in my 64 bit machines, neither in Solaris
    neither in Ubuntu. I guess the assertion can be fooled by compiler
    optimizacions.

    You should compile in debug mode.

    @jcea
    Copy link
    Member

    jcea commented May 23, 2012

    sha1 fails the same way. Same error. Just clone the test to show it.

    Please, correct sha1 too and add a test for it :).

    sha224, sha256, sha384 and sha512 seems OK.

    @jcea
    Copy link
    Member

    jcea commented May 23, 2012

    sha224, sha256, sha384 and sha512 are not failing because they are missing the "Py_SAFE_DOWNCAST" safety net completely.

    So I would tell that we have an issue here :).

    @jcea
    Copy link
    Member

    jcea commented May 23, 2012

    Same can be said about Python 3 hash modules: they are not using the sanity check, so they work.

    Maybe the real question should be if the sanity check really makes sense at all. If not, remove everywhere (the calculated md5 with no checks looks correct, after all). If it makes sense, mark this as "python 3.2 and 3.3" and patch everywhere :).

    Do you agree, Antoine?

    @pitrou
    Copy link
    Member Author

    pitrou commented May 23, 2012

    sha1 fails the same way. Same error. Just clone the test to show it.

    Please, correct sha1 too and add a test for it :).

    Well, do you want to provide an updated patch?

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented May 23, 2012

    New changeset 290d970c011d by Antoine Pitrou in branch '2.7':
    Issue bpo-14888: Fix misbehaviour of the _md5 module when called on data larger than 2**32 bytes.
    http://hg.python.org/cpython/rev/290d970c011d

    @pitrou
    Copy link
    Member Author

    pitrou commented May 23, 2012

    I've now pushed the fix. Jesus, if you want to propose a test and patch for the _sha1 issue, please open a separate issue. Thanks!

    @pitrou pitrou closed this as completed May 23, 2012
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    extension-modules C modules in the Modules dir type-crash A hard crash of the interpreter, possibly with a core dump
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants