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

[CVE-2007-4965] Integer overflow in imageop module #45520

Closed
donmez mannequin opened this issue Sep 19, 2007 · 28 comments
Closed

[CVE-2007-4965] Integer overflow in imageop module #45520

donmez mannequin opened this issue Sep 19, 2007 · 28 comments
Labels
extension-modules C modules in the Modules dir release-blocker type-security A security issue

Comments

@donmez
Copy link
Mannequin

donmez mannequin commented Sep 19, 2007

BPO 1179
Nosy @gvanrossum, @warsaw, @matejcik, @benjaminp
Files
  • poc.py
  • python-2.5.CVE-2007-4965-int-overflow.patch
  • python-2.5.CVE-2007-4965-int-overflow.patch
  • python-2.5.CVE-2007-4965-int-overflow.patch
  • python-2.5-int-overflow-2.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 2008-08-19.21:02:22.605>
    created_at = <Date 2007-09-19.01:02:34.329>
    labels = ['type-security', 'extension-modules', 'release-blocker']
    title = '[CVE-2007-4965] Integer overflow in imageop module'
    updated_at = <Date 2008-08-19.21:02:22.604>
    user = 'https://bugs.python.org/donmez'

    bugs.python.org fields:

    activity = <Date 2008-08-19.21:02:22.604>
    actor = 'gvanrossum'
    assignee = 'none'
    closed = True
    closed_date = <Date 2008-08-19.21:02:22.605>
    closer = 'gvanrossum'
    components = ['Extension Modules']
    creation = <Date 2007-09-19.01:02:34.329>
    creator = 'donmez'
    dependencies = []
    files = ['8448', '8450', '8452', '8592', '9975']
    hgrepos = []
    issue_num = 1179
    keywords = ['patch']
    message_count = 28.0
    messages = ['56020', '56022', '56042', '56045', '56047', '56049', '56050', '56051', '56052', '56053', '56596', '56659', '58789', '58820', '58828', '58829', '63888', '64682', '64955', '65130', '66394', '66405', '66407', '66408', '70476', '70744', '71477', '71483']
    nosy_count = 11.0
    nosy_names = ['gvanrossum', 'barry', 'nnorwitz', 'anthonybaxter', 'jafo', 'chmod007', 'donmez', 'matejcik', 'nevyn', 'jhpanetta', 'benjamin.peterson']
    pr_nums = []
    priority = 'release blocker'
    resolution = 'accepted'
    stage = None
    status = 'closed'
    superseder = None
    type = 'security'
    url = 'https://bugs.python.org/issue1179'
    versions = ['Python 2.6', 'Python 2.5']

    @donmez
    Copy link
    Mannequin Author

    donmez mannequin commented Sep 19, 2007

    As reported at
    http://lists.grok.org.uk/pipermail/full-disclosure/2007-September/065826.html
    . There is an integer overflow in imageop module which results in an
    interpreter crash. Original proof of concept code is attached.

    @donmez donmez mannequin added stdlib Python modules in the Lib dir type-security A security issue labels Sep 19, 2007
    @jafo
    Copy link
    Mannequin

    jafo mannequin commented Sep 19, 2007

    It's unclear if this only causes a crash or if it can inject data.
    Referenced mailing list post points out where one error is.

    @gvanrossum
    Copy link
    Member

    Cartman, please refrain from using vulgarities in your sample code. It's
    hard to take a bug report seriously with such variable names.

    @jafo
    Copy link
    Mannequin

    jafo mannequin commented Sep 19, 2007

    Guido: That code came from the full-disclosure list posting, I think
    cartman was just passing it on.

    @nevyn
    Copy link
    Mannequin

    nevyn mannequin commented Sep 19, 2007

    So I think this is all the places integer overflow checking is needed
    in imageop.c and rbgimgmodule.c.
    There might be checks here which can't be exploited anyway, and I
    haven't checked any other files yet.

    Feel free to comment.

    Ps. This is against the 2.5 in Fedora-7, but it should apply to upstream.

    @donmez
    Copy link
    Mannequin Author

    donmez mannequin commented Sep 19, 2007

    Guido,

    The poc is taken as is, sorry.

    @nevyn
    Copy link
    Mannequin

    nevyn mannequin commented Sep 19, 2007

    And now the obvious typo fix, *sigh*.

    @donmez
    Copy link
    Mannequin Author

    donmez mannequin commented Sep 19, 2007

    nevyn: Your patch cleanly applies to python 2.4.4 and fixes the
    interpreter crash with poc.py

    Thanks.

    @gvanrossum
    Copy link
    Member

    Hm. First of all, it seems the imageop module has completely missed the
    Py_ssize_t changes.

    Second, I don't think that "if ( x != len / y )" is a valid replacement
    for "if ( x*y != len )" -- consider x==5, y==2, len==11.

    @nevyn
    Copy link
    Mannequin

    nevyn mannequin commented Sep 20, 2007

    Guido: It's true that that len can be slightly bigger than x*y, the big
    thing is that it can't be smaller so we can malloc(len) and use upto x*y
    (which was my main focus).
    I first looked at any of this code today, but I didn't see any reason
    that having len be slightly larger would be a problem ... and in pretty
    much all cases it'll be len == x*y.

    However we could have both cases covered by doing:

    if ( (len != x*y) || (x != (len / y)) )

    ...but esp. at that point it seems like we'd want some interface so that
    we could just do something like:

    if ( check_mutliplies2(len, x, y) )

    @gvanrossum
    Copy link
    Member

    Neal, didn't you say you had a fix for this?

    @nevyn
    Copy link
    Mannequin

    nevyn mannequin commented Oct 22, 2007

    Not sure who Neal is, and this probably isn't a final upstream fix ...
    but it's what I've applied to Fedora's python. It's basically the same
    patch as before, but it keeps the original * tests instead of just
    replacing them with / tests. So given:

    if x * y != len

    ...the first patch did:

    if len / x != y

    ...and this patch does:

    if x * y != len ||
    len / x != y

    @jhpanetta
    Copy link
    Mannequin

    jhpanetta mannequin commented Dec 19, 2007

    Is this final yet? Our system security group is a little paranoid about
    buffer overflows of any sort and are starting to make noises. I can
    confirm that the Oct 20 patch applies against Python 2.5.1 on RHEL4, and
    that the string length error is generated when running poc.py.

    @gvanrossum
    Copy link
    Member

    Sigh. I'll try to make time to review & apply this.

    @gvanrossum gvanrossum self-assigned this Dec 19, 2007
    @nevyn
    Copy link
    Mannequin

    nevyn mannequin commented Dec 19, 2007

    I've applied the last patch I posted to recent RHEL and Fedora
    releases, and it doesn't seem to break anything ... and from what I
    could see it fixed the problem.

    @donmez
    Copy link
    Mannequin Author

    donmez mannequin commented Dec 19, 2007

    Same here for Pardus Linux, applied the patch without a regression.

    @gvanrossum
    Copy link
    Member

    Sorry this missed the 2.5.2 release. I'll try to look again before
    2.5.3 is imminent.

    @gvanrossum gvanrossum added extension-modules C modules in the Modules dir and removed stdlib Python modules in the Lib dir labels Mar 18, 2008
    @chmod007
    Copy link
    Mannequin

    chmod007 mannequin commented Mar 29, 2008

    The following test cases still cause bus errors with the patch applied:

    import imageop; imageop.rgb82rgb('A'*(2**30), 32768, 32768)
    import imageop; imageop.grey2rgb('A'*(2**30), 32768, 32768)

    @nnorwitz
    Copy link
    Mannequin

    nnorwitz mannequin commented Apr 5, 2008

    I think this was a module that I skipped. I think Anthony might have
    had a patch, but if we have a fix, I'm not sure it matters. We need to
    fix this for 2.5.3, upping the priority.

    @nnorwitz nnorwitz mannequin added the release-blocker label Apr 5, 2008
    @chmod007
    Copy link
    Mannequin

    chmod007 mannequin commented Apr 7, 2008

    Uploading patch that addresses the test cases above. It applies on top of
    nevyn’s latest patch.

    @warsaw
    Copy link
    Member

    warsaw commented May 8, 2008

    This is not a release blocker for 2.6 or 3.0.

    @donmez
    Copy link
    Mannequin Author

    donmez mannequin commented May 8, 2008

    This _must_ be a release blocker for Python 3.0, Its a shame that this
    bug still is not fixed and a patch is available for months now.

    @gvanrossum
    Copy link
    Member

    imageop is deleted in 3.0. See PEP-3108. So it can't be a release
    blocker. This also explains my general lack of interest in this module.

    @donmez
    Copy link
    Mannequin Author

    donmez mannequin commented May 8, 2008

    I am sorry for the drama then, :)

    @benjaminp
    Copy link
    Contributor

    Does anybody still care about this for 2.6?

    @gvanrossum
    Copy link
    Member

    The two segfaults reported in msg64682 are still there in 2.6.
    I'm elevating this to release blocker but don't have time to fix this
    myself.

    @gvanrossum gvanrossum removed their assignment Aug 5, 2008
    @gvanrossum
    Copy link
    Member

    Looking into this now.

    @gvanrossum
    Copy link
    Member

    Latest patches applied to 2.5 branch: r65878.
    And to 2.6 trunk: r65880.

    @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 release-blocker type-security A security issue
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants