Makefile: build the vendored Brotli without unaligned typed loads

[iceray00]

Problem

The fuzz targets built by this Makefile compile the Brotli decoder from the
brotli submodule. Brotli's default build reads 16, 32 and 64-bit words by
casting a byte pointer to the target type and dereferencing it, for example in
brotli/c/common/platform.h:

static BROTLI_INLINE uint32_t BrotliUnalignedRead32(const void* p) {
  return *(const uint32_t*)p;
}

The compressed input is byte addressed and the bit reader advances next_in by
an arbitrary number of bytes, so those pointers are frequently misaligned and
the dereference is undefined behavior. Found with UBSan under the upstream
src/convert_woff2ttf_fuzzer.cc harness:

c/dec/../common/platform.h:214:10: runtime error: load of misaligned address 0x630000010469 for type 'const uint32_t' (aka 'const unsigned int'), which requires 4 byte alignment
    #0 BrotliUnalignedRead32 brotli/c/common/platform.h:214:10
    #1 BrotliFillBitWindow brotli/c/dec/bit_reader.h:130:30
    #2 BrotliFillBitWindow16 brotli/c/dec/bit_reader.h:159:3
    #3 ReadSymbolCodeLengths brotli/c/dec/decode.c:607:5
    #4 ReadHuffmanCode brotli/c/dec/decode.c:826:41
    #5 BrotliDecoderDecompressStream brotli/c/dec/decode.c:2210:18
    #6 BrotliDecoderDecompress brotli/c/dec/decode.c:1956:12
    #7 woff2::Woff2Uncompress src/woff2_dec.cc:801:32
    #8 woff2::ConvertWOFF2ToTTF src/woff2_dec.cc:1381:7
    #9 LLVMFuzzerTestOneInput src/convert_woff2ttf_fuzzer.cc:11:3

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior c/dec/../common/platform.h:214:10 in
SUMMARY: libFuzzer: deadly signal

This is not specific to malformed input. With the same build, a small corpus of
ordinary WOFF2 fonts reports the identical read on its first file.

woff2 hands Brotli the raw compressed bytes of the WOFF2 stream, so the
misalignment cannot be avoided from this side; the load itself has to change.

Change

Brotli already ships the implementation this needs. When
BROTLI_BUILD_PORTABLE is defined, the same helpers read through memcpy, and
Brotli's own CMake build defines it whenever ENABLE_SANITIZER is undefined:

By default, brotli depends on undefined behavior, but setting
BROTLI_BUILD_PORTABLE should result in a build which does not.

Pass that define to the dependency from the deps rule, which is the build
that produces the fuzz targets.

Alternatives

• Patching the decoder sources is not possible: they are no longer part of this
  repository. brotli became a submodule, and the helper named in the original
  report (brotli/c/dec/bit_reader.h, BrotliLoad32LE(), from the v1.0.2
  snapshot) does not exist at the pinned revision. The same reads are now
  BrotliUnalignedRead32() and friends in brotli/c/common/platform.h.
• Updating the brotli submodule to a release whose loads no longer use typed
  casts, and keeping the vendored copy in sync, is the right long term fix, but
  it is not sufficient by itself: the submodule is pinned to a 2018 commit, and
  Brotli removed its Makefile build in July 2023 (google/brotli d1fadddc94),
  which is the build this Makefile drives. Renewing the dependency therefore
  means moving woff2 onto Brotli's CMake build first (Update brotli to v1.0.9 #144, feat: update brotli to v1.1.0  #162).

Verification

Built with clang and
-fsanitize=address,undefined -fno-sanitize-recover=all -fno-omit-frame-pointer -O2 -g,
using make for the dependency and the fuzz target, and run through
src/convert_woff2ttf_fuzzer.cc.

• Before: the reproducer and a corpus of ordinary fonts both abort with the
  misaligned load above.
• After: the reproducer and the same corpus complete with no sanitizer
  diagnostics.
• Checked with clang 9.0 and clang 12.0, same result.
• No behavior change: decoding a set of real WOFF2 fonts with the two builds
  produces byte-identical TTF output.

Fixes #202

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[iceray00]

Friendly nudge: this has been open a week with no reviewer. @eustas @rsheeter would either of you have a moment to look?

It is a one-line Makefile flag (-DBROTLI_BUILD_PORTABLE) so the vendored Brotli uses its own memcpy-based loads instead of unaligned typed loads - Brotli's own CMake does the same in sanitizer builds. Happy to move it into the Brotli submodule instead, or to drop it, if you prefer.

[iceray00]

Friendly nudge — and apologies in advance for the direct pings.

This has been open for a week with the CLA check and CI green and no reviewer
assigned, and the repository has had no commits since May, so I am not sure
anyone is seeing it. Since I cannot request a reviewer myself (this is a fork),
pinging is the only lever I have — sorry about that.

@eustas @rsheeter @drott @sharadboni — would one of you have a moment to look at
this?

The change is a one-line addition to the deps rule plus a default for a new
variable, so the vendored Brotli is built with its own memcpy-based loaders
instead of unaligned typed loads:

BROTLI_CPPFLAGS ?= -DBROTLI_BUILD_PORTABLE
...
-	$(MAKE) -C $(BROTLI) lib
+	$(MAKE) -C $(BROTLI) lib CPPFLAGS="$(BROTLI_CPPFLAGS)"

Brotli's own CMake build does the same thing, and ?= means anyone passing
CPPFLAGS still wins. If you would rather have it another way — patching the
submodule, wording it as a separate target, or dropping it entirely — I am happy
to redo it.

Why it seemed worth a PR rather than just a report: with clang and
-fsanitize=undefined the upstream src/convert_woff2ttf_fuzzer.cc harness
aborts on ordinary WOFF2 fonts, not only malformed ones —

c/dec/../common/platform.h:214:10: runtime error: load of misaligned address 0x630000010469 for type 'const uint32_t' (aka 'const unsigned int'), which requires 4 byte alignment
    #0 BrotliUnalignedRead32 brotli/c/common/platform.h:214:10
    #1 BrotliFillBitWindow  brotli/c/dec/bit_reader.h:130:30
    ...
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior c/dec/../common/platform.h:214:10

Reported in #202. Thanks either way.