Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1158)

Side by Side Diff: third_party/WebKit/Source/bindings/core/v8/V8DOMWrapper.h

Issue 2745313003: Move securityCheck out of V8WrapperInstantiationScope (Closed)
Patch Set: Do security check before entering creation context Created 3 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
OLDNEW
1 /* 1 /*
2 * Copyright (C) 2009 Google Inc. All rights reserved. 2 * Copyright (C) 2009 Google Inc. All rights reserved.
3 * 3 *
4 * Redistribution and use in source and binary forms, with or without 4 * Redistribution and use in source and binary forms, with or without
5 * modification, are permitted provided that the following conditions are 5 * modification, are permitted provided that the following conditions are
6 * met: 6 * met:
7 * 7 *
8 * * Redistributions of source code must retain the above copyright 8 * * Redistributions of source code must retain the above copyright
9 * notice, this list of conditions and the following disclaimer. 9 * notice, this list of conditions and the following disclaimer.
10 * * Redistributions in binary form must reproduce the above 10 * * Redistributions in binary form must reproduce the above
(...skipping 13 matching lines...) Expand all
24 * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, 24 * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
25 * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY 25 * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
26 * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT 26 * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
27 * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE 27 * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
28 * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. 28 * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
29 */ 29 */
30 30
31 #ifndef V8DOMWrapper_h 31 #ifndef V8DOMWrapper_h
32 #define V8DOMWrapper_h 32 #define V8DOMWrapper_h
33 33
34 #include "bindings/core/v8/BindingSecurity.h"
35 #include "bindings/core/v8/DOMDataStore.h" 34 #include "bindings/core/v8/DOMDataStore.h"
36 #include "bindings/core/v8/ScriptWrappable.h" 35 #include "bindings/core/v8/ScriptWrappable.h"
37 #include "bindings/core/v8/V8Binding.h" 36 #include "bindings/core/v8/V8Binding.h"
37 #include "bindings/core/v8/WrapperCreationSecurityCheck.h"
38 #include "core/CoreExport.h" 38 #include "core/CoreExport.h"
39 #include "v8/include/v8.h" 39 #include "v8/include/v8.h"
40 #include "wtf/Compiler.h" 40 #include "wtf/Compiler.h"
41 #include "wtf/text/AtomicString.h" 41 #include "wtf/text/AtomicString.h"
42 42
43 namespace blink { 43 namespace blink {
44 44
45 struct WrapperTypeInfo; 45 struct WrapperTypeInfo;
46 46
47 class V8DOMWrapper { 47 class V8DOMWrapper {
(...skipping 70 matching lines...) Expand 10 before | Expand all | Expand 10 after
118 SECURITY_CHECK(toScriptWrappable(wrapper) == impl); 118 SECURITY_CHECK(toScriptWrappable(wrapper) == impl);
119 return wrapper; 119 return wrapper;
120 } 120 }
121 121
122 class V8WrapperInstantiationScope { 122 class V8WrapperInstantiationScope {
123 STACK_ALLOCATED(); 123 STACK_ALLOCATED();
124 124
125 public: 125 public:
126 V8WrapperInstantiationScope(v8::Local<v8::Object> creationContext, 126 V8WrapperInstantiationScope(v8::Local<v8::Object> creationContext,
127 v8::Isolate* isolate, 127 v8::Isolate* isolate,
128 bool withSecurityCheck) 128 const WrapperTypeInfo* type)
129 : m_didEnterContext(false), 129 : m_didEnterContext(false),
130 m_context(isolate->GetCurrentContext()), 130 m_context(isolate->GetCurrentContext()),
131 m_tryCatch(isolate), 131 m_tryCatch(isolate),
132 m_convertExceptions(false) { 132 m_type(type) {
133 // creationContext should not be empty. Because if we have an 133 // creationContext should not be empty. Because if we have an
134 // empty creationContext, we will end up creating 134 // empty creationContext, we will end up creating
135 // a new object in the context currently entered. This is wrong. 135 // a new object in the context currently entered. This is wrong.
136 RELEASE_ASSERT(!creationContext.IsEmpty()); 136 RELEASE_ASSERT(!creationContext.IsEmpty());
137 v8::Local<v8::Context> contextForWrapper = 137 v8::Local<v8::Context> contextForWrapper =
138 creationContext->CreationContext(); 138 creationContext->CreationContext();
139 139
140 // For performance, we enter the context only if the currently running 140 // For performance, we enter the context only if the currently running
141 // context is different from the context that we are about to enter. 141 // context is different from the context that we are about to enter.
142 if (contextForWrapper == m_context) 142 if (contextForWrapper == m_context)
143 return; 143 return;
144 if (withSecurityCheck) { 144 m_context = v8::Local<v8::Context>::New(isolate, contextForWrapper);
Yuki 2017/04/06 08:26:16 Why do we need to call v8::Local::New? I thought t
adithyas 2017/04/06 19:04:20 This is how it was written when V8WrapperInstantia
145 securityCheck(isolate, contextForWrapper); 145
146 } else { 146 if (!WrapperCreationSecurityCheck::verifyContextAccess(isolate, m_context,
147 m_convertExceptions = true; 147 m_type)) {
148 DCHECK(m_tryCatch.HasCaught());
149 m_tryCatch.ReThrow();
150 return;
148 } 151 }
149 m_context = v8::Local<v8::Context>::New(isolate, contextForWrapper); 152
150 m_didEnterContext = true; 153 m_didEnterContext = true;
151 m_context->Enter(); 154 m_context->Enter();
152 } 155 }
153 156
154 ~V8WrapperInstantiationScope() { 157 ~V8WrapperInstantiationScope() {
155 if (!m_didEnterContext) { 158 if (!m_didEnterContext) {
156 m_tryCatch.ReThrow(); 159 m_tryCatch.ReThrow();
157 return; 160 return;
158 } 161 }
159 m_context->Exit(); 162 m_context->Exit();
160 // Rethrow any cross-context exceptions as security error. 163
161 if (m_tryCatch.HasCaught()) { 164 v8::Isolate* isolate = m_context->GetIsolate();
Yuki 2017/04/06 08:26:15 Put this line just before its use, or you may not
adithyas 2017/04/06 19:04:20 Fixed.
162 if (m_convertExceptions) { 165
163 m_tryCatch.Reset(); 166 if (!m_tryCatch.HasCaught())
164 convertException(); 167 return;
165 } 168
166 m_tryCatch.ReThrow(); 169 v8::Local<v8::Value> caughtException = m_tryCatch.Exception();
167 } 170 m_tryCatch.Reset();
171 WrapperCreationSecurityCheck::rethrowCrossContextException(
172 isolate, m_context, m_type, caughtException);
173 m_tryCatch.ReThrow();
168 } 174 }
169 175
170 v8::Local<v8::Context> context() const { return m_context; } 176 v8::Local<v8::Context> context() const { return m_context; }
171 177
172 private: 178 private:
173 void securityCheck(v8::Isolate*, v8::Local<v8::Context> contextForWrapper);
174 void convertException();
175
176 bool m_didEnterContext; 179 bool m_didEnterContext;
177 v8::Local<v8::Context> m_context; 180 v8::Local<v8::Context> m_context;
178 v8::TryCatch m_tryCatch; 181 v8::TryCatch m_tryCatch;
179 bool m_convertExceptions; 182 const WrapperTypeInfo* m_type;
180 }; 183 };
181 184
182 } // namespace blink 185 } // namespace blink
183 186
184 #endif // V8DOMWrapper_h 187 #endif // V8DOMWrapper_h
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698