serenity icon indicating copy to clipboard operation
serenity copied to clipboard

AK+Tests+Everywhere: Fix UTF-8-related crashes

Open BenWiederhake opened this issue 2 years ago • 2 comments

This PR:

  • Fixes silent data corruption due to StringView::contains(char) weirdly accepting (and truncating) u32 code points, which is one cause of the crash https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=49184 .
  • Fixes invalid code points like U+123456 being produced by Utf8CodePointIterator, which is the other cause of the crash https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=49184 .
  • Fixes a slightly-wrong function signature as a drive-by fix.

Due to this, this PR should close the OSS Fuzz issue 49184 for good, and prevent future such issues.

Also, all the calls to StringView::contains(char) are now guaranteed to be correct, because either the type matches exactly, or the compiler complains about ambiguous types (e.g. u16 being passed in, which is why I had to touch LibJS and LibWeb).

BenWiederhake avatar Sep 12 '22 17:09 BenWiederhake

Changes since last push:

  • Changed C-style casts to static_casts (thanks @ADKaster!)

BenWiederhake avatar Sep 12 '22 18:09 BenWiederhake

Changes since last push:

  • Added a comment about not allocating
  • Rebased for convenience

BenWiederhake avatar Sep 13 '22 10:09 BenWiederhake

Changes since last push:

  • Added a dbgln_if statement in case of rejected code point (thanks IdanHo!)
  • Rebased for convenience
  • Re-validated that this still fixes the fuzzer crash

BenWiederhake avatar Sep 18 '22 15:09 BenWiederhake