Pylauncher, launcher.c: Assigning NULL to a pointer instead of testing against NULL

[ariccio]

BPO	25844
Nosy	@pfmoore, @vsajip, @tjguk, @zware, @serhiy-storchaka, @zooba, @ariccio

^{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 2015-12-18.17:50:51.060>
created_at = <Date 2015-12-12.04:33:27.785>
labels = ['OS-windows', 'type-crash']
title = 'Pylauncher, launcher.c: Assigning NULL to a pointer instead of testing against NULL'
updated_at = <Date 2015-12-21.06:47:30.230>
user = 'https://github.com/ariccio'

bugs.python.org fields:

activity = <Date 2015-12-21.06:47:30.230>
actor = 'python-dev'
assignee = 'none'
closed = True
closed_date = <Date 2015-12-18.17:50:51.060>
closer = 'serhiy.storchaka'
components = ['Windows']
creation = <Date 2015-12-12.04:33:27.785>
creator = 'Alexander Riccio'
dependencies = []
files = []
hgrepos = []
issue_num = 25844
keywords = []
message_count = 4.0
messages = ['256254', '256319', '256697', '256784']
nosy_count = 8.0
nosy_names = ['paul.moore', 'vinay.sajip', 'tim.golden', 'python-dev', 'zach.ware', 'serhiy.storchaka', 'steve.dower', 'Alexander Riccio']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue25844'
versions = ['Python 3.5', 'Python 3.6']

[ariccio]

I found this while writing up a separate bug (CPython doesn't use static analysis!).

In PC/launcher.c, get_env has a bug:

        /* Large environment variable. Accept some leakage */
        wchar_t *buf2 = (wchar_t*)malloc(sizeof(wchar_t) * (result+1));
        if (buf2 = NULL) {
            error(RC_NO_MEMORY, L"Could not allocate environment buffer");
        }
        GetEnvironmentVariableW(key, buf2, result);
        return buf2;

See: https://hg.python.org/cpython/file/tip/PC/launcher.c#l117

Instead of buf2 == NULL, Vinay Sajip wrote buf2 = NULL. The commit where the error was introduced: https://hg.python.org/cpython/rev/4123e002a1af

Thus, whatever value was in buf2 is lost, the branch is NOT taken (because buf2 evaluates to false), and GetEnvironmentVariableW will (probably) cause an access violation.

Compiling with /analyze found this quite easily:

c:\pythondev\repo\pc\launcher.c(117): warning C6282: Incorrect operator: assignment of constant in Boolean context. Consider using '==' instead.

[python-dev]

New changeset 5015440beec2 by Vinay Sajip in branch '3.4':
Fixes bpo-25844: Corrected =/== typo potentially leading to crash in launcher.
https://hg.python.org/cpython/rev/5015440beec2

New changeset 9552fcd303fd by Vinay Sajip in branch '3.5':
Fixes bpo-25844: Corrected =/== typo potentially leading to crash in launcher.
https://hg.python.org/cpython/rev/9552fcd303fd

New changeset cdf8033d8820 by Vinay Sajip in branch 'default':
Fixes bpo-25844: Merged fix from 3.5.
https://hg.python.org/cpython/rev/cdf8033d8820

[serhiy-storchaka]

To the fix for this bug we have to provoke the unsufficient memory error just at this point. This is unlikely worth.

Thank you Alexander for your reports.

[python-dev]

New changeset 48b3cac0dbcd by Vinay Sajip in branch '3.4':
Fixes bpo-25844: Corrected =/== typo potentially leading to crash in launcher.
https://hg.python.org/cpython/rev/48b3cac0dbcd