Can someone on the Skia team please help triage this?

Gladly.  The failing assert is __msan_check_mem_is_initialized() at the top of that Skia function.  It's telling us that Chrome's calling into it with uninitialized bytes.  sse41::blit_row_s32a_opaque() works perfectly fine with uninitialized inputs, but we got really tired of Chrome filing bugs on us related to MSAN in this function, so we added that __msan_check_mem_is_initialized() at the top to remind us it's Chrome's problem, not a bug in this function.

Thanks for the explanation.

I think that means this bug should remain open but needs a Blink or cc/ owner.

OK by me.  I'd love to know what the real source of this is!

danakj@: Can you PTAL? This is looking like it might be a security regression from a CL that you landed last August.

The compositor is asking Skia to draw an uninitialized bitmap, and the CF report implies that the resource comes from PaintedScrollbarLayer::RasterizeScrollbarPart().

This CL, in the report's regression range, looks like a candidate:
https://chromium.googlesource.com/chromium/src/+/cbd37e2ef2b8fa6292f5ea8cfb41df409e63cde5

This is also in the regression range, perhaps less likely: https://chromium.googlesource.com/chromium/src/+/8c93919a29947fc27190cdf00bd54de1bff14d84

I think this has been logically present since well before I joined the Skia team a few years ago, but probably only in its present form since we ported this method from assembly to instrumentable intrinsics in January 2015 under its old name, S32A_Opaque_BlitRow32_SSE4.

https://codereview.chromium.org/874863002

The original assembly dates to June 2014, https://codereview.chromium.org/289473009

Before that, we'd be doing the equivalent function (S32A_Opaque_BlitRow32_SSE2) with SSE2 unconditionally, I think, which wouldn't tip off any uninitialized-memory checkers, probably Valgrind at the time?

At least, this isn't a security problem. In the worst case someone would control some pixels in the scrollbar (tho it's not clear what to me since they look ok anyways).

danakj@: It might be true that this doesn't have security implications, but I need a bit of convincing.

You might already know this, but to make sure everybody is on the same page, the primary risk with uninitialized memory usage is that you might have random values influencing the pixels that get displayed, which in some cases could allow an attacker to make inferences about what data was in the uninitialized section. If this includes memory addresses (e.g. raw pointer values) then that is useful for defeating address space layout randomization, which is an important anti-exploit measure.

If an attacker loaded HTML with scrollbars into an SVG <foreignobject> and then drew that to a <canvas>, isn't there a potential for inspecting pixels that might have painted using uninitialized data?

I'm not sure this would be the same path, these are compositor scrollbars which can't go into a canas.

canvas*

That's a good point. It might be okay to lower the priority on this then, if we're confident there is no way to infer pixel values in the bitmap.

I am going to deprioritize then?

Stripping security flags before our bots get cranky about an uninitialized read getting ignored. There's a strong case that this doesn't leak information to web content.

ClusterFuzz has detected this issue as fixed in range 489104:489161.

Detailed report: https://clusterfuzz.com/testcase?key=5817101393854464

Fuzzer: marty_html_twiddler
Job Type: linux_msan_chrome
Platform Id: linux

Crash Type: Use-of-uninitialized-value
Crash Address: 
Crash State:
  sse41::blit_row_s32a_opaque
  SkARGB32_Shader_Blitter::blitRect
  SkScan::FillIRect
  
Sanitizer: memory (MSAN)

Recommended Security Severity: Medium

Regressed: https://clusterfuzz.com/revisions?job=linux_msan_chrome&range=414545:414653
Fixed: https://clusterfuzz.com/revisions?job=linux_msan_chrome&range=489104:489161

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5817101393854464


See https://github.com/google/clusterfuzz-tools for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.

ClusterFuzz testcase 5817101393854464 is verified as fixed, so closing issue as verified.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.