Bug 108688 - Add ASSERT_WITH_SECURITY_IMPLICATION to detect bad cast in DOM, CSS, etc.
Summary: Add ASSERT_WITH_SECURITY_IMPLICATION to detect bad cast in DOM, CSS, etc.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-01 13:56 PST by Abhishek Arya
Modified: 2013-02-05 08:57 PST (History)
27 users (show)

See Also:

Attachments
Patch (48.07 KB, patch)
2013-02-01 14:04 PST, Abhishek Arya 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Abhishek Arya 2013-02-01 13:56:49 PST 
Add ASSERT_WITH_SECURITY_IMPLICATION to detect bad cast in DOM, CSS, etc.
Comment 1 Abhishek Arya 2013-02-01 14:04:33 PST 
Created attachment 186132 [details]
Patch
Comment 2 Build Bot 2013-02-01 16:36:23 PST 
Comment on attachment 186132 [details]
Patch

Attachment 186132 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/16344187
Comment 3 Eric Seidel (no email) 2013-02-04 09:22:55 PST 
So remind me... these are ASSERTS which are on for builds sent through your fuzzers? but not generally for release buidls?
Comment 4 Eric Seidel (no email) 2013-02-04 09:23:24 PST 
Could you give some context (here, or on the security list) about how well this has worked/hasn't?
Comment 5 Abhishek Arya 2013-02-04 09:25:57 PST 
(In reply to comment #3)
> So remind me... these are ASSERTS which are on for builds sent through your fuzzers? but not generally for release buidls?

Yes, only for the fuzzing builds, they won't impact any production branches. http://trac.webkit.org/changeset/140633/trunk/Source/WTF/wtf/Assertions.h

(In reply to comment #4)
> Could you give some context (here, or on the security list) about how well this has worked/hasn't?

If you open https://bugs.webkit.org/show_bug.cgi?id=107699, you will see the list of bugs it is finding. (in blocks field - 107748 108150 108153 108307 108503 108522 108828 108829). This is just the beginning, since clusterfuzz has been down a lot last week because of migration work.
Comment 6 Eric Seidel (no email) 2013-02-04 09:28:29 PST 
Wow.  8 real sec bugs found with just a couple asserts... not bad man.
Comment 7 Abhishek Arya 2013-02-04 09:29:39 PST 
(In reply to comment #6)
> Wow.  8 real sec bugs found with just a couple asserts... not bad man.

Just filed the 9th :) https://bugs.webkit.org/show_bug.cgi?id=108833
Comment 8 Eric Seidel (no email) 2013-02-04 10:27:23 PST 
Comment on attachment 186132 [details]
Patch

LGTM.
Comment 9 Abhishek Arya 2013-02-04 10:45:07 PST 
Comment on attachment 186132 [details]
Patch

Clearing flags on attachment: 186132

Committed r141783: <http://trac.webkit.org/changeset/141783>
Comment 10 Abhishek Arya 2013-02-04 10:45:11 PST 
All reviewed patches have been landed.  Closing bug.
Comment 11 Hajime Morrita 2013-02-05 01:22:39 PST 
Heeey, please build before land. ews won't help you in this case :-/
http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20ASAN/builds/3122
Comment 12 Abhishek Arya 2013-02-05 08:57:25 PST 
(In reply to comment #11)
> Heeey, please build before land. ews won't help you in this case :-/
> http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20ASAN/builds/3122

Sorry about that. We definitely need an ASAN ews, we rely a lot on this tool these days and can't expect what life would look like without it.