classification
Title: Uninitialized variable may be used in PyUnicode_DecodeUTF7Stateful()
Type: Stage:
Components: Versions: Python 2.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: georg.brandl, gvanrossum, lemburg, loewis, pitrou
Priority: normal Keywords:

Created on 2009-02-27 22:13 by gvanrossum, last changed 2009-03-05 21:47 by gvanrossum. This issue is now closed.

Messages (9)
msg82881 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2009-02-27 22:13
[Found by a Googler who prefers to remain anonymous]

This might be easier to trigger on a 64-bit:

PyObject *PyUnicode_DecodeUTF7Stateful(...)
{
    ...
    Py_ssize_t startinpos;
    ...
    while (s < e) {
    ...
      utf7Error:
        outpos = p-PyUnicode_AS_UNICODE(unicode);
        endinpos = s-starts;
        if (unicode_decode_call_errorhandler(
                errors, &errorHandler,
                "utf7", errmsg,
                starts, size, &startinpos, &endinpos, &exc, &s,
                &unicode, &outpos, &p))
        ...
    }
    ...
}

The lack of initialization of startinpos will lead to the likelihood of
the value being >= INT_MAX with a 64-bit value, leading to the
subsequent assert [somewhere in unicode_decode_call_errorhandler()]. In
theory the assert could trigger in 32-bit if the uninitialized value
happened to get set to INT_MAX.

The other similar variable also probably need to be initialized.
Furthermore, the function PyUnicode_DecodeUTF8Stateful also has the same
uninitialized variables.
msg82897 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2009-02-28 10:34
I can't see at the moment how the unicode_decode_call_errorhandler call
can be made without startinpos being previously set to some value.
Antoine, maybe you can verify?
msg82901 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-02-28 10:51
Hmm, I know nothing about UTF7...

Anyway, looking at the code, the utf7Error code path can be called from
the following places (trunk line numbers):
- line 1595, and startinpos was set three lines before
- a bunch of places in the "if (inShift) { ... }" chunk between lines
1537 and 1578; inShift would have had previously been set to 1 and
that's at line 1587, a couple of lines after setting startinpos

So it seems things are fine, but perhaps I'm missing something.
msg83007 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2009-03-02 09:00
The UTF-7 codec implementation has a few problems (one of them is that
it is hardly being used, so bugs only get detected very slowly).

issue4426 has a patch with cleaned up and more standards compliant
implementation. Perhaps that also fixes the problem with uninitialized
variables.
msg83041 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2009-03-02 21:56
It looks like it was fixed in 2.6 by adding an assignment to startinpos
to this block:

       else if (SPECIAL(ch,0,0)) {
           startinpos = s-starts;     /* <---------- This was added */
           errmsg = "unexpected special character";
           s++;
           goto utf7Error;
       }

Are we going to release another 2.5, ever?
msg83186 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2009-03-05 09:28
Only with security fixes IIRC. Letting Martin decide.
msg83207 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2009-03-05 17:34
Well, this one is technically a security fix, though I have no idea how
it could be exploited unless you offer your users a facility to execute
arbitrary Python code.
msg83219 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2009-03-05 21:33
I agree it is technically a security fix, so somebody please feel free
to commit it. I will make another 2.5 release when enough of these have
accumulated, or something urgent happens, or somebody wants to see a
release really badly :-)
msg83222 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2009-03-05 21:47
OK, submitted.
History
Date User Action Args
2009-03-05 21:47:49gvanrossumsetstatus: open -> closed
resolution: fixed
messages: + msg83222
2009-03-05 21:33:31loewissetassignee: loewis ->
messages: + msg83219
2009-03-05 17:34:15gvanrossumsetmessages: + msg83207
2009-03-05 09:28:51georg.brandlsetassignee: pitrou -> loewis
messages: + msg83186
nosy: + loewis
2009-03-02 21:56:02gvanrossumsetmessages: + msg83041
versions: - Python 2.6, Python 3.0, Python 3.1, Python 2.7
2009-03-02 09:00:57lemburgsetnosy: + lemburg
messages: + msg83007
2009-02-28 10:51:09pitrousetmessages: + msg82901
2009-02-28 10:34:16georg.brandlsetassignee: pitrou
messages: + msg82897
nosy: + georg.brandl, pitrou
2009-02-27 22:13:12gvanrossumcreate