The default bug view has changed. See this FAQ.

firefox 44 crashes due to depending on undefined behavior with memcpy()

RESOLVED DUPLICATE of bug 1224291

Status

()

Core
Networking: HTTP
RESOLVED DUPLICATE of bug 1224291
a year ago
a year ago

People

(Reporter: Natanael Copa, Unassigned)

Tracking

44 Branch
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

a year ago
Created attachment 8713158 [details]
firefox-44.0-brotli-backtrace.txt

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/47.0.2526.106 Safari/537.36

Steps to reproduce:

built firefox 44.0 on x86_64 alpine linux (which uses musl libc) and opened http://streambuilder.pro/docs/how-tos/check-iqsv/


Actual results:

fortify check caught bad use of memcpy() and crashed.

This does not happen with firefox-43.

What happens is that brotli is built without BROTLI_BUILD_PORTABLE by default.

from: modules/brotli/dec/port.h:
#ifdef BROTLI_BUILD_PORTABLE
#define BROTLI_ALIGNED_READ 1
#define BROTLI_SAFE_MEMMOVE 1
#else
#define BROTLI_ALIGNED_READ 0
#define BROTLI_SAFE_MEMMOVE 0
#endif

So we get BROTLI_SAFE_MEMMOVE=0. This will select the following code in 
modules/brotli/dec/decode.c:

static BROTLI_INLINE BROTLI_NO_ASAN void memmove16(
    uint8_t* dst, uint8_t* src) {
#if BROTLI_SAFE_MEMMOVE
  /* For x86 this compiles to the same binary as signle memcpy.
     On ARM memcpy is not inlined, so it works slower.
     This implementation makes decompression 1% slower than regular one,
     and 2% slower than NEON implementation.
   */
  uint32_t buffer[4];
  memcpy(buffer, src, 16);
  memcpy(dst, buffer, 16);
#elif defined(__ARM_NEON__)
  vst1q_u8(dst, vld1q_u8(src));
#else
  /* memcpy is unsafe for overlapping regions and ASAN detects this.
     But, because of optimizations, it works exactly as memmove:
     copies data to registers first, and then stores them to dst. */
  memcpy(dst, src, 16);
#endif
}



and we end up using memcpy() in a way that depends on how implementation deals with overlapping memory regions. FORTIFY_SOURCE on alpine linux did not like this and went boom.




Expected results:

pages should have open without crashing, regardless of how memcpy() is implemented.
(Reporter)

Updated

a year ago
OS: Unspecified → Linux
Hardware: Unspecified → x86_64
(Reporter)

Comment 1

a year ago
I think it would make sense to build brotli with BROTLI_BUILD_PORTABLE by default, and possibly have a whitelist of platforms where you know for sure that it can be disabled.
Moving to Core: Networking and marking this crash blocking the brotli implementation.
Blocks: 366559
Component: Untriaged → Networking: HTTP
Product: Firefox → Core
(Reporter)

Comment 3

a year ago
Credit to musl libc developer Szabolcs Nagy, who helped analyze the disassembly.
This is basically the same report as bug 1224291, except that there it was reported by ASAN.

I guess we should go ahead and take the patch in that bug, to avoid this issue.
Depends on: 1224291

Comment 5

a year ago
Fixed in upstream
Status: UNCONFIRMED → RESOLVED
Last Resolved: a year ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1224291
You need to log in before you can comment on or make changes to this bug.