Emscripten: Upgrade MUSL Version

[EricWF]

Hi All,

I'm wondering if there are any plans to upgrade the MUSL libc version? There are a number of printf bugs that have been fixed since 1.1.15.

I tried upgrading it myself but it's non-trivial and there is very little information about the steps needed to do so. I would be willing to try again if I can be provided with a little direction.

[kripken]

I added some docs for how to do that now, see #7286. Let me know if that helps or if anything still isn't clear.

[stale[bot]]

This issue has been automatically marked as stale because there has been no activity in the past year. It will be closed automatically if no further activity occurs in the next 7 days. Feel free to re-open at any time if this issue is still relevant.

[sbc100]

We should do this.

[nhooyr]

Please do.

https://github.com/emscripten-core/emscripten/blob/46df50cfd170521d91a31e0a74f2dfdeb4522968/system/lib/libc/musl/src/string/strlcpy.c#L19 has caused issues for us as it will read out of bounds when it tries to read 4 bytes at a time. Latest muslc has fixed this: https://git.musl-libc.org/cgit/musl/tree/src/string/strlcpy.c

[kripken]

@nhooyr we updated that file specifically now to fix that issue.

A full update of musl is still very important to do, hopefully someone volunteers to take the time soon!

[kleisauke]

+1 for full update of musl. fwiw, it looks like stpncpy is also affected for the above mentioned bug.

Details

=================================================================
==42==ERROR: AddressSanitizer: global-buffer-overflow on address 0x0226c8e8 at pc 0x009ebc59 bp 0x035abbb0 sp 0x035abbbc
READ of size 4 at 0x0226c8e8 thread T0
    #0 0x9ebc59 in __stpncpy+0x9ebc59 (http://localhost:5000/lib/vips.wasm+0x9ebc59)
    #1 0x9ea6aa in strncpy+0x9ea6aa (http://localhost:5000/lib/vips.wasm+0x9ea6aa)

0x0226c8e8 is located 56 bytes to the left of global variable '<string literal>' defined in 'exif-entry.c:680:27' (0x226c920) of size 10
  '<string literal>' is ascii string 'Top-right'
0x0226c8e9 is located 0 bytes to the right of global variable '<string literal>' defined in 'exif-entry.c:680:11' (0x226c8e0) of size 9
  '<string literal>' is ascii string 'Top-left'
SUMMARY: AddressSanitizer: global-buffer-overflow (http://localhost:5000/lib/vips.wasm+0x9ebc58) in __stpncpy+0x9ebc58
Shadow bytes around the buggy address:
  0x0044d8c0: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
  0x0044d8d0: f9 f9 f9 f9 00 06 f9 f9 f9 f9 f9 f9 00 06 f9 f9
  0x0044d8e0: f9 f9 f9 f9 01 f9 f9 f9 f9 f9 f9 f9 00 00 00 03
  0x0044d8f0: f9 f9 f9 f9 00 00 00 03 f9 f9 f9 f9 00 00 00 05
  0x0044d900: f9 f9 f9 f9 00 00 00 05 f9 f9 f9 f9 00 00 01 f9
=>0x0044d910: f9 f9 f9 f9 00 00 00 07 f9 f9 f9 f9 00[01]f9 f9
  0x0044d920: f9 f9 f9 f9 00 02 f9 f9 f9 f9 f9 f9 00 05 f9 f9
  0x0044d930: f9 f9 f9 f9 00 04 f9 f9 f9 f9 f9 f9 00 01 f9 f9
  0x0044d940: f9 f9 f9 f9 00 02 f9 f9 f9 f9 f9 f9 00 05 f9 f9
  0x0044d950: f9 f9 f9 f9 00 04 f9 f9 f9 f9 f9 f9 00 01 f9 f9
  0x0044d960: f9 f9 f9 f9 00 01 f9 f9 f9 f9 f9 f9 00 06 f9 f9
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==42==ABORTING

(memccpy, memchr, memmove, stpcpy, strchrnul and strlen are probably also affected. See this commit)

[jeromelaban]

I wonder, I see that @kripken has updated many of the MUSL in https://github.com/emscripten-core/emscripten/pull/11215, yet I'm still getting the same buffer overrun issue mentioned in https://github.com/emscripten-core/emscripten/issues/7279#issuecomment-617867062 when using asan with strncpy.

Would someone know more about this ?

The overrun happens for this code, for instance:

char test[] = "12345";
auto target = (char*)malloc(150);
strncpy(target, test, 150);

as the destination size is larger, forcing the aligned copy loop to read out of bounds of the test buffer.

A workaround can be to define a method that will override the weak alias of strncpy, without the aligned copy behavior:

char *strncpy(char * d, const char * s, size_t n)
{
    for (; n && (*d=*s); n--, s++, d++);
    memset(d, 0, n);
    return d;
}

[sbc100]

Does your example repro under asan? if not how have your been detecting it? It would be good to have failing test so we can be sure we've fixed it.

[jeromelaban]

Yes it does with asan enabled, here's how I reproduced it:

emcc \
    test.cpp \
    -fsanitize=address -s TOTAL_MEMORY=393216000 \
    -s DISABLE_EXCEPTION_CATCHING=0 \
    --profiling \
    -o index.html

#include <stdio.h>
#include <emscripten.h>
#include <stdlib.h>
#include <string.h>

int main() {
    char test[] = "12345";

    auto target = (char*)malloc(150);
    strncpy(target, test, 150);
    printf("target: %s\n", target);

    return 0;
}

With the following error:

=================================================================
==42==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x03845eb4 at pc 0x0000106f bp 0x03845e90 sp 0x03845e9c
READ of size 4 at 0x03845eb4 thread T0
    #0 0x106f in __stpncpy+0x106f (http://localhost:8000/index.wasm+0x106f)

Address 0x03845eb4 is located in stack of thread T0 at offset 20 in frame
    #0 0x11 in setTempRet0+0x11 (http://localhost:8000/index.wasm+0x11)

  This frame has 1 object(s):
    [16, 22) 'test' <== Memory access at offset 20 partially overflows this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow (http://localhost:8000/index.wasm+0x106e) in __stpncpy+0x106e
Shadow bytes around the buggy address:
  0x00708b80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x00708b90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x00708ba0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x00708bb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x00708bc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x00708bd0: 00 00 00 00 f1 f1[06]f3 f3 f3 00 00 00 00 00 00
  0x00708be0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x00708bf0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x00708c00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x00708c10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x00708c20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==42==ABORTING

Where would be the best location to add this repro in tests ?

[sbc100]

I think you could add this alongside test_strndup.c in test_core.py.

In order to run that test with asan you would run ./tests/runner.py asan.test_strndup.

[jeromelaban]

https://github.com/emscripten-core/emscripten/pull/12905 should do. It fails when running the test locally.

[kleisauke]

I started working to update musl to version 1.2.1 (i.e. latest version as the time of writing), see:
https://github.com/emscripten-core/emscripten/compare/master...kleisauke:musl-1.2.1

It already passes the test suite locally, so expect a PR soon.

[sbc100]

I also had a go at the musl upgrade last week: https://github.com/emscripten-core/emscripten/pull/13006. Happy to go with your version if they are basically equivalent.