time_strftime() Buffer Over-read

@malemburg

BPO	24917
Nosy	@malemburg, @birkenfeld, @pfmoore, @abalkin, @vstinner, @larryhastings, @tjguk, @zware, @eryksun, @zooba
Superseder	bpo-25029: MemoryError in test_strptime
Files	time_strftime_Buffer_Over-read.patch: patch time_strftime_Buffer_Over-read.py: repro time_strftime_Buffer_Over-read_v2.patch: patch time_strftime_Buffer_Over-read_v3.patch 24917_4.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 = 'https://github.com/zooba'
closed_at = <Date 2015-09-08.15:15:28.839>
created_at = <Date 2015-08-22.23:39:19.706>
labels = ['type-security', 'extension-modules', 'deferred-blocker', 'OS-windows']
title = 'time_strftime() Buffer Over-read'
updated_at = <Date 2015-09-08.15:15:28.839>
user = 'https://bugs.python.org/JohnLeitch'

bugs.python.org fields:

activity = <Date 2015-09-08.15:15:28.839>
actor = 'steve.dower'
assignee = 'steve.dower'
closed = True
closed_date = <Date 2015-09-08.15:15:28.839>
closer = 'steve.dower'
components = ['Extension Modules', 'Windows']
creation = <Date 2015-08-22.23:39:19.706>
creator = 'JohnLeitch'
dependencies = []
files = ['40228', '40229', '40367', '40368', '40383']
hgrepos = []
issue_num = 24917
keywords = ['patch']
message_count = 67.0
messages = ['248998', '249800', '249801', '249805', '249806', '249857', '249860', '249861', '249862', '249863', '249864', '249867', '249868', '249870', '249872', '249874', '249876', '249877', '249878', '249881', '249883', '249884', '249899', '249904', '249905', '249908', '249919', '249920', '249950', '249956', '249958', '249960', '249961', '249962', '249963', '249964', '249965', '249966', '249967', '249969', '249970', '249971', '249972', '249973', '249974', '249975', '249976', '249977', '249994', '250014', '250020', '250022', '250023', '250024', '250030', '250031', '250035', '250039', '250040', '250041', '250042', '250062', '250064', '250071', '250202', '250203', '250213']
nosy_count = 14.0
nosy_names = ['lemburg', 'georg.brandl', 'paul.moore', 'belopolsky', 'vstinner', 'larry', 'tim.golden', 'BreamoreBoy', 'python-dev', 'zach.ware', 'eryksun', 'steve.dower', 'JohnLeitch', 'brycedarling']
pr_nums = []
priority = 'deferred blocker'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = '25029'
type = 'security'
url = 'https://bugs.python.org/issue24917'
versions = ['Python 3.4', 'Python 3.5']

Python 3.5 suffers from a vulnerability caused by the behavior of the time_strftime() function. When called, the function loops over the format string provided, using strchr to search for each instance of '%'. After finding a '%', it continues to search two characters ahead, assuming that each instance is the beginning of a well formed format string token.

However, if a string ends with '%', this logic will result in a call to strchr that reads off the end of the format string buffer:

    /* check that the format string contains only valid directives */
    for(outbuf = strchr(fmt, '%'); <<<< Assuming fmt ends with a '%', this will return a pointer to the end of the string.
        outbuf != NULL;
        outbuf = strchr(outbuf+2, '%')) <<<< Once outbuf is pointing to the end of the string, outbuf+2 skips
    {                                        past the null terimnator, leading to a buffer over-read.
        if (outbuf[1]=='#')
            ++outbuf; /* not documented by python, */
        if ((outbuf[1] == 'y') && buf.tm_year < 0)
        {
            PyErr_SetString(PyExc_ValueError,
                        "format %y requires year >= 1900 on Windows");
            Py_DECREF(format);
            return NULL;
        }
    }

In some applications, it may be possible to exploit this behavior to disclose the contents of adjacent memory. The buffer over-read can be observed by running the following script:

from time import *
strftime("AA%"*0x10000)

Which, depending on the arrangement of memory, may produce an exception such as this:

0:000> g
(20b8.18d4): Access violation - code c0000005 (first chance)
First chance exceptions are reported before any exception handling.
This exception may be expected and handled.
eax=ffffffff ebx=52c1a6a0 ecx=00000000 edx=08ef3000 esi=08ec2fe8 edi=08ec2ff8
eip=52d254f3 esp=004cf9d4 ebp=004cfa58 iopl=0 nv up ei pl nz na pe nc
cs=0023 ss=002b ds=002b es=002b fs=0053 gs=002b efl=00010206
python35!strchr+0x33:
52d254f3 f30f6f0a movdqu xmm1,xmmword ptr [edx] ds:002b:08ef3000=????????????????????????????????
0:000> db edx-0x10
08ef2ff0 41 25 41 41 25 41 41 25-00 d0 d0 d0 d0 d0 d0 d0 A%AA%AA%........
08ef3000 ?? ?? ?? ?? ?? ?? ?? ??-?? ?? ?? ?? ?? ?? ?? ?? ????????????????
08ef3010 ?? ?? ?? ?? ?? ?? ?? ??-?? ?? ?? ?? ?? ?? ?? ?? ????????????????
08ef3020 ?? ?? ?? ?? ?? ?? ?? ??-?? ?? ?? ?? ?? ?? ?? ?? ????????????????
08ef3030 ?? ?? ?? ?? ?? ?? ?? ??-?? ?? ?? ?? ?? ?? ?? ?? ????????????????
08ef3040 ?? ?? ?? ?? ?? ?? ?? ??-?? ?? ?? ?? ?? ?? ?? ?? ????????????????
08ef3050 ?? ?? ?? ?? ?? ?? ?? ??-?? ?? ?? ?? ?? ?? ?? ?? ????????????????
08ef3060 ?? ?? ?? ?? ?? ?? ?? ??-?? ?? ?? ?? ?? ?? ?? ?? ????????????????
0:000> k5
ChildEBP RetAddr
004cf9d0 52c1a7f python35!strchr+0x33 [f:\dd\vctools\crt\vcruntime\src\string\i386\strchr_sse.inc @ 75]
004cfa58 52c832d3 python35!time_strftime+0x156 [c:\build\cpython\modules\timemodule.c @ 615]
004cfa74 52ce442f python35!PyCFunction_Call+0x113 [c:\build\cpython\objects\methodobject.c @ 109]
004cfaa8 52ce18ec python35!call_function+0x2ff [c:\build\cpython\python\ceval.c @ 4651]
004cfb20 52ce339f python35!PyEval_EvalFrameEx+0x232c [c:\build\cpython\python\ceval.c @ 3184]

To fix this issue, it is recommended that time_strftime() be updated to check outputbuf[1] for null in the body of the format string directive validation loop. A proposed patch is attached.

Credit: John Leitch (johnleitch@outlook.com), Bryce Darling (darlingbryce@gmail.com)

Is there a CERT report associated with this vulnerability?

Currently, no. Would you like us to report this and future vulnerabilities to CERT?

This is happening in windows-only code, so not being a windows user I cannot reproduce it. It does not look like something new. This code was last touched in bpo-10653.

Can someone confirm that only 3.5 is affected?

Would you like us to report this and future vulnerabilities to CERT?

If it affects released versions, I think this is the right thing to do. Note that I don't know what PSF policy is on this is or whether they even have one.

I have tried the reproducer on Windows 10 with 2.6, 2.7, 3.3, 3.4, 3.5 and 3.6. In every case I got this.

C:\py -3.4 -c"from time import *;strftime('AA%'*0x10000)"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ValueError: Invalid format string

In every case I got [ValueError]

You shouldn't have. I am no Windows expert, but I suspect something is wrong with your use of the command line. Please try it at the Python prompt or put the code in a script.

I have tried the reproducer on Windows 10 with 2.6, 2.7, 3.3, 3.4, 3.5 and 3.6. In every case I got this.

What you are observing is due to the arrangement and contents of process memory. With a simple repro (such as the one provided), there's a good chance the null terminator of the format string will be followed by more null bytes, and thus the code will appear to work as intended. In more complex scripts where memory is ultimately reused, it's more likely that the null terminator will be followed by garbage, non-null bytes.

To make the issue reproduce more reliably, use GFlags to enable heap tail checking, heap free checking, and page heap.

https://msdn.microsoft.com/en-us/library/windows/hardware/ff549557(v=vs.85).aspx

Then, when you repro the issue, you'll see the crash because the uninitialized memory will contain the fill pattern 0xd0 rather than 0x00, like this:

0:000> db edx-0x10
08ef2ff0 41 25 41 41 25 41 41 25-00 d0 d0 d0 d0 d0 d0 d0 A%AA%AA%........
08ef3000 ?? ?? ?? ?? ?? ?? ?? ??-?? ?? ?? ?? ?? ?? ?? ?? ????????????????
08ef3010 ?? ?? ?? ?? ?? ?? ?? ??-?? ?? ?? ?? ?? ?? ?? ?? ????????????????
08ef3020 ?? ?? ?? ?? ?? ?? ?? ??-?? ?? ?? ?? ?? ?? ?? ?? ????????????????
08ef3030 ?? ?? ?? ?? ?? ?? ?? ??-?? ?? ?? ?? ?? ?? ?? ?? ????????????????
08ef3040 ?? ?? ?? ?? ?? ?? ?? ??-?? ?? ?? ?? ?? ?? ?? ?? ????????????????
08ef3050 ?? ?? ?? ?? ?? ?? ?? ??-?? ?? ?? ?? ?? ?? ?? ?? ????????????????
08ef3060 ?? ?? ?? ?? ?? ?? ?? ??-?? ?? ?? ?? ?? ?? ?? ?? ????????????????

To be clear, heap verification is not a requirement--the bug can indeed be reproduced without it. However, it will make life easier by introducing more determinism.

Python prompt or script makes no difference.

C:\Users\Mark\Desktop>py -2.6 time_strftime_Buffer_Over-read.py
Traceback (most recent call last):
  File "time_strftime_Buffer_Over-read.py", line 2, in <module>
    strftime("AA%"*0x10000)
ValueError: Invalid format string

Python 3.5.0rc2 (v3.5.0rc2:cc15d736d860, Aug 25 2015, 05:08:37) [MSC v.1900 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> from time import *
>>> strftime("AA%"*0x10000)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: Invalid format string

Sorry for a bad guess. John's advise makes much more sense than mine.

Okay, so presumably this applies to all supported versions and not just 3.5? Can you also remove the reproducer and supply specific instructions on how to build the code so that the problem can be reproduced.

It very well may apply to versions apart from 3.5. Our test environment is quite complex and unfriendly to working with multiple versions of Python. Plus, we're strapped for time, so we tend to file under the version we're currently targeting and defer to those better equipped and more familiar with the codebase.

The repro file is fine and there's no need to rebuild anything; the heap verification I'm suggesting is applied to binaries, not source. You can find information here:

https://msdn.microsoft.com/en-us/library/windows/hardware/ff549557(v=vs.85).aspx

Install the debugging tools, run GFlags, select the python.exe/pythonw.exe image file, and apply the settings I mentioned in my previous post.

John,

There is no doubt that this is a bona fide bug. I was just hoping that someone with a Windows development machine would help figuring out the affected versions.

From the hg history, it looks like the faulty code is present in all versions starting from 3.2:

https://hg.python.org/cpython/annotate/977c5753ca32/Modules/timemodule.c#l515

I will set the issue versions accordingly.

New PR: https://bitbucket.org/larry/cpython350/pull-requests/19/issue-24917-time_strftime-buffer-over-read

Steve, did you confirm that the test triggers the array bounds bug when the patch *isn't* applied? I want to confirm both that a) the test exercises the bug, and b) the fix fixes the bug. I assume you ran the test suite with the patch applied, so that covers b).

I wasn't able to repro the crash at all, even with the debugging flags that make this sort of issue more prominent. It relies on a very precise layout of multiple objects in memory, or possibly a specific sequence of allocations/deallocations, as well as a format string ending in an unescaped '%' or (on Windows) '%#'.

It's still obviously an issue though - we should have the check for '\0' there by any reasonably analysis of the code, or else should not be adding 2 to the pointer to start the next step of the search.

Okay, I think *I* reproduced it.

I pulled your cpython350 fork down locally.
I updated to your checkin that fixed the bug. (c31dad22c80d)
I reverted the change to Modules/timemodule.c to put the bug back:
% hg cat -r 97393 Modules/timemodule.c > Modules/timemodule.c
I changed line 611 (or so) from "#if defined(MS_WINDOWS) && !defined(HAVE_WCSFTIME)" to "#if 1" so I'd get the code that had the bug.
I ran "./configure --with-valgrind && make" to make it.
I ran "valgrind ./python -m test test_time" and ***Valgrind complained about an array overrun***.
I restored the bugfix to Modules/timemodule.c, then reinstated the change from 4) above.
I ran make and valgrind again and didn't get the complaint about the array overrun.

For grins I also tried enabling the other stanza of code that has the bug (the AIX / sun / have_wcsftime) and observed the same thing.

Is that convincing?

Good enough for me.

Backout pull request merged, please forward-merge, thanks!

(I meant, just normal pull request. I did your two pull requests right in a row and got my wires crossed.)

New changeset c31dad22c80d by Steve Dower in branch '3.5':
Issue bpo-24917: time_strftime() buffer over-read.
https://hg.python.org/cpython/rev/c31dad22c80d

New changeset f185917498ca by Steve Dower in branch '3.4':
Issue bpo-24917: time_strftime() buffer over-read.
https://hg.python.org/cpython/rev/f185917498ca

The change c31dad22c80d introduced a regression: issue bpo-25029.

Oh, tracemalloc sees the memory peak of 8 GB too:

$ ./python -X tracemalloc -i -m test -v test_strptime
...
SystemExit: True
>>> import tracemalloc; tracemalloc.get_traced_memory()[1] / 1024.**2
8201.658247947693

Memory peak: 8201.7 MB (8.2 GB!).

I'll fix it on bpo-25029. This thread is already too long for my liking.

Navigation Menu

Repository navigation

time_strftime() Buffer Over-read #69105

50 remaining items

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

time_strftime() Buffer Over-read #69105

Description

Activity

JohnLeitch commented on Aug 22, 2015

abalkin commented on Sep 4, 2015

JohnLeitch commented on Sep 4, 2015

abalkin commented on Sep 4, 2015

abalkin commented on Sep 4, 2015

BreamoreBoy commented on Sep 4, 2015

abalkin commented on Sep 4, 2015

JohnLeitch commented on Sep 4, 2015

BreamoreBoy commented on Sep 4, 2015

abalkin commented on Sep 4, 2015

BreamoreBoy commented on Sep 4, 2015

JohnLeitch commented on Sep 4, 2015

abalkin commented on Sep 4, 2015

50 remaining items

zooba commented on Sep 7, 2015

larryhastings commented on Sep 7, 2015

zooba commented on Sep 7, 2015

larryhastings commented on Sep 7, 2015

zooba commented on Sep 7, 2015

larryhastings commented on Sep 7, 2015

larryhastings commented on Sep 7, 2015

python-dev commented on Sep 7, 2015

vstinner commented on Sep 8, 2015

vstinner commented on Sep 8, 2015

zooba commented on Sep 8, 2015

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions