classification
Title: buffer overrun in wcstombs_errorpos()
Type: resource usage Stage:
Components: Versions: Python 3.3
process
Status: closed Resolution: not a bug
Dependencies: Superseder:
Assigned To: Nosy List: christian.heimes, georg.brandl, skrah, vstinner
Priority: normal Keywords: 3.3regression

Created on 2012-09-12 12:36 by christian.heimes, last changed 2012-09-12 14:44 by christian.heimes. This issue is now closed.

Messages (5)
msg170373 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2012-09-12 12:36
Coverity has found a buffer overrun in wcstombs_errorpos() defined at 
http://hg.python.org/cpython/file/25e41fdc4e60/Objects/unicodeobject.c#l3237

Message:
CID 719672: Out-of-bounds access (OVERRUN)At (2): Overrunning array "buf" of 2 4-byte elements by passing it to a function which accesses it at element index 15 (byte offset 60) using argument "16UL". 

On a 64bit Linux system SIZE_OF_WCHAR_T is 4 and MB_LEN_MAX 16. In this constellation buf is 8 bytes long (wchar_t[2]) but outbuf has a size of 16 bytes. This causes a buffer overrun in wcstombs(outbuf, buf, sizeof(outbuf)).
msg170376 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2012-09-12 13:00
Georg,
this issue might be security relevant and should be reviewed before the next release.
msg170377 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2012-09-12 13:09
buf[1] contains NUL if SIZE_OF_WCHAR_T is 4.

The man page says:

   size_t wcstombs(char *dest, const wchar_t *src, size_t n)

The conversion can stop for three reasons:

3.  The wide-character string has been completely converted, including the terminating L'\0'.  In
       this case the conversion ends in the initial state.  The number of bytes written to dest, exclud-
       ing the terminating '\0' byte, is returned.


To me this sounds like there cannot be an invalid write.
msg170383 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2012-09-12 14:19
I'm convinced that this is a false positive:

    size_t wcstombs(char *dest, const wchar_t *src, size_t n);

We have: 

     1) buf[0] = *wstr and buf[1] = 0.

So:

     2) wcstombs(NULL, buf, 0) <= 4.


Then the man page says:

   "... the  programmer  should  make  sure  n  is  greater  or  equal  to
    wcstombs(NULL,src,0)+1."


In this case, wcstombs(NULL, buf, 0) + 1 <= 5 and we call:

    wcstombs(char *dest, const wchar_t *src, 16);
msg170384 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2012-09-12 14:44
Stefan,
I agree with your analysis. With the terminating null wide char wcstombs will never read beyond the end of buf.
History
Date User Action Args
2012-09-12 14:44:01christian.heimessetstatus: open -> closed
resolution: not a bug
messages: + msg170384
2012-09-12 14:19:54skrahsetmessages: + msg170383
2012-09-12 13:09:34skrahsetnosy: + skrah
messages: + msg170377
2012-09-12 13:00:05christian.heimessetnosy: + georg.brandl
messages: + msg170376
2012-09-12 12:36:41christian.heimescreate