Bug 119327

Summary: Replace CRASH() on overflow with Checked<>
Product: WebKit Reporter: Kwang Yul Seo <skyul>
Component: WebCore Misc.Assignee: Kwang Yul Seo <skyul>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, eric.carlson, esprehn+autocc, glenn, jer.noble, kangil.han, oliver
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch oliver: review+

Description Kwang Yul Seo 2013-07-31 00:20:10 PDT 
Use a checked type that allows us to automate bound checks. We use a non-recording Checked<> to keep the behavior.
Comment 1 Kwang Yul Seo 2013-07-31 00:34:42 PDT 
Created attachment 207813 [details]
Patch
Comment 2 Oliver Hunt 2013-07-31 14:07:29 PDT 
Comment on attachment 207813 [details]
Patch

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

> Source/WebCore/platform/audio/AudioArray.h:55
>      void allocate(size_t n)

Make n Checked<size_t>, otherwise the multiplication below may silently fail before the arithmetic becomes safe.
Comment 3 Kwang Yul Seo 2013-07-31 16:46:54 PDT 
Created attachment 207886 [details]
Patch
Comment 4 Kwang Yul Seo 2013-07-31 16:47:57 PDT 
(In reply to comment #2)
> Make n Checked<size_t>, otherwise the multiplication below may silently fail before the arithmetic becomes safe.

Done. Thanks for the review.
Comment 5 Kwang Yul Seo 2013-07-31 17:02:02 PDT 
Committed r153548: <http://trac.webkit.org/changeset/153548>
Comment 6 Darin Adler 2013-08-01 12:45:49 PDT 
Comment on attachment 207886 [details]
Patch

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

> Source/WebCore/platform/audio/AudioArray.h:55
> +    void allocate(Checked<size_t> n)

I think it’s not good to make <n> itself a Checked. There must be a better way to make the initialSize computation be checked.