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.

Sign up for GitHub

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

Jump to bottom

crash in os.utime() in case of a bad ns argument #75758

Closed
orenmn mannequin opened this issue Sep 25, 2017 · 12 comments
Closed

crash in os.utime() in case of a bad ns argument #75758

orenmn mannequin opened this issue Sep 25, 2017 · 12 comments
Labels
3.7 (EOL) end of life 3.8 only security fixes extension-modules C modules in the Modules dir type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@orenmn
Copy link
Mannequin

orenmn mannequin commented Sep 25, 2017

BPO 31577
Nosy @mdickinson, @ned-deily, @zware, @serhiy-storchaka, @orenmn, @miss-islington, @tirkarthi
PRs
  • bpo-31577: Fix a crash in os.utime() in case of a bad ns argument #3752
  • [3.7] bpo-31577: Fix a crash in os.utime() in case of a bad ns argument. (GH-3752) #9222
    		•

    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 2018-09-14.20:50:26.547>
created_at = <Date 2017-09-25.16:47:57.601>
labels = ['extension-modules', '3.7', '3.8', 'type-crash']
title = 'crash in os.utime() in case of a bad ns argument'
updated_at = <Date 2018-09-14.20:50:26.545>
user = 'https://github.com/orenmn'

    bugs.python.org fields:

    activity = <Date 2018-09-14.20:50:26.545>
actor = 'zach.ware'
assignee = 'none'
closed = True
closed_date = <Date 2018-09-14.20:50:26.547>
closer = 'zach.ware'
components = ['Extension Modules']
creation = <Date 2017-09-25.16:47:57.601>
creator = 'Oren Milman'
dependencies = []
files = []
hgrepos = []
issue_num = 31577
keywords = ['patch']
message_count = 12.0
messages = ['302962', '302976', '303020', '324940', '324941', '325154', '325166', '325171', '325174', '325257', '325259', '325397']
nosy_count = 7.0
nosy_names = ['mark.dickinson', 'ned.deily', 'zach.ware', 'serhiy.storchaka', 'Oren Milman', 'miss-islington', 'xtreak']
pr_nums = ['3752', '9222']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue31577'
versions = ['Python 3.7', 'Python 3.8']
    @orenmn
    Copy link
    Mannequin Author

    orenmn mannequin commented Sep 25, 2017

    The following code causes the interpreter to crash:
    class BadInt:
    def __divmod__(*args):
    return 42

    import os
os.utime('foo.txt', ns=(BadInt(), 1))

    This is because split_py_long_to_s_and_ns() (in Modules/posixmodule.c) assumes
    that PyNumber_Divmod() returns a 2-tuple, and passes it to PyTuple_GET_ITEM(),
    which assumes it is a tuple. Thus, PyTuple_GET_ITEM() might return a non-NULL
    value which is not an address of a Python object.

    @orenmn orenmn mannequin added 3.7 (EOL) end of life extension-modules C modules in the Modules dir type-crash A hard crash of the interpreter, possibly with a core dump labels Sep 25, 2017
    @orenmn
    Copy link
    Mannequin Author

    orenmn mannequin commented Sep 25, 2017

    I opened a PR.
    I think another fix might be to use
    PyLong_Type.tp_as_number->long_divmod() instead of PyNumber_Divmod().

    @serhiy-storchaka
    Copy link
    Member

    There is also similar issue in timedelta.__divmod__.

    PyLong_Type.tp_as_number->nb_divmod() works only with integers.

    The different way of solving this issue is used in microseconds_to_delta_ex() in _datetimemodule.c.

    Perhaps the best solution is to add a check that the result of nb_divmod() is a 2-tuple in PyNumber_Divmod(). This could fix similar errors in third-party code. What is your thoughts Mark?

    @zware
    Copy link
    Member

    zware commented Sep 10, 2018

    We definitely can't make that change to PyNumber_Divmod in 3.7 at this point, I'm sure someone somewhere is relying on being able to get arbitrary information out of their objects with divmod(crazy_object). I don't know enough math to say whether there could be any legitimate mathematical use for arbitrary return values so I leave it to others to determine whether we could make that consider that change in 3.8 just to clean things up.

    I've looked through _datetimemodule.c and I don't see how timedelta.__divmod__ could fail like this, since it actually creates new timedelta objects from its arguments to work from.

    @zware zware added the 3.8 only security fixes label Sep 10, 2018
    @zware
    Copy link
    Member

    zware commented Sep 10, 2018

    Adding Ned and marking as release blocker as this is a crasher in 3.7.0.

    @ned-deily
    Copy link
    Member

    @Serihy, @mark, others, any suggestions for what to do for 3.7.1?

    @serhiy-storchaka
    Copy link
    Member

    PR 3752 LGTM. I have reran CI tests, if they will be passed, the PR can be merged. This PR was not merged only because we discussed possible alternate solutions and lost an opportunity to merge it for 3.7.0.

    I agree that timedelta.__divmod__ doesn't have such issue.

    @serhiy-storchaka
    Copy link
    Member

    New changeset 0bd1a2d by Serhiy Storchaka (Oren Milman) in branch 'master':
    bpo-31577: Fix a crash in os.utime() in case of a bad ns argument. (GH-3752)
    0bd1a2d

    @miss-islington
    Copy link
    Contributor

    New changeset 329ea4e by Miss Islington (bot) in branch '3.7':
    bpo-31577: Fix a crash in os.utime() in case of a bad ns argument. (GH-3752)
    329ea4e

    @ned-deily
    Copy link
    Member

    Thanks, Serihy. Can we either close this now or remove 3.7 and "release blocker"?

    @zware
    Copy link
    Member

    zware commented Sep 13, 2018

    The crash is fixed, so I've lowered the priority. Serhiy, are you still interested in pursuing an alternative fix in 3.8, or content with what's merged?

    @zware
    Copy link
    Member

    zware commented Sep 14, 2018

    Serhiy has opened bpo-34676 for the idea of restricting PyNumber_Divmod()'s return type, so I'm closing the issue.

    @zware zware closed this as completed Sep 14, 2018
    @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
    Assignees
    No one assigned
    Labels
    3.7 (EOL) end of life 3.8 only security fixes extension-modules C modules in the Modules dir type-crash A hard crash of the interpreter, possibly with a core dump
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    4 participants
    @serhiy-storchaka @ned-deily @zware @miss-islington