buffer overrun in wcstombs_errorpos()

[tiran]

BPO	15930
Nosy	@birkenfeld, @vstinner, @tiran, @skrah

^{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 2012-09-12.14:44:01.196>
created_at = <Date 2012-09-12.12:36:41.349>
labels = ['invalid', 'performance']
title = 'buffer overrun in wcstombs_errorpos()'
updated_at = <Date 2012-09-12.14:44:01.195>
user = 'https://github.com/tiran'

bugs.python.org fields:

activity = <Date 2012-09-12.14:44:01.195>
actor = 'christian.heimes'
assignee = 'none'
closed = True
closed_date = <Date 2012-09-12.14:44:01.196>
closer = 'christian.heimes'
components = []
creation = <Date 2012-09-12.12:36:41.349>
creator = 'christian.heimes'
dependencies = []
files = []
hgrepos = []
issue_num = 15930
keywords = ['3.3regression']
message_count = 5.0
messages = ['170373', '170376', '170377', '170383', '170384']
nosy_count = 4.0
nosy_names = ['georg.brandl', 'vstinner', 'christian.heimes', 'skrah']
pr_nums = []
priority = 'normal'
resolution = 'not a bug'
stage = None
status = 'closed'
superseder = None
type = 'resource usage'
url = 'https://bugs.python.org/issue15930'
versions = ['Python 3.3']

[tiran]

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)).

[tiran]

Georg,
this issue might be security relevant and should be reviewed before the next release.

[skrah]

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.

[skrah]

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);

[tiran]

Stefan,
I agree with your analysis. With the terminating null wide char wcstombs will never read beyond the end of buf.