Bug 249633 – WebGLRenderingContextBase::copyTexImage2D does not handle "GCGLint level" parameter

• Status: RESOLVED
• Resolution: CONFIGURATION CHANGED
• Component: WebGL

[byao]

'level' parameter can not be minus value which has not been handled in WebGLRenderingContextBase::validateTexFuncParameters function.
This easily causes WPEWebProcess process crash!

[Kimmo Kinnunen]

Thank you for the report.
Would you have a html+JS reproduction of the problem?
In the trunk code, he intention is that this is handled at ANGLE level, at:
  bool ValidMipLevel(const Context *context, TextureType type, GLint level)

[byao]

Yes, I run below codes:
PASS: Bad target (in assertFail)(function(){ 
  gl.copyTexImage2D(gl.FLOAT, 0, gl.RGBA, 0,0, 16,16,0); 
})
PASS: Bad internal format (in assertFail)(function(){ 
  gl.copyTexImage2D(gl.TEXTURE_2D, 0, gl.FLOAT, 0,0, 16,16,0); 
})
Fail: Negative level (in assertFail)(function(){ 
  gl.copyTexImage2D(gl.TEXTURE_2D, -1, gl.RGBA, 0,0, 16,16,0); 
})
The last one causes the crash.
I don't know ValidMipLevel until you told me actually.

[Kimmo Kinnunen]

If you have a standalone runnable .html file to try, I can try it.
Additionally, if possible a  backtrace of the crash is always helpful.
Also useful information would be to know if the version of WebKit you are using is close to current trunk or older?

[Austin Chen]

Try this page:

https://registry.khronos.org/webgl/conformance-suites/1.0.3/conformance/more/functions/copyTexImage2DBadArgs.html

[Kimmo Kinnunen]

That page passes for current trunk at least on Mac.
We've recently, within last 6 months, removed legacy WebGL backend implementation which might not have the validation. 

In case you are testing an older WPE build, it might be triggering the old code path. A backtrace of the crash would help clarify that.

[byao]

We are really using the older version webkit, but I also checked the latest version webkit it has the same codes which I pasted below.

void WebGLRenderingContextBase::copyTexImage2D(GCGLenum target, GCGLint level, GCGLenum internalFormat, GCGLint x, GCGLint y, GCGLsizei width, GCGLsizei height, GCGLint border)
{
...
// FIXME: if the framebuffer is not complete, none of the below should be executed.
    tex->setLevelInfo(target, level, internalFormat, width, height, GraphicsContextGL::UNSIGNED_BYTE);
...
}
void WebGLTexture::setLevelInfo(GCGLenum target, GCGLint level, GCGLenum internalFormat, GCGLsizei width, GCGLsizei height, GCGLenum type)
{
...
    m_info[index][level].setInfo(internalFormat, width, height, type);
...
}
It possibly uses minus level as the array index then cause the crash.
Hopefully it is helpful to you!

[byao]

I just patched below codes in bool WebGLRenderingContextBase::validateTexFuncParameters

  6          synthesizeGLError(GraphicsContextGL::INVALID_VALUE, functionName, "width or height <     0");
  7          return false;
  8      }
  9 +
 10 +    if (level < 0) {
 11 +        synthesizeGLError(GraphicsContextGL::INVALID_VALUE, functionName, "level < 0");
 12 +        return false;
 13 +    }

[Kimmo Kinnunen]

> I also checked the latest version webkit it has the same codes which I pasted below.
> void WebGLTexture::setLevelInfo(GCGLenum target, GCGLint level, GCGLenum internalFormat, GCGLsizei width, GCGLsizei height, GCGLenum type)

Thanks!
The code has been removed from WebKit around 3 months ago.

https://github.com/WebKit/WebKit/blob/main/Source/WebCore/html/canvas/WebGLTexture.cpp