Replace RELEASE_ASSERT_WITH_SECURITY_IMPLICATION with SECURITY_CHECK.

third_party/WebKit/Source/wtf/Assertions.h

 /*
  * Copyright (C) 2003, 2006, 2007 Apple Inc.  All rights reserved.
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
@@ skipping 197 matching lines @@
 
 #define ASSERT(assertion) ((void)0)
 #define DCHECK_AT(assertion, file, line) EAT_STREAM_PARAMETERS
 #define ASSERT_NOT_REACHED() ((void)0)
 #define NO_RETURN_DUE_TO_ASSERT
 
 #define ASSERT_UNUSED(variable, assertion) ((void)variable)
 
 #endif
 
-// ASSERT_WITH_SECURITY_IMPLICATION / RELEASE_ASSERT_WITH_SECURITY_IMPLICATION
-// They are deprecated.  ASSERT_WITH_SECURITY_IMPLICATION should be replaced
-// with SECURITY_DCHECK, and RELEASE_ASSERT_WITH_SECURITY_IMPLICATION should be
-// replaced with RELEASE_ASSERT.
+// ASSERT_WITH_SECURITY_IMPLICATION
+// It is deprecated.  ASSERT_WITH_SECURITY_IMPLICATION should be replaced
+// with SECURITY_DCHECK.

[haraken, 2016/03/31 00:54:58]

SECURITY_DCHECK => SECURITY_CHECK ?

[dcheng, 2016/03/31 01:09:40]

I'm not sure how expensive this will be, all the toDerivedClass macros use
ASSERT_WITH_SECURITY_IMPLICATIONS right now.

[tkent, 2016/03/31 05:26:12]

SECURITY_DCHECK is correct here.

ASSERT -> DCHECK
RELEASE_ASSERT -> CHECK
ASSERT_WITH_S_I -> SECURITY_DCHECK
RELEASE_ASSERT_WITH_S_SI -> SECURITY_CHECK
We'll replace all ASSERT_WITH_S_Is with SECURITY_DCHECKs.

 #ifdef ADDRESS_SANITIZER
 
 #define ASSERT_WITH_SECURITY_IMPLICATION(assertion) \
     (!(assertion) ? \
         (WTFReportAssertionFailure(__FILE__, __LINE__, WTF_PRETTY_FUNCTION, #assertion), \
             CRASH()) : \
         (void)0)
 
-#define RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(assertion) ASSERT_WITH_SECURITY_IMPLICATION(assertion)
-
 #else
-
 #define ASSERT_WITH_SECURITY_IMPLICATION(assertion) ASSERT(assertion)
-#define RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(assertion) RELEASE_ASSERT(assertion)
-
 #endif
 
 // Users must test "#if ENABLE(SECURITY_ASSERT)", which helps ensure
 // that code testing this macro has included this header.
 #if defined(ADDRESS_SANITIZER) || ENABLE(ASSERT)
 #define ENABLE_SECURITY_ASSERT 1
 #else
 #define ENABLE_SECURITY_ASSERT 0
 #endif
 
-// SECURITY_DCHECK
+// SECURITY_DCHECK and SECURITY_CHECK
 // Use in places where failure of the assertion indicates a possible security
 // vulnerability. Classes of these vulnerabilities include bad casts, out of
 // bounds accesses, use-after-frees, etc. Please be sure to file bugs for these
 // failures using the security template:
-//    http://code.google.com/p/chromium/issues/entry?template=Security%20Bug
+//    https://bugs.chromium.org/p/chromium/issues/entry?template=Security%20Bug
 #if ENABLE_SECURITY_ASSERT
 #define SECURITY_DCHECK(condition) LOG_IF(FATAL, !(condition)) << "Security check failed: " #condition ". "
+// TODO(tkent): Should we make SECURITY_CHECK different from SECURITY_DCHECK?
+// A SECURITY_CHECK failure is actually not vulnerable.

[haraken, 2016/03/31 00:54:58]

Maybe failures that are not vulnerable should use CHECK?

BTW, I don't think Blink has distinguished
RELEASE_ASSERT_WITH_SECURITY_IMPLICATION and RELEASE_ASSERT very well. Is it
worth having the difference?

[dcheng, 2016/03/31 01:09:40]

(I think) the security team uses this as a signal of how critical a reported
issue might be.

[inferno, 2016/03/31 04:04:18]

Yes correct, otherwise we won't be able to distinguish these. and hard to
prioritize stuff everything shows as plain checks.

[Oliver Chang, 2016/03/31 06:13:39]

Why do we need to distinguish between CHECK and SECURITY_CHECK here? As tkent
mentioned, SECURITY_CHECK failures don't indicate any exploitable vulnerability,
so there's no reason to prioritise SECURITY_CHECK over CHECK or make this
distinction? The one advantage I see is that it could deter someone from
unknowingly removing a security sensitive CHECK (or turning it into a DCHECK).

Either way, as this is right now it can actually cause confusion for us during
fuzzing since we just see the "Security check failed:" message for both
SECURITY_CHECK and SECURITY_DCHECK, but only the latter represents a real
vulnerability.

+#define SECURITY_CHECK(condition) SECURITY_DCHECK(condition)
 #else
 #define SECURITY_DCHECK(condition) ((void)0)
+#define SECURITY_CHECK(condition) CHECK(condition)
 #endif
 
 // WTF_LOG
 // This is deprecated.  Should be replaced with DVLOG(verboselevel), which works
 // only in debug build, or VLOG(verboselevel), which works in release build too.
 #if LOG_DISABLED
 #define WTF_LOG(channel, ...) ((void)0)
 #else
 #define WTF_LOG(channel, ...) WTFLog(&JOIN_LOG_CHANNEL_WITH_PREFIX(LOG_CHANNEL_PREFIX, channel), __VA_ARGS__)
 #define JOIN_LOG_CHANNEL_WITH_PREFIX(prefix, channel) JOIN_LOG_CHANNEL_WITH_PREFIX_LEVEL_2(prefix, channel)
@@ skipping 55 matching lines @@
 } \
 inline const thisType& to##thisType(const argumentType& argumentName) \
 { \
     ASSERT_WITH_SECURITY_IMPLICATION(referencePredicate); \
     return static_cast<const thisType&>(argumentName); \
 } \
 void to##thisType(const thisType*); \
 void to##thisType(const thisType&)
 
 #endif /* WTF_Assertions_h */