Bug 146652

Summary: Memory corruption in WebGLRenderingContext::simulateVertexAttrib0
Product: WebKit Reporter: Dean Jackson <dino>
Component: New BugsAssignee: Dean Jackson <dino>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, darin, esprehn+autocc, gyuyoung.kim, kondapallykalyan, roger_fong
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch bfulgham: review+

Description Dean Jackson 2015-07-06 15:08:51 PDT
Memory corruption in WebGLRenderingContext::simulateVertexAttrib0
Comment 1 Dean Jackson 2015-07-06 15:11:47 PDT
<rdar://problem/21567767>
Comment 2 Dean Jackson 2015-07-06 15:12:26 PDT
Created attachment 256244 [details]
Patch
Comment 3 Brent Fulgham 2015-07-06 15:33:55 PDT
Comment on attachment 256244 [details]
Patch

r=me
Comment 4 Dean Jackson 2015-07-06 15:36:39 PDT
Committed r186380: <http://trac.webkit.org/changeset/186380>
Comment 5 Darin Adler 2015-07-06 20:04:50 PDT
Comment on attachment 256244 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=256244&action=review

> Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp:4687
> +    Checked<GC3Dsizeiptr, RecordOverflow> bufferDataSize = (numVertex + 1) * 4 * sizeof(GC3Dfloat);

This doesn’t start using checked arithmetic until after doing all the math. Too late!!!
Comment 6 Darin Adler 2015-07-06 20:06:20 PDT
OK, seems like you fixed that in http://trac.webkit.org/changeset/186384
Comment 7 Dean Jackson 2015-07-07 01:26:25 PDT
(In reply to comment #6)
> OK, seems like you fixed that in http://trac.webkit.org/changeset/186384

Yeah. It was my mistake.

I found it really hard to replicate the conditions that lead to this - things failed other verification steps first. So in the end I disabled the other checks and then stepped through in the debugger to exercise the new Checked stuff.