Skip to content

Commit f8cb6cd

Browse files
Poison small JSObject derivatives which only contain pointers
https://bugs.webkit.org/show_bug.cgi?id=181483 <rdar://problem/36407127> Reviewed by Mark Lam. Source/JavaScriptCore: I wrote a script that finds interesting things to poison or generally harden. These stood out because they derive from JSObject and only contain a few pointer or pointer-like fields, and could therefore just be poisoned. This also requires some template "improvements" to our poisoning machinery. Worth noting is that I'm making PoisonedUniquePtr move-assignable and move-constructible from unique_ptr, which makes it a better drop-in replacement because we don't need to use makePoisonedUniquePtr. This means function-locals can be unique_ptr and get the nice RAII pattern, and once the function is done you can just move to the class' PoisonedUniquePtr without worrying. * API/JSAPIWrapperObject.h: (JSC::JSAPIWrapperObject::wrappedObject): * API/JSAPIWrapperObject.mm: (JSC::JSAPIWrapperObject::JSAPIWrapperObject): * API/JSCallbackObject.h: * runtime/ArrayPrototype.h: * runtime/DateInstance.h: * runtime/JSArrayBuffer.cpp: (JSC::JSArrayBuffer::finishCreation): (JSC::JSArrayBuffer::isShared const): (JSC::JSArrayBuffer::sharingMode const): * runtime/JSArrayBuffer.h: * runtime/JSCPoison.h: Source/WTF: The associated JSC poisoning change requires some template "improvements" to our poisoning machinery. Worth noting is that I'm making PoisonedUniquePtr move-assignable and move-constructible from unique_ptr, which makes it a better drop-in replacement because we don't need to use makePoisonedUniquePtr. This means function-locals can be unique_ptr and get the nice RAII pattern, and once the function is done you can just move to the class' PoisonedUniquePtr without worrying. * wtf/Poisoned.h: (WTF::PoisonedImpl::PoisonedImpl): * wtf/PoisonedUniquePtr.h: (WTF::PoisonedUniquePtr::PoisonedUniquePtr): (WTF::PoisonedUniquePtr::operator=): Tools: Test the new move-assign and move-copy from unique_ptr, as well as nullptr_t ctors. * TestWebKitAPI/Tests/WTF/Poisoned.cpp: (TestWebKitAPI::TEST): * TestWebKitAPI/Tests/WTF/PoisonedUniquePtr.cpp: (TestWebKitAPI::TEST): * TestWebKitAPI/Tests/WTF/PoisonedUniquePtrForTriviallyDestructibleArrays.cpp: (TestWebKitAPI::TEST): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@226752 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent bc4d9c9 commit f8cb6cd

16 files changed

+191
-26
lines changed

Source/JavaScriptCore/API/JSAPIWrapperObject.h

+5-3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2013 Apple Inc. All rights reserved.
2+
* Copyright (C) 2013-2018 Apple Inc. All rights reserved.
33
*
44
* Redistribution and use in source and binary forms, with or without
55
* modification, are permitted provided that the following conditions
@@ -27,8 +27,10 @@
2727
#define JSAPIWrapperObject_h
2828

2929
#include "JSBase.h"
30+
#include "JSCPoison.h"
3031
#include "JSDestructibleObject.h"
3132
#include "WeakReferenceHarvester.h"
33+
#include <wtf/Poisoned.h>
3234

3335
#if JSC_OBJC_API_ENABLED
3436

@@ -41,14 +43,14 @@ class JSAPIWrapperObject : public JSDestructibleObject {
4143
void finishCreation(VM&);
4244
static void visitChildren(JSCell*, JSC::SlotVisitor&);
4345

44-
void* wrappedObject() { return m_wrappedObject; }
46+
void* wrappedObject() { return m_wrappedObject.unpoisoned(); }
4547
void setWrappedObject(void*);
4648

4749
protected:
4850
JSAPIWrapperObject(VM&, Structure*);
4951

5052
private:
51-
void* m_wrappedObject;
53+
ConstExprPoisoned<JSAPIWrapperObjectPoison, void*> m_wrappedObject;
5254
};
5355

5456
} // namespace JSC

Source/JavaScriptCore/API/JSAPIWrapperObject.mm

+1-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2013 Apple Inc. All rights reserved.
2+
* Copyright (C) 2013-2018 Apple Inc. All rights reserved.
33
*
44
* Redistribution and use in source and binary forms, with or without
55
* modification, are permitted provided that the following conditions
@@ -80,7 +80,6 @@
8080

8181
JSAPIWrapperObject::JSAPIWrapperObject(VM& vm, Structure* structure)
8282
: Base(vm, structure)
83-
, m_wrappedObject(0)
8483
{
8584
}
8685

Source/JavaScriptCore/API/JSCallbackObject.h

+4-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2006-2016 Apple Inc. All rights reserved.
2+
* Copyright (C) 2006-2018 Apple Inc. All rights reserved.
33
* Copyright (C) 2007 Eric Seidel <[email protected]>
44
*
55
* Redistribution and use in source and binary forms, with or without
@@ -27,10 +27,12 @@
2727
#ifndef JSCallbackObject_h
2828
#define JSCallbackObject_h
2929

30+
#include "JSCPoison.h"
3031
#include "JSCPoisonedPtr.h"
3132
#include "JSObjectRef.h"
3233
#include "JSValueRef.h"
3334
#include "JSObject.h"
35+
#include <wtf/PoisonedUniquePtr.h>
3436

3537
namespace JSC {
3638

@@ -233,7 +235,7 @@ class JSCallbackObject : public Parent {
233235
static EncodedJSValue staticFunctionGetter(ExecState*, EncodedJSValue, PropertyName);
234236
static EncodedJSValue callbackGetter(ExecState*, EncodedJSValue, PropertyName);
235237

236-
std::unique_ptr<JSCallbackObjectData> m_callbackObjectData;
238+
WTF::PoisonedUniquePtr<JSCallbackObjectPoison, JSCallbackObjectData> m_callbackObjectData;
237239
PoisonedClassInfoPtr m_classInfo;
238240
};
239241

Source/JavaScriptCore/ChangeLog

+35
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,38 @@
1+
2018-01-10 JF Bastien <[email protected]>
2+
3+
Poison small JSObject derivatives which only contain pointers
4+
https://bugs.webkit.org/show_bug.cgi?id=181483
5+
<rdar://problem/36407127>
6+
7+
Reviewed by Mark Lam.
8+
9+
I wrote a script that finds interesting things to poison or
10+
generally harden. These stood out because they derive from
11+
JSObject and only contain a few pointer or pointer-like fields,
12+
and could therefore just be poisoned. This also requires some
13+
template "improvements" to our poisoning machinery. Worth noting
14+
is that I'm making PoisonedUniquePtr move-assignable and
15+
move-constructible from unique_ptr, which makes it a better
16+
drop-in replacement because we don't need to use
17+
makePoisonedUniquePtr. This means function-locals can be
18+
unique_ptr and get the nice RAII pattern, and once the function is
19+
done you can just move to the class' PoisonedUniquePtr without
20+
worrying.
21+
22+
* API/JSAPIWrapperObject.h:
23+
(JSC::JSAPIWrapperObject::wrappedObject):
24+
* API/JSAPIWrapperObject.mm:
25+
(JSC::JSAPIWrapperObject::JSAPIWrapperObject):
26+
* API/JSCallbackObject.h:
27+
* runtime/ArrayPrototype.h:
28+
* runtime/DateInstance.h:
29+
* runtime/JSArrayBuffer.cpp:
30+
(JSC::JSArrayBuffer::finishCreation):
31+
(JSC::JSArrayBuffer::isShared const):
32+
(JSC::JSArrayBuffer::sharingMode const):
33+
* runtime/JSArrayBuffer.h:
34+
* runtime/JSCPoison.h:
35+
136
2018-01-10 Commit Queue <[email protected]>
237

338
Unreviewed, rolling out r226667 and r226673.

Source/JavaScriptCore/runtime/ArrayPrototype.h

+5-3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/*
22
* Copyright (C) 1999-2000 Harri Porten ([email protected])
3-
* Copyright (C) 2007, 2011, 2015 Apple Inc. All rights reserved.
3+
* Copyright (C) 2007-2018 Apple Inc. All rights reserved.
44
*
55
* This library is free software; you can redistribute it and/or
66
* modify it under the terms of the GNU Lesser General Public
@@ -21,6 +21,8 @@
2121
#pragma once
2222

2323
#include "JSArray.h"
24+
#include "JSCPoison.h"
25+
#include <wtf/PoisonedUniquePtr.h>
2426

2527
namespace JSC {
2628

@@ -60,8 +62,8 @@ class ArrayPrototype : public JSArray {
6062
private:
6163
// This bit is set if any user modifies the constructor property Array.prototype. This is used to optimize species creation for JSArrays.
6264
friend ArrayPrototypeAdaptiveInferredPropertyWatchpoint;
63-
std::unique_ptr<ArrayPrototypeAdaptiveInferredPropertyWatchpoint> m_constructorWatchpoint;
64-
std::unique_ptr<ArrayPrototypeAdaptiveInferredPropertyWatchpoint> m_constructorSpeciesWatchpoint;
65+
PoisonedUniquePtr<ArrayPrototypePoison, ArrayPrototypeAdaptiveInferredPropertyWatchpoint> m_constructorWatchpoint;
66+
PoisonedUniquePtr<ArrayPrototypePoison, ArrayPrototypeAdaptiveInferredPropertyWatchpoint> m_constructorSpeciesWatchpoint;
6567
};
6668

6769
EncodedJSValue JSC_HOST_CALL arrayProtoFuncToString(ExecState*);

Source/JavaScriptCore/runtime/DateInstance.h

+3-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/*
22
* Copyright (C) 1999-2000 Harri Porten ([email protected])
3-
* Copyright (C) 2008 Apple Inc. All rights reserved.
3+
* Copyright (C) 2008-2018 Apple Inc. All rights reserved.
44
*
55
* This library is free software; you can redistribute it and/or
66
* modify it under the terms of the GNU Lesser General Public
@@ -20,6 +20,7 @@
2020

2121
#pragma once
2222

23+
#include "JSCPoison.h"
2324
#include "JSWrapperObject.h"
2425

2526
namespace JSC {
@@ -76,7 +77,7 @@ class DateInstance : public JSWrapperObject {
7677
JS_EXPORT_PRIVATE const GregorianDateTime* calculateGregorianDateTime(ExecState*) const;
7778
JS_EXPORT_PRIVATE const GregorianDateTime* calculateGregorianDateTimeUTC(ExecState*) const;
7879

79-
mutable RefPtr<DateInstanceData> m_data;
80+
mutable PoisonedRefPtr<DateInstancePoison, DateInstanceData> m_data;
8081
};
8182

8283
DateInstance* asDateInstance(JSValue);

Source/JavaScriptCore/runtime/JSArrayBuffer.cpp

+5-5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2013, 2016 Apple Inc. All rights reserved.
2+
* Copyright (C) 2013-2018 Apple Inc. All rights reserved.
33
*
44
* Redistribution and use in source and binary forms, with or without
55
* modification, are permitted provided that the following conditions
@@ -46,8 +46,8 @@ void JSArrayBuffer::finishCreation(VM& vm, JSGlobalObject* globalObject)
4646
{
4747
Base::finishCreation(vm);
4848
// This probably causes GCs in the various VMs to overcount the impact of the array buffer.
49-
vm.heap.addReference(this, m_impl);
50-
vm.m_typedArrayController->registerWrapper(globalObject, m_impl, this);
49+
vm.heap.addReference(this, impl());
50+
vm.m_typedArrayController->registerWrapper(globalObject, impl(), this);
5151
}
5252

5353
JSArrayBuffer* JSArrayBuffer::create(
@@ -70,12 +70,12 @@ Structure* JSArrayBuffer::createStructure(
7070

7171
bool JSArrayBuffer::isShared() const
7272
{
73-
return m_impl->isShared();
73+
return impl()->isShared();
7474
}
7575

7676
ArrayBufferSharingMode JSArrayBuffer::sharingMode() const
7777
{
78-
return m_impl->sharingMode();
78+
return impl()->sharingMode();
7979
}
8080

8181
size_t JSArrayBuffer::estimatedSize(JSCell* cell)

Source/JavaScriptCore/runtime/JSArrayBuffer.h

+5-3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2013, 2016 Apple Inc. All rights reserved.
2+
* Copyright (C) 2013-2018 Apple Inc. All rights reserved.
33
*
44
* Redistribution and use in source and binary forms, with or without
55
* modification, are permitted provided that the following conditions
@@ -26,7 +26,9 @@
2626
#pragma once
2727

2828
#include "ArrayBuffer.h"
29+
#include "JSCPoison.h"
2930
#include "JSObject.h"
31+
#include <wtf/Poisoned.h>
3032

3133
namespace JSC {
3234

@@ -43,7 +45,7 @@ class JSArrayBuffer final : public JSNonFinalObject {
4345
// This function will register the new wrapper with the vm's TypedArrayController.
4446
JS_EXPORT_PRIVATE static JSArrayBuffer* create(VM&, Structure*, RefPtr<ArrayBuffer>&&);
4547

46-
ArrayBuffer* impl() const { return m_impl; }
48+
ArrayBuffer* impl() const { return m_impl.unpoisoned(); }
4749

4850
static Structure* createStructure(VM&, JSGlobalObject*, JSValue prototype);
4951

@@ -59,7 +61,7 @@ class JSArrayBuffer final : public JSNonFinalObject {
5961
static size_t estimatedSize(JSCell*);
6062

6163
private:
62-
ArrayBuffer* m_impl;
64+
ConstExprPoisoned<JSArrayBufferPoison, ArrayBuffer*> m_impl;
6365
};
6466

6567
inline ArrayBuffer* toPossiblySharedArrayBuffer(VM& vm, JSValue value)

Source/JavaScriptCore/runtime/JSCPoison.h

+5
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,12 @@ enum Poison {
3535
// Add new poison keys below in alphabetical order. The order doesn't really
3636
// matter, but we might as well keep them in alphabetically order for
3737
// greater readability.
38+
ArrayPrototypePoison,
3839
CodeBlockPoison,
40+
DateInstancePoison,
41+
JSAPIWrapperObjectPoison,
42+
JSArrayBufferPoison,
43+
JSCallbackObjectPoison,
3944
JSGlobalObjectPoison,
4045
JSScriptFetchParametersPoison,
4146
JSScriptFetcherPoison,

Source/WTF/ChangeLog

+24
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,27 @@
1+
2018-01-10 JF Bastien <[email protected]>
2+
3+
Poison small JSObject derivatives which only contain pointers
4+
https://bugs.webkit.org/show_bug.cgi?id=181483
5+
<rdar://problem/36407127>
6+
7+
Reviewed by Mark Lam.
8+
9+
The associated JSC poisoning change requires some template
10+
"improvements" to our poisoning machinery. Worth noting is that
11+
I'm making PoisonedUniquePtr move-assignable and
12+
move-constructible from unique_ptr, which makes it a better
13+
drop-in replacement because we don't need to use
14+
makePoisonedUniquePtr. This means function-locals can be
15+
unique_ptr and get the nice RAII pattern, and once the function is
16+
done you can just move to the class' PoisonedUniquePtr without
17+
worrying.
18+
19+
* wtf/Poisoned.h:
20+
(WTF::PoisonedImpl::PoisonedImpl):
21+
* wtf/PoisonedUniquePtr.h:
22+
(WTF::PoisonedUniquePtr::PoisonedUniquePtr):
23+
(WTF::PoisonedUniquePtr::operator=):
24+
125
2018-01-10 Don Olmstead <[email protected]>
226

327
Add nullptr_t specialization of poison

Source/WTF/wtf/Poisoned.h

+5-1
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,11 @@
2525

2626
#pragma once
2727

28-
#include <utility>
2928
#include <wtf/Assertions.h>
3029

30+
#include <cstddef>
31+
#include <utility>
32+
3133
#define ENABLE_POISON 1
3234
#define ENABLE_POISON_ASSERTS 0
3335

@@ -93,6 +95,8 @@ class PoisonedImpl {
9395
public:
9496
PoisonedImpl() { }
9597

98+
PoisonedImpl(std::nullptr_t) { }
99+
96100
PoisonedImpl(T ptr)
97101
: m_poisonedBits(poison(ptr))
98102
{ }

Source/WTF/wtf/PoisonedUniquePtr.h

+35-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2017 Apple Inc. All rights reserved.
2+
* Copyright (C) 2017-2018 Apple Inc. All rights reserved.
33
*
44
* Redistribution and use in source and binary forms, with or without
55
* modification, are permitted provided that the following conditions
@@ -28,6 +28,9 @@
2828
#include <wtf/FastMalloc.h>
2929
#include <wtf/Poisoned.h>
3030

31+
#include <cstddef>
32+
#include <memory>
33+
3134
namespace WTF {
3235

3336
template<uint32_t key, typename T, typename Enable = void>
@@ -36,8 +39,15 @@ class PoisonedUniquePtr : public ConstExprPoisoned<key, T*> {
3639
using Base = ConstExprPoisoned<key, T*>;
3740
public:
3841
PoisonedUniquePtr() = default;
42+
PoisonedUniquePtr(std::nullptr_t) : Base() { }
3943
PoisonedUniquePtr(T* ptr) : Base(ptr) { }
4044
PoisonedUniquePtr(PoisonedUniquePtr&& ptr) : Base(WTFMove(ptr)) { ptr.clearWithoutDestroy(); }
45+
PoisonedUniquePtr(std::unique_ptr<T>&& unique) : PoisonedUniquePtr(unique.release()) { }
46+
47+
template<typename U, typename = std::enable_if_t<std::is_base_of<T, U>::value>>
48+
PoisonedUniquePtr(std::unique_ptr<U>&& unique)
49+
: Base(unique.release())
50+
{ }
4151

4252
template<uint32_t key2, typename U, typename = std::enable_if_t<std::is_base_of<T, U>::value>>
4353
PoisonedUniquePtr(PoisonedUniquePtr<key2, U>&& ptr)
@@ -65,6 +75,21 @@ class PoisonedUniquePtr : public ConstExprPoisoned<key, T*> {
6575
return *this;
6676
}
6777

78+
PoisonedUniquePtr& operator=(std::unique_ptr<T>&& unique)
79+
{
80+
this->clear();
81+
this->Base::operator=(unique.release());
82+
return *this;
83+
}
84+
85+
template<typename U, typename = std::enable_if_t<std::is_base_of<T, U>::value>>
86+
PoisonedUniquePtr& operator=(std::unique_ptr<U>&& unique)
87+
{
88+
this->clear();
89+
this->Base::operator=(unique.release());
90+
return *this;
91+
}
92+
6893
template<uint32_t key2, typename U, typename = std::enable_if_t<std::is_base_of<T, U>::value>>
6994
PoisonedUniquePtr& operator=(PoisonedUniquePtr<key2, U>&& ptr)
7095
{
@@ -113,8 +138,10 @@ class PoisonedUniquePtr<key, T[]> : public ConstExprPoisoned<key, T*> {
113138
using Base = ConstExprPoisoned<key, T*>;
114139
public:
115140
PoisonedUniquePtr() = default;
141+
PoisonedUniquePtr(std::nullptr_t) : Base() { }
116142
PoisonedUniquePtr(T* ptr) : Base(ptr) { }
117143
PoisonedUniquePtr(PoisonedUniquePtr&& ptr) : Base(WTFMove(ptr)) { ptr.clearWithoutDestroy(); }
144+
PoisonedUniquePtr(std::unique_ptr<T[]>&& unique) : PoisonedUniquePtr(unique.release()) { }
118145

119146
template<uint32_t key2>
120147
PoisonedUniquePtr(PoisonedUniquePtr<key2, T[]>&& ptr)
@@ -144,6 +171,13 @@ class PoisonedUniquePtr<key, T[]> : public ConstExprPoisoned<key, T*> {
144171
}
145172
return *this;
146173
}
174+
175+
PoisonedUniquePtr& operator=(std::unique_ptr<T[]>&& unique)
176+
{
177+
this->clear();
178+
this->Base::operator=(unique.release());
179+
return *this;
180+
}
147181

148182
template<uint32_t key2>
149183
PoisonedUniquePtr& operator=(PoisonedUniquePtr<key2, T[]>&& ptr)

0 commit comments

Comments
 (0)