Some patches contains unsafe code

[uazo]

Thanks to the spanification activated by default from 128, this possible security error came to light:

../../third_party/blink/renderer/platform/image-decoders/jxl/jxl_image_decoder.cc(52,17): error: function introduces unsafe buffer manipulation [-Werror,-Wunsafe-buffer-usage]
   52 |   auto buffer = base::span<const uint8_t>(
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~
   53 |       static_cast<const uint8_t*>(buffer_donor.GetProfile()->buffer),
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   54 |       buffer_donor.GetProfile()->size);
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../third_party/blink/renderer/platform/image-decoders/jxl/jxl_image_decoder.cc(52,17): note: See //docs/unsafe_buffers.md for help.
../../third_party/blink/renderer/platform/image-decoders/jxl/jxl_image_decoder.cc(382,19): error: function introduces unsafe buffer manipulation [-Werror,-Wunsafe-buffer-usage]
  382 |                   base::span(icc_profile.data(), icc_profile.size()));
      |                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../third_party/blink/renderer/platform/image-decoders/jxl/jxl_image_decoder.cc(382,19): note: See //docs/unsafe_buffers.md for help.

in v128 I will disable jxl by default pending further investigation.

1362

[uazo]

the enable-jxl flag is present in the cromite flags, for those who wish to disable it in 127 as well.

[uazo]

same for Multiple fingerprinting mitigations patch:

../../third_party/blink/renderer/platform/graphics/static_bitmap_image.cc(206,24): error: unsafe pointer arithmetic [-Werror,-Wunsafe-buffer-usage]
  206 |          auto *pixel = writable_addr(uint16_t, addr, fRowBytes, x, y);
      |                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

[uazo]

it is interesting how the chromium team is dealing with the issue. for example, this code triggers this error:

template<int N>
bool IsInList(const std::u16string& font_name, const char16_t*(&list)[N]) {
  for(int t = 0; t < N; ++t)
    if (base::EqualsCaseInsensitiveASCII(font_name, list[t]))
      return true;
  return false;
}

-------------
    if (base::EqualsCaseInsensitiveASCII(font_name, list[t]))
                                                        ^ error: unsafe buffer access [-Werror,-Wunsafe-buffer-usage]

Technically, there is no security problem, since the template is generated compiler-time on an N always defined compiler-time. yet it is not good for them.

[uazo]

same for this one:

../../third_party/blink/renderer/platform/image-decoders/jxl/jxl_image_decoder.cc(29,3): 
error: unsafe buffer access [-Werror,-Wunsafe-buffer-usage]                                                     
29 |   profile.trc[1].table_entries = 0;   
                  ^~~~~~~~~~~  

trc is fixed at [3] code side.

[uazo]

for the point on multiple fingerprinting mitigations patch the issue is different and more easily understandable, in which case actually the code could lead to an index overflow. however, index checking in SkPixmap is disabled in the non-debug version, I do not understand why some things are allowed and others are not.

[uazo]

Ah, OK, it's not a perfect science:

• https://www.github.com/llvm/llvm-project/issues/64646
• https://www.github.com/llvm/llvm-project/issues/92191

[uazo]

so this is the situation.

the chromium team decided to activate clang's '-Wunsafe-buffer-usage' by default, incrementally on their code. which is why it introduced support for unsafe_buffers_paths.txt in the plugins

it would be interesting to delete that file and check whether there are other parts of the code that I have introduced that need to be checked.