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

Unified Diff: Source/bindings/v8/ScriptWrappable.h

Issue 13828008: Second part of moving V8 Binding Integrity off of vtables. (Closed) Base URL: svn://svn.chromium.org/blink/trunk
Patch Set: Fix comments. Created 7 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 side-by-side diff with in-line comments
Download patch
« no previous file with comments | « Source/bindings/v8/DOMDataStore.h ('k') | Source/bindings/v8/WorkerScriptController.cpp » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: Source/bindings/v8/ScriptWrappable.h
diff --git a/Source/bindings/v8/ScriptWrappable.h b/Source/bindings/v8/ScriptWrappable.h
index a5738c455e9f663bc71ffd408c7b97064a38e6df..b8614908efbbd6abaad42b1545e2da55ea244709 100644
--- a/Source/bindings/v8/ScriptWrappable.h
+++ b/Source/bindings/v8/ScriptWrappable.h
@@ -41,72 +41,145 @@ namespace WebCore {
class ScriptWrappable {
friend class WeakHandleListener<ScriptWrappable>;
public:
- ScriptWrappable()
+ ScriptWrappable() : m_maskedStorage(0) { }
+
+ // Wrappables need to be initialized with their most derrived type for which
+ // bindings exist, in much the same way that certain other types need to be
+ // adopted and so forth. The overloaded initializeScriptWrappableForInterface()
+ // functions are implemented by the generated V8 bindings code. Declaring the
+ // extern function in the template avoids making a centralized header of all
+ // the bindings in the universe. C++11's extern template feature may provide
+ // a cleaner solution someday.
+ template <class C> static void init(C* object)
{
-#ifndef NDEBUG
- m_init = false;
-#endif
- }
-
- template <class C> static void init(C *object)
- {
-#ifndef NDEBUG
- object->m_init = true;
-#endif
+ void initializeScriptWrappableForInterface(C*);
+ initializeScriptWrappableForInterface(object);
}
v8::Handle<v8::Object> wrapper() const
{
- return v8::Handle<v8::Object>(maskOrUnmaskPointer(*m_maskedWrapper));
+ v8::Object* object = containsWrapper() ? reinterpret_cast<v8::Object*>(maskOrUnmaskValue(m_maskedStorage)) : 0;
+ return v8::Handle<v8::Object>(object);
}
void setWrapper(v8::Handle<v8::Object> wrapper, v8::Isolate* isolate, const WrapperConfiguration& configuration)
{
- ASSERT(m_maskedWrapper.IsEmpty());
+ ASSERT(!containsWrapper());
+ if (!*wrapper) {
+ m_maskedStorage = 0;
+ return;
+ }
v8::Persistent<v8::Object> persistent = v8::Persistent<v8::Object>::New(isolate, wrapper);
configuration.configureWrapper(persistent, isolate);
WeakHandleListener<ScriptWrappable>::makeWeak(isolate, persistent, this);
- m_maskedWrapper = maskOrUnmaskPointer(*persistent);
+ m_maskedStorage = maskOrUnmaskValue(reinterpret_cast<uintptr_t>(*persistent));
+ ASSERT(containsWrapper());
+ }
+
+ const WrapperTypeInfo* typeInfo()
+ {
+ if (containsTypeInfo())
+ return reinterpret_cast<const WrapperTypeInfo*>(m_maskedStorage);
+
+ if (containsWrapper())
+ return toWrapperTypeInfo(wrapper());
+
+ return 0;
+ }
+
+ void setTypeInfo(const WrapperTypeInfo* info)
+ {
+ m_maskedStorage = reinterpret_cast<uintptr_t>(info);
+ ASSERT(containsTypeInfo());
}
void reportMemoryUsage(MemoryObjectInfo* memoryObjectInfo) const
{
MemoryClassInfo info(memoryObjectInfo, this, WebCoreMemoryTypes::DOM);
- info.ignoreMember(m_maskedWrapper);
+ info.ignoreMember(m_maskedStorage);
+ }
+
+ static bool wrapperCanBeStoredInObject(const void*) { return false; }
+ static bool wrapperCanBeStoredInObject(const ScriptWrappable*) { return true; }
+
+ static v8::Handle<v8::Object> getWrapperFromObject(void*)
+ {
+ ASSERT_NOT_REACHED();
+ return v8::Handle<v8::Object>();
+ }
+
+ static v8::Handle<v8::Object> getWrapperFromObject(ScriptWrappable* object)
+ {
+ return object->wrapper();
+ }
+
+ static void setWrapperInObject(void*, v8::Handle<v8::Object>, v8::Isolate*, const WrapperConfiguration&)
+ {
+ ASSERT_NOT_REACHED();
+ }
+
+ static void setWrapperInObject(ScriptWrappable* object, v8::Handle<v8::Object> wrapper, v8::Isolate* isolate, const WrapperConfiguration& configuration)
+ {
+ object->setWrapper(wrapper, isolate, configuration);
+ }
+
+ static const WrapperTypeInfo* getTypeInfoFromObject(void* object)
+ {
+ ASSERT_NOT_REACHED();
+ return 0;
+ }
+
+ static const WrapperTypeInfo* getTypeInfoFromObject(ScriptWrappable* object)
+ {
+ return object->typeInfo();
+ }
+
+ static void setTypeInfoInObject(void* object, const WrapperTypeInfo* info)
+ {
+ ASSERT_NOT_REACHED();
+ }
+
+ static void setTypeInfoInObject(ScriptWrappable* object, const WrapperTypeInfo* info)
+ {
+ object->setTypeInfo(info);
}
protected:
~ScriptWrappable()
{
-#ifndef NDEBUG
- ASSERT(m_init);
-#endif
+ ASSERT(m_maskedStorage); // Assert initialization via init() even if not subsequently wrapped.
+ m_maskedStorage = 0; // Break UAF attempts to wrap.
}
private:
- inline void disposeWrapper(v8::Persistent<v8::Value> value, v8::Isolate* isolate)
+ inline bool containsWrapper() const { return (m_maskedStorage & 1) == 1; }
+ inline bool containsTypeInfo() const { return m_maskedStorage && ((m_maskedStorage & 1) == 0); }
+
+ static inline uintptr_t maskOrUnmaskValue(uintptr_t value)
{
- ASSERT(!m_maskedWrapper.IsEmpty());
- ASSERT(*value == maskOrUnmaskPointer(*m_maskedWrapper));
- value.Dispose(isolate);
- m_maskedWrapper.Clear();
+ // Entropy via ASLR, bottom bit set to always toggle the bottom bit in the result. Since masking is only
+ // applied to wrappers, not wrapper type infos, and these are aligned poitners with zeros in the bottom
+ // bit(s), this automatically set the wrapper flag in the bottom bit upon encoding. Simiarlry,this
+ // automatically zeros out the bit upon decoding. Additionally, since setWrapper() now performs an explicit
+ // null test, and wrapper() requires the bottom bit to be set, there is no need to preserve null here.
+ const uintptr_t randomMask = ~((reinterpret_cast<uintptr_t>(&WebCoreMemoryTypes::DOM) >> 13)) | 1;
+ return value ^ randomMask;
}
- static inline v8::Object* maskOrUnmaskPointer(const v8::Object* object)
+ inline void disposeWrapper(v8::Persistent<v8::Value> value, v8::Isolate* isolate, const WrapperTypeInfo* info)
{
- const uintptr_t objectPointer = reinterpret_cast<uintptr_t>(object);
- const uintptr_t randomMask = ~(reinterpret_cast<uintptr_t>(&WebCoreMemoryTypes::DOM) >> 13); // Entropy via ASLR.
- return reinterpret_cast<v8::Object*>((objectPointer ^ randomMask) & (!objectPointer - 1)); // Preserve null without branching.
+ ASSERT(containsWrapper());
+ ASSERT(reinterpret_cast<uintptr_t>(*value) == maskOrUnmaskValue(m_maskedStorage));
+ value.Dispose(isolate);
+ setTypeInfo(info);
}
- // Stores a masked wrapper to prevent attackers from overwriting this field
- // with a phony wrapper.
- v8::Persistent<v8::Object> m_maskedWrapper;
-
-#ifndef NDEBUG
- bool m_init;
-#endif
-
+ // If zero, then this contains nothing, otherwise:
+ // If the bottom bit it set, then this contains a masked pointer to a wrapper object in the remainging bits.
+ // If the bottom bit is clear, then this contains a pointer to the wrapper type info in the remaining bits.
+ // Masking wrappers prevents attackers from overwriting this field with pointers to sprayed data.
+ // Pointers to (and inside) WrapperTypeInfo are already protected by ASLR.
+ uintptr_t m_maskedStorage;
};
template<>
@@ -121,7 +194,7 @@ inline void WeakHandleListener<ScriptWrappable>::callback(v8::Isolate* isolate,
WrapperTypeInfo* info = toWrapperTypeInfo(wrapper);
ASSERT(info->derefObjectFunction);
- key->disposeWrapper(value, isolate);
+ key->disposeWrapper(value, isolate, info);
// FIXME: I noticed that 50%~ of minor GC cycle times can be consumed
// inside key->deref(), which causes Node destructions. We should
// make Node destructions incremental.
« no previous file with comments | « Source/bindings/v8/DOMDataStore.h ('k') | Source/bindings/v8/WorkerScriptController.cpp » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698