Bug 108688

Summary: Add ASSERT_WITH_SECURITY_IMPLICATION to detect bad cast in DOM, CSS, etc.
Product: WebKit Reporter: Abhishek Arya <inferno>
Component: DOMAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, alecflett, allan.jensen, apinheiro, cfleizach, cmarcelo, dgrogan, dmazzoni, dpranke, d-r, eric, fmalita, japhet, jdiggs, jsbell, macpherson, menard, mifenton, morrita, ojan.autocc, pdr, rwlbuis, schenney, tonikitoo, webkit.review.bot, yong.li.webkit, yutak
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Abhishek Arya
Reported 2013-02-01 13:56:49 PST
Add ASSERT_WITH_SECURITY_IMPLICATION to detect bad cast in DOM, CSS, etc.
Attachments
Patch (48.07 KB, patch)
2013-02-01 14:04 PST, Abhishek Arya 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Abhishek Arya
Comment 1 2013-02-01 14:04:33 PST
Created attachment 186132 [details] Patch
Build Bot
Comment 2 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
Eric Seidel (no email)
Comment 3 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?
Eric Seidel (no email)
Comment 4 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?
Abhishek Arya
Comment 5 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.
Eric Seidel (no email)
Comment 6 2013-02-04 09:28:29 PST
Wow. 8 real sec bugs found with just a couple asserts... not bad man.
Abhishek Arya
Comment 7 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
Eric Seidel (no email)
Comment 8 2013-02-04 10:27:23 PST
Comment on attachment 186132 [details] Patch LGTM.
Abhishek Arya
Comment 9 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>
Abhishek Arya
Comment 10 2013-02-04 10:45:11 PST
All reviewed patches have been landed. Closing bug.
Hajime Morrita
Comment 11 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
Abhishek Arya
Comment 12 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.
Note You need to log in before you can comment on or make changes to this bug.