Bug 178269 - Add RELEASE_ASSERT_WITH_SECURITY_IMPLICATION() macro
Summary: Add RELEASE_ASSERT_WITH_SECURITY_IMPLICATION() macro
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: David Kilzer (:ddkilzer)
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-10-13 10:46 PDT by David Kilzer (:ddkilzer)
Modified: 2017-10-16 12:02 PDT (History)
16 users (show)

See Also:


Attachments
Patch v1 (4.77 KB, patch)
2017-10-13 10:57 PDT, David Kilzer (:ddkilzer)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Kilzer (:ddkilzer) 2017-10-13 10:46:47 PDT
It turns out that some ASSERT_WITH_SECURITY_IMPLICATION() statements would have prevented security issues in WebKit had they been compiled into release builds.*

Toward that end, I'm introducing the RELEASE_ASSERT_WITH_SECURITY_IMPLICATION() macro so that we can transition to release assertions incrementally.  (Doing a wholesale change would incur performance regressions, so we want to be able to change a few at a time.)

I'm also adding a webkit-style-checker (security/assertion) to suggest that new patches stop using ASSERT_WITH_SECURITY_IMPLICATION().

Q: Why not just use RELEASE_ASSERT() instead of adding RELEASE_ASSERT_WITH_SECURITY_IMPLICATION()?

A: Switching from ASSERT_WITH_SECURITY_IMPLICATION() to RELEASE_ASSERT() would lose some important information that the original author thought that this assertion had security implications if it was hit.  When an engineer investigates such crashes, they may be more inclined to consider the security implications of the assertion than if it were a simple RELEASE_ASSERT().  Also, if we later decide that RELEASE_ASSERT_WITH_SECURITY_IMPLICATION() no longer provides useful context, then the Tools/Scripts/do-webcore-rename script can be used to remove it relatively easily in the future.

* Using -DENABLE_SECURITY_ASSERTIONS=1 when compiling WebKit will enable ASSERT_WITH_SECURITY_IMPLICATION() on release builds.
Comment 1 David Kilzer (:ddkilzer) 2017-10-13 10:57:50 PDT
Created attachment 323704 [details]
Patch v1
Comment 2 Radar WebKit Bug Importer 2017-10-13 10:59:28 PDT
<rdar://problem/34981321>
Comment 3 WebKit Commit Bot 2017-10-16 12:02:15 PDT
Comment on attachment 323704 [details]
Patch v1

Clearing flags on attachment: 323704

Committed r223421: <https://trac.webkit.org/changeset/223421>
Comment 4 WebKit Commit Bot 2017-10-16 12:02:17 PDT
All reviewed patches have been landed.  Closing bug.