#2572 wchar regression test crashes for host

[Maarten Brock]

When I run the regression tests on Ubuntu LTS 16.04 it crashes:

Running host regression tests
*** buffer overflow detected ***: gen/host/wchar/wchar.bin terminated
======= Backtrace: =========
/lib/x86_64-linux-gnu/libc.so.6(+0x777e5)[0x7f46eaa637e5]
/lib/x86_64-linux-gnu/libc.so.6(__fortify_fail+0x5c)[0x7f46eab0456c]
/lib/x86_64-linux-gnu/libc.so.6(+0x116570)[0x7f46eab02570]
/lib/x86_64-linux-gnu/libc.so.6(+0x117c35)[0x7f46eab03c35]
gen/host/wchar/wchar.bin[0x400bbb]
gen/host/wchar/wchar.bin[0x4007d5]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf0)[0x7f46eaa0c830]
gen/host/wchar/wchar.bin[0x400909]
======= Memory map: ========
00400000-00402000 r-xp 00000000 08:01 2905308                            /home/maarten/sdcc/support/regression/gen/host/wchar/wchar.bin
00601000-00602000 r--p 00001000 08:01 2905308                            /home/maarten/sdcc/support/regression/gen/host/wchar/wchar.bin
00602000-00603000 rw-p 00002000 08:01 2905308                            /home/maarten/sdcc/support/regression/gen/host/wchar/wchar.bin
00971000-00993000 rw-p 00000000 00:00 0                                  [heap]
7f46ea7d6000-7f46ea7ec000 r-xp 00000000 08:01 3413003                    /lib/x86_64-linux-gnu/libgcc_s.so.1
7f46ea7ec000-7f46ea9eb000 ---p 00016000 08:01 3413003                    /lib/x86_64-linux-gnu/libgcc_s.so.1
7f46ea9eb000-7f46ea9ec000 rw-p 00015000 08:01 3413003                    /lib/x86_64-linux-gnu/libgcc_s.so.1
7f46ea9ec000-7f46eabab000 r-xp 00000000 08:01 3412929                    /lib/x86_64-linux-gnu/libc-2.23.so
7f46eabab000-7f46eadab000 ---p 001bf000 08:01 3412929                    /lib/x86_64-linux-gnu/libc-2.23.so
7f46eadab000-7f46eadaf000 r--p 001bf000 08:01 3412929                    /lib/x86_64-linux-gnu/libc-2.23.so
7f46eadaf000-7f46eadb1000 rw-p 001c3000 08:01 3412929                    /lib/x86_64-linux-gnu/libc-2.23.so
7f46eadb1000-7f46eadb5000 rw-p 00000000 00:00 0 
7f46eadb5000-7f46eaddb000 r-xp 00000000 08:01 3408048                    /lib/x86_64-linux-gnu/ld-2.23.so
7f46eafbb000-7f46eafbe000 rw-p 00000000 00:00 0 
7f46eafd7000-7f46eafda000 rw-p 00000000 00:00 0 
7f46eafda000-7f46eafdb000 r--p 00025000 08:01 3408048                    /lib/x86_64-linux-gnu/ld-2.23.so
7f46eafdb000-7f46eafdc000 rw-p 00026000 08:01 3408048                    /lib/x86_64-linux-gnu/ld-2.23.so
7f46eafdc000-7f46eafdd000 rw-p 00000000 00:00 0 
7ffc0bb85000-7ffc0bba6000 rw-p 00000000 00:00 0                          [stack]
7ffc0bbd3000-7ffc0bbd5000 r--p 00000000 00:00 0                          [vvar]
7ffc0bbd5000-7ffc0bbd7000 r-xp 00000000 00:00 0                          [vdso]
ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0                  [vsyscall]
Aborted (core dumped)
wchar                               (f:  0, t:   0, c:  0, b:      0, T:        0)
Summary for 'host': 1 failures, 9645 tests, 1806 test cases, 0 bytes, 0 ticks
   Failure: results/host/wchar.out

This is my gcc:
$ gcc --version
gcc (Ubuntu 5.4.0-6ubuntu1~16.04.4) 5.4.0 20160609

Maarten

[Philipp Klaus Krause]

Do you know which of the functions in tests/wchar.c causes this issue?

Philipp

[Maarten Brock]

It crashes in testwcharstringnorestart()
And it's this line:
ASSERT(mbstowcs(wcs2, mbs, 5 * MB_LEN_MAX) > 0);

[Philipp Klaus Krause]

I can't reproduce the issue on my Debian GNU/Linux system. In revision #9825, I added some more tests for testwcharstringnorestart(). This might help narrow down the issue.

Philipp

[Philipp Klaus Krause]

How did the changes in revision [r9825] affect your test-host? Which lines do fail now?

Philipp

[Maarten Brock]

Now these two tests fail with a crash due to buffer overflow:

  ASSERT(wcstombs(mbs, wcs1, 1000) > 0);
  ASSERT(mbstowcs(wcs2, mbs, 1000) > 0);

[Philipp Klaus Krause]

So apparently, your host ignores the 0-termination fos strings, and consider only the parameter n for string length?
Or maybe the 0-termination gets omitted when storing strings?

Philipp

[Maarten Brock]

This still fails on Ubuntu 18.04 LTS with gcc version 7.5.0
*** buffer overflow detected ***: gen/host/wchar/wchar.bin terminated

[Philipp Klaus Krause]

What should we do? Create a small reproducer and file a bug against the glibc package in Ubuntu (since it is a buffer overflow, it might be considered a security issue, and thus in need of fixing even for LTS).

The C standard clearly states that wcstombs and mbstowcs are not allowed to read beyond a terminating 0.

[Philipp Klaus Krause]

Which architecture is your Ubuntu 18.04 LTS for? x86? amd64? I'd like to try to reproduce this issue in a VM.

[Maarten Brock]

$ uname -a
Linux ubuntults 4.15.0-99-generic #100-Ubuntu SMP Wed Apr 22 20:32:56 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux

[Philipp Klaus Krause]

I can reproduce the behaviour using Ubuntu 18.04 in qemu.

P.S.: I tried the various compilers that come with Ubuntu using -O2. I can reproduce the issue using GCC 5.5.0, GCC 6.5.0, GCC 7.5.0 and GCC 8.4.0, but not LLVM 60.0 and LLVM 7.0.0.

On the other hand, I was not able to reproduce the issue on my Debian system using any GCC or LLVM there (but the oldest GCC there is GCC 8.4.0, so the only version I tired on bth Ubuntu and Debian is that). Does Ubuntu use some patched version of GCC that introduces this bug?

[Maarten Brock]

Things get fuzzier. When I compile manually, it just passes:

support/regression/gen/host/wchar$ gcc wchar.c -o wchar -I ../../../fwk/include/ -DPORT_HOST ../testfwk.o ../support.o
support/regression/gen/host/wchar$ ./wchar
--- Running: gen/host/wchar/wchar
Running testwcharnorestart
Running testwcharstringnorestart
Running testwcharrestart
Running testchar16restart
Running testchar32restart
--- Summary: 0/45/5: 0 failed of 45 tests in 5 cases.

[Maarten Brock]

When I enable optimization I get compilation warnings and then the runtime fails:

support/regression/gen/host/wchar$ gcc wchar.c -o wchar -I ../../../fwk/include/ -DPORT_HOST ../testfwk.o ../support.o -O2
In file included from /usr/include/stdlib.h:1020:0,
                 from wchar.c:5:
In function ‘wcstombs’,
    inlined from ‘testwcharstringnorestart’ at wchar.c:72:2,
    inlined from ‘__runSuite’ at wchar.c:175:3:
/usr/include/x86_64-linux-gnu/bits/stdlib.h:152:9: warning: call to ‘__wcstombs_chk_warn’ declared with attribute warning: wcstombs called with dst buffer smaller than len
  return __wcstombs_chk_warn (__dst, __src, __len, __bos (__dst));
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In function ‘mbstowcs’,
    inlined from ‘testwcharstringnorestart’ at wchar.c:73:2,
    inlined from ‘__runSuite’ at wchar.c:175:3:
/usr/include/x86_64-linux-gnu/bits/stdlib.h:123:9: warning: call to ‘__mbstowcs_chk_warn’ declared with attribute warning: mbstowcs called with dst buffer smaller than len * sizeof (wchar_t)
  return __mbstowcs_chk_warn (__dst, __src, __len,
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
          __bos (__dst) / sizeof (wchar_t));
          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

support/regression/gen/host/wchar$ ./wchar
--- Running: gen/host/wchar/wchar
Running testwcharnorestart
Running testwcharstringnorestart
*** buffer overflow detected ***: ./wchar terminated
Aborted (core dumped)

[Maarten Brock]

This leads me to believe that it is not reading beyond the terminating 0, but that it is writing 1000 bytes to mbs which is only 5 * MB_LEN_MAX bytes long. Where is specified that this function is not allowed to clear out the full destination buffer?

IMO the test is wrong and should be fixed by making mbs larger and use its size for wcstombs parameter max.

[Philipp Klaus Krause]

See section 7.22.8 of N2479, the current C standard draft:

"The wcstombs function converts a sequence of wide characters from the array pointed to by pwcs into a sequence of corresponding multibyte characters that begins in the initial shift state, and stores these multibyte characters into the array pointed to by s, stopping if a multibyte character would exceed the limit of n total bytes or if a null character is stored."

IMO, this states that wcstombs stops storing bytes after it stored a null character.

[Philipp Klaus Krause]

However, the wording for mbstowcs seems less clear to me. I'll try to get clarification from comp.lang.c (and failing that from WG14).

[Philipp Klaus Krause]

Doing the same on my Debian system gives no warnings (not even with -Wall -pedantic). I tried gcc 9.3.0 and clang 9.0.1.

[Maarten Brock]

Fixed in SDCC [r11611]

[Philipp Klaus Krause]

I do not agree with the fix. IMO, the specification does not allow these functions to store characters beyond the terminating 0.
The point of the two tests with the large length parameter was to check that terminatin zeroes are handled correctly.

[Maarten Brock]

Then it is maybe better to check that the destination was not changed after the stored terminating zero. I still think that lying to the function about the size of the destination is wrong.

I have not checked what was actually written after the terminating zero. It could be more zeroes, but it could also very well be a 32-bit or 64-bit read-modify-write. The latter would not result in anything changed, but it does mean writing outside the buffer, because you promised that the buffer was larger than it actually is.

[Philipp Klaus Krause]

I do not see the parameter n as a promise on buffer size. I see it as another way to limit the size of the output.

Full wording for wcstombs in N2479:

"
7.22.8.2 The wcstombs function

Synopsis

#include <stdlib.h></stdlib.h>

size_t mbstowcs(wchar_t * restrict pwcs, const char *restrict s, size_t n);

Description

The mbstowcs function converts a sequence of multibyte characters that
begins in the initial shift state from the array pointed to by s into a
sequence of corresponding wide characters and stores not more than n
wide characters into the array pointed to by pwcs. No multibyte
characters that follow a null character (which is converted into a null
wide character) will be examined or converted. Each multibyte character
is converted as if by a call to the mbtowc function, except that the
conversion state of the mbtowc function is not affected.

No more than n elements will be modified in the array pointed to by
pwcs. If copying takes place between objects that overlap, the behavior
is undefined.

Returns

If a wide character is encountered that does not correspond to a valid multibyte character, the wcstombs function returns (size_t)(-1) . Otherwise, the wcstombs function returns the number of bytes modified, not including a terminating null character, if any.324)

324) The array will not be null-terminated if the value returned is n.
"

[Maarten Brock]

The warning issued by gcc seems to indicate it is a promise on buffer size.

[Philipp Klaus Krause]

The question was discussed on comp.lang.c and comp.std.c today. A glibc developer joined, which helped clarify:

The test failure was due to glibc with -D_FORTIFY_SOURCE=2 not being a conforming C implementation.

So I restored the old test, but made it conditional _FORTIFY_SOURCE.

I still don't know why Ubuntu decided to make D_FORTIFY_SOURCE=2 the default for GCC (but not LLVM), deviating from Debian.