Changeset 224534 in webkit
- Timestamp:
- Nov 7, 2017 9:41:05 AM (6 years ago)
- Location:
- trunk/Source/WebCore
- Files:
-
- 6 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/Source/WebCore/ChangeLog
r224533 r224534 1 2017-11-07 Ryosuke Niwa <rniwa@webkit.org> 2 3 Release-assert NoEventDispatchAssertion in canExecute, updateLayout, and updateStyle 4 https://bugs.webkit.org/show_bug.cgi?id=179281 5 <rdar://problem/35008993> 6 7 Reviewed by Antti Koivisto. 8 9 Surgically enable NoEventDispatchAssertion::InMainThread::isEventAllowed() in release builds to prevent 10 against insecure execution of author scripts. 11 12 No new tests since there should be no behavioral changes (other than preventing potential security bugs 13 from being exploited). 14 15 * bindings/js/ScriptController.cpp: 16 (WebCore::ScriptController::canExecuteScripts): Use the release assert here. This function is consulted 17 whenever author scripts are executed in event handler, script element, etc... in the main thread so 18 enabling the release assert here should basically prevent all unwanted script executions protected by 19 NoEventDispatchAssertion. 20 * dom/ContainerNode.cpp: 21 (NoEventDispatchAssertion::s_count): Now always compiled. 22 * dom/Document.cpp: 23 (WebCore::Document::updateStyleIfNeeded): Use the release assert here. This assertion would prevent 24 unwanted style updating. This part of the change can be reverted if it turns out to be too crashy since 25 just updating the style would not directly introduce a security vulnerability. 26 (WebCore::Document::updateLayout): Ditto for updating the layout. 27 * dom/NoEventDispatchAssertion.h: 28 (WebCore::NoEventDispatchAssertion::NoEventDispatchAssertion): Enabled this in release builds. 29 (WebCore::NoEventDispatchAssertion::~NoEventDispatchAssertion): Ditto. 30 (WebCore::NoEventDispatchAssertion::isEventAllowedInMainThread): Ditto. 31 (WebCore::NoEventDispatchAssertion::InMainThread::InMainThread): Ditto. 32 (WebCore::NoEventDispatchAssertion::InMainThread::~InMainThread): Ditto. 33 (WebCore::NoEventDispatchAssertion::InMainThread::isEventDispatchAllowedInSubtree): We still don't enable 34 this assertion because this check requires O(n) operation. Added a comment to that end. 35 (WebCore::NoEventDispatchAssertion::InMainThread::isEventAllowed): Enabled this in release builds. 36 (WebCore::NoEventDispatchAssertion::DisableAssertionsInScope): Ditto. 37 * dom/ScriptElement.cpp: 38 (WebCore::ScriptElement::executeClassicScript): Use the release assert here. This is the function used by 39 the HTML parser to run scripts via HTMLScriptRunner::executePendingScriptAndDispatchEvent. Having a release 40 assertion here should prevent the rest of the unwanted script executions in the HTML parser not caught by 41 canExecuteScripts. 42 1 43 2017-11-07 Adrian Perez de Castro <aperez@igalia.com> 2 44 -
trunk/Source/WebCore/bindings/js/ScriptController.cpp
r224356 r224534 679 679 { 680 680 if (reason == AboutToExecuteScript) 681 ASSERT_WITH_SECURITY_IMPLICATION(NoEventDispatchAssertion::InMainThread::isEventAllowed());681 RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(NoEventDispatchAssertion::InMainThread::isEventAllowed()); 682 682 683 683 if (m_frame.document() && m_frame.document()->isSandboxed(SandboxScripts)) { -
trunk/Source/WebCore/dom/ContainerNode.cpp
r224378 r224534 71 71 ChildNodesLazySnapshot* ChildNodesLazySnapshot::latestSnapshot; 72 72 73 unsigned NoEventDispatchAssertion::s_count = 0; 73 74 #if !ASSERT_DISABLED 74 unsigned NoEventDispatchAssertion::s_count = 0;75 75 NoEventDispatchAssertion::EventAllowedScope* NoEventDispatchAssertion::EventAllowedScope::s_currentScope = nullptr; 76 76 #endif -
trunk/Source/WebCore/dom/Document.cpp
r224472 r224534 1940 1940 1941 1941 // The early exit for needsStyleRecalc() is needed when updateWidgetPositions() is called in runOrScheduleAsynchronousTasks(). 1942 ASSERT(NoEventDispatchAssertion::InMainThread::isEventAllowed() || (frameView && frameView->isInChildFrameWithFrameFlattening()));1942 RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(NoEventDispatchAssertion::InMainThread::isEventAllowed() || (frameView && frameView->isInChildFrameWithFrameFlattening())); 1943 1943 1944 1944 resolveStyle(); … … 1957 1957 return; 1958 1958 } 1959 ASSERT(NoEventDispatchAssertion::InMainThread::isEventAllowed() || (frameView && frameView->isInChildFrameWithFrameFlattening()));1959 RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(NoEventDispatchAssertion::InMainThread::isEventAllowed() || (frameView && frameView->isInChildFrameWithFrameFlattening())); 1960 1960 1961 1961 -
trunk/Source/WebCore/dom/NoEventDispatchAssertion.h
r224378 r224534 31 31 class NoEventDispatchAssertion { 32 32 public: 33 // This variant is expensive. Use NoEventDispatchAssertion::InMainThread whenever possible. 33 34 NoEventDispatchAssertion() 34 35 { 35 #if !ASSERT_DISABLED36 36 if (!isMainThread()) 37 37 return; 38 38 ++s_count; 39 #endif40 39 } 41 40 … … 47 46 ~NoEventDispatchAssertion() 48 47 { 49 #if !ASSERT_DISABLED50 48 if (!isMainThread()) 51 49 return; 52 50 ASSERT(s_count); 53 51 s_count--; 54 #endif55 52 } 56 53 57 54 static bool isEventAllowedInMainThread() 58 55 { 59 #if ASSERT_DISABLED60 return true;61 #else62 56 return !isMainThread() || !s_count; 63 #endif64 57 } 65 58 … … 69 62 { 70 63 ASSERT(isMainThread()); 71 #if !ASSERT_DISABLED72 64 ++s_count; 73 #endif74 65 } 75 66 … … 77 68 { 78 69 ASSERT(isMainThread()); 79 #if !ASSERT_DISABLED80 70 ASSERT(s_count); 81 71 --s_count; 82 #endif83 72 } 84 73 74 // Don't enable this assertion in release since it's O(n). 75 // Release asserts in canExecuteScript should be sufficient for security defense purposes. 85 76 static bool isEventDispatchAllowedInSubtree(Node& node) 86 77 { 78 #if !ASSERT_DISABLED || ENABLE(SECURITY_ASSERTIONS) 87 79 return isEventAllowed() || EventAllowedScope::isAllowedNode(node); 80 #else 81 UNUSED_PARAM(node); 82 return true; 83 #endif 88 84 } 89 85 … … 91 87 { 92 88 ASSERT(isMainThread()); 93 #if !ASSERT_DISABLED94 89 return !s_count; 95 #else96 return true;97 #endif98 90 } 99 91 }; … … 138 130 #endif 139 131 140 #if !ASSERT_DISABLED141 132 // FIXME: Remove this class once the sync layout inside SVGImage::draw is removed. 142 133 class DisableAssertionsInScope { … … 155 146 unsigned m_originalCount { 0 }; 156 147 }; 157 #else158 class DisableAssertionsInScope {159 public:160 DisableAssertionsInScope() { }161 };162 #endif163 148 164 #if !ASSERT_DISABLED165 149 private: 166 150 WEBCORE_EXPORT static unsigned s_count; 167 #endif168 151 }; 169 152 -
trunk/Source/WebCore/dom/ScriptElement.cpp
r223628 r224534 362 362 void ScriptElement::executeClassicScript(const ScriptSourceCode& sourceCode) 363 363 { 364 ASSERT_WITH_SECURITY_IMPLICATION(NoEventDispatchAssertion::isEventAllowedInMainThread());364 RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(NoEventDispatchAssertion::InMainThread::isEventAllowed()); 365 365 ASSERT(m_alreadyStarted); 366 366
Note: See TracChangeset
for help on using the changeset viewer.