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

[orenmn]

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]

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]

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]

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]

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]

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

[ned-deily]

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

[serhiy-storchaka]

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]

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]

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]

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