[Webkit-unassigned] [Bug 238134] Validate readpixels format and type inside of WebCore

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=238134

--- Comment #10 from Kenneth Russell <kbr at google.com> ---
(In reply to John Cunningham from comment #9)
> (In reply to Kenneth Russell from comment #8)
> > Comment on attachment 455380 [details]
> > Patch
> > 
> > Just FYI: ANGLE does do this validation. The only validation that should be
> > needed at the WebCore level is to ensure that the incoming typed array type
> > is compatible with the format being requested in readPixels. See Chromium's
> > associated validation:
> > 
> > https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/
> > renderer/modules/webgl/webgl_rendering_context_base.cc;l=4568
> > https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/
> > renderer/modules/webgl/webgl2_rendering_context_base.cc;l=5608
> 
> Thanks, Ken. My original patch here was on ANGLE. It seems there are two
> things going on:
> 
> 1. ANGLE's readpixel validation allows UNSIGNED_INT_24_8, which does not
> seem to be accurate to my spec reading.
> 
> 2. ANGLE's readpixel validation early outs for read-pixels-test.html because
> of a missing attachment which is undefined behavior, see these lines in
> validationES.cpp
> 
> if (readBuffer == nullptr)
> {
>     context->validationError(entryPoint, GL_INVALID_OPERATION,
> kMissingReadAttachment);
>     return false;
> }
> 
> I'm happy to fix the first in upstream ANGLE, and fix the undefined behavior
> in the test which is causing it to fail. I wonder if it still makes sense to
> swap the order of the undefined behavior check of missing attachment to
> after the pixel/type validation in ANGLE.

Thanks for the confirmation; I see.

For (1), it sounds fine to look into fixing this in ANGLE; but the bug is in Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp, WebGLRenderingContextBase::validateArrayBufferType. The case for GraphicsContextGL::UNSIGNED_INT_24_8 should just be removed; then that part of the test will pass.

Not sure about the second bug though. Happy to discuss further.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20220322/8b5e01f2/attachment.htm>