subject:"\[webkit\-dev\] RenderArena\: Teaching an old dog new tricks"

On Wed, Nov 14, 2012 at 11:32 PM, Maciej Stachowiak m...@apple.com wrote:


 On Nov 14, 2012, at 11:09 PM, Chris Evans cev...@chromium.org wrote:

 On Wed, Nov 14, 2012 at 8:59 PM, Ryosuke Niwa rn...@webkit.org wrote:

 On Wed, Nov 14, 2012 at 8:52 PM, Elliott Sprehn espr...@chromium.orgwrote:

 I was present for one of the discussions about the exploit and how an
 arena like allocator could have helped at Google. One proposed solution was
 to allocate all the JS typed buffers in an arena.

 Is there a reason we can't just do that? It's much less intrusive to
 allocate ArrayBuffer in an arena than to allocate all DOM objects in one.


 I don’t think allocating all JS objects in an arena is good enough
 because attackers can inject nearly arbitrary sequence of bytes into DOM
 objects (e.g. text node).


 Yeah, pretty much this. The worry is that it's very hard to be sure you've
 identified all cases / classes where the attacker has reasonable control
 over the size and exact content of the allocation. You have to start
 looking at the buffers backing ArrayBuffers, the buffers backing
 WTF::Vectors, the buffers backing the (multitude of) string classes, and
 even then, you're left worrying about objects that simply have a bunch of
 consecutive ints that the attacker can set as properties, etc. And even in
 the unlikely event you catch everything in WebKit, you still have other
 very attacker-controllable allocations going on in the same process such as
 audio and video packets; canvas buffers; rendered image buffers; the list
 goes on. I don't think it's a battle than can be won.

 So we think the problem is best approached from another observation:

 - Use-after-free of certain classes is either very lethal, or very common,
 or both.

 Use-after-free is pretty common for the RenderObject hierarchy and the
 Node hierarchy. Inferno ran a quick script for use-after-free stats in
 automated ClusterFuzz reports and it was approximately 332 Render and 134
 Node. Due to historical accident, we already have a protection for
 RenderObject.

 The most lethal use-after-frees, though, are DOM objects. A freed DOM
 object is often wired directly into Javascript and the attacker can prod
 all sorts of methods and properties on the freed object in order to cause a
 chosen set of accesses (corresponding to reads, writes and vtable usages
 under the covers) in a chosen order.

 I'm not comfortable sharing it verbatim on a public list, but happy to
 send you a copy of the Pinkie Pie exploit if you're interested. It relies
 on a lethal DOM use-after-free.


 Because use-after-free in the Node hierarchy is both common and lethal, a
 separate allocation area seems a profitable path forward.


 It still seems to me like the key difference is vtable vs no vtable,


It's an important difference, but if we partitioned in to two based on that
difference alone, we'd have the following issues:

1) Classes with multiple vtables would spoil our day. The sheer number of
classes in the vtable partition would practically ensure that an attacker
could overlap data of their choosing on top of a secondary vtable.
Overlap of attacker data onto a secondary vtable is also a concern with the
DOM partition approach, but the chances of it being possible are much much
lower. In the Pinkie Pie exploit case, the DOM arena solution made it
impossible.

2) The Pinkie Pie exploit wasn't exclusively about the vtable.
Before you can abuse an overlap with a freed vtable pointer, you need to
defeat ASLR. This was partly achieved by overlapping the pointer and size
of a freed WTF::Vector member with arbitrary data. (This is highly
dangerous for DOM objects as it can lead to a direct arbitrary memory read
or write primitive from Javascript!) Again, the sheer number of classes in
a vtable partition would make a collision profitable to the attacker a
statistical likelihood. Again, with a strict DOM arena, possibilities are
really shut down. Back to the Pinkie Pie case, there wasn't any immediately
useful overlap in either the 32-bit or 64-bit memory layout.

rather than DOM vs. not DOM. Also having a per-document arena for DOM nodes
 (as is done for render objects via RenderArena) seems irrelevant to the
 security goal and likely to cause bad memory fragmentation.


My read on the Arena is that it's fragmentation resistant (i.e. it will not
repurpose a larger free chunk to satisfy a smaller allocation.) However,
memory usage at any given time is defined by peak usage since it cannot
release pages back to the system without ruining its security guarantee.
Interestingly, it can't be super bad: we already bite this bullet for
RenderArena as used by RenderObjects. The RenderArena lifetime is the same
as the document / DOM and I was surprised to recently be told that we don't
throw away the RenderArena on a full layout.


Cheers
Chris


 Allocating everything with a vtable separately seems like it would achieve
 the security goals, avoid the need to thread arena

Re: [webkit-dev] RenderArena: Teaching an old dog new tricks


On Nov 15, 2012, at 12:02 AM, Ryosuke Niwa rn...@webkit.org wrote:

 On Wed, Nov 14, 2012 at 11:37 PM, Chris Evans cev...@chromium.org wrote:
 On Wed, Nov 14, 2012 at 10:29 PM, Ryosuke Niwa rn...@webkit.org wrote:
 In other words, why are you interested in using the proposed allocation 
 mechanism for only DOM nodes/objects instead of everything in WebCore/WebKit?
 
 The challenge is to add partitioning without going too crazy on number of 
 partitions, whilst maximizing benefit (i.e minimizing the options the 
 attacker has.)
 
 If we stuff every virtual class in WebCore/WebKit together, the attacker has 
 just tons of options for overlap of freed object and attacker-chosen object. 
 The situation is complicated by the presence of multiple vtables in some 
 classes.
 
 Right. That kind of trade off makes sense.
 
 But there’s also a danger of making our codebase too complex. As you’re well 
 aware, increasing code complexity will lead to more bugs including but not 
 limited to security bugs. I’ve been seen many new and even some experienced 
 contributors introducing memory management bugs because they’re not used to 
 either RefCounted objects or RenderArena allocated objects because many 
 people tend to work on either rendering code or DOM code but not both.
 
 If we’re deploying RenderArea for DOM nodes, I’d like to make sure we have a 
 solid API that’s extremely hard to misuse (in addition to making sure 
 adopting a node from one document to another doesn’t result in pathological 
 performance, etc…).

You can achieve all this by having only one global custom allocator for all 
objects inheriting from Node, instead of one per Document based on RenderArena. 
They could stay RefCounted and would just need to overload operator new and 
operator delete at the Node level. There would be no need to pass around arena 
pointers and clients of Node and its subclasses would not even have to be aware 
of the difference.

Still not convince that just targeting Node subclasses is a rational solution.

 
 To come up with the suggestion of partitioning off the Node hierarchy, I 
 actually did a blind analysis against the Pinkie Pie exploit. i.e. I picked 
 the best defense idea we had without fully dissecting the exploit and then 
 ran an analysis of how the defense would have impacted the exploitability. 
 With the defense in place, the attacker's options for this particular case 
 turned out to be severely limited and I couldn't find a good place to start. 
 I even managed to locate and chat to Pinkie Pie who described the proposed 
 defense as quite useful.
 
 Are there other exploits we can get data from? Getting statistics from one 
 exploit results in a strong sampling bias so ideally we can assess whether 
 slab memory allocation is a good defense against other types of exploits.

I tend to agree that if we make a memory allocation architecture change, it 
should be based on sample size  1.

Regards,
Maciej

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] RenderArena: Teaching an old dog new tricks

2012-11-15 Thread Elliott Sprehn

On Thu, Nov 15, 2012 at 3:22 AM, Chris Evans cev...@chromium.org wrote:


 ...


 My read on the Arena is that it's fragmentation resistant (i.e. it will
 not repurpose a larger free chunk to satisfy a smaller allocation.)
 However, memory usage at any given time is defined by peak usage since it
 cannot release pages back to the system without ruining its security
 guarantee. Interestingly, it can't be super bad: we already bite this
 bullet for RenderArena as used by RenderObjects. The RenderArena lifetime
 is the same as the document / DOM and I was surprised to recently be told
 that we don't throw away the RenderArena on a full layout.


My understanding is that the render tree is rather small compared to the
DOM in terms of memory usage on most pages so not releasing it may not be
so bad, but never releasing the memory used from DOM nodes seems like it'd
be bad for large web apps.

- E
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] RenderArena: Teaching an old dog new tricks

2012-11-15 Thread Ryosuke Niwa

On Thu, Nov 15, 2012 at 12:22 AM, Chris Evans cev...@chromium.org wrote:

 On Wed, Nov 14, 2012 at 11:32 PM, Maciej Stachowiak m...@apple.com wrote:

 rather than DOM vs. not DOM. Also having a per-document arena for DOM
 nodes (as is done for render objects via RenderArena) seems irrelevant to
 the security goal and likely to cause bad memory fragmentation.


 My read on the Arena is that it's fragmentation resistant (i.e. it will
 not repurpose a larger free chunk to satisfy a smaller allocation.)
 However, memory usage at any given time is defined by peak usage since it
 cannot release pages back to the system without ruining its security
 guarantee. Interestingly, it can't be super bad: we already bite this
 bullet for RenderArena as used by RenderObjects. The RenderArena lifetime
 is the same as the document / DOM and I was surprised to recently be told
 that we don't throw away the RenderArena on a full layout.


Not really.

Render tree is really small. It's in the order of a few megabytes on most
websites. On the other hand, DOM tree and CSS objects can consume as much
as tens, if not hundreds, of megabytes because there are many DOM nodes
that are not displayed on the screen.

Also, a large proportion of render objects tend to be allocated and
deallocated at the same time while DOM nodes tend to be created and deleted
at different times on many script heavy page.

These two characteristics of render tree makes it particularly attractive
to make use of memory management strategies like the one used in
RenderArena. I'm not convinced that using the same strategy for DOM nodes
is a good idea.

- R. Niwa
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] RenderArena: Teaching an old dog new tricks


On Nov 15, 2012, at 12:22 AM, Chris Evans cev...@chromium.org wrote:

 On Wed, Nov 14, 2012 at 11:32 PM, Maciej Stachowiak m...@apple.com wrote:
 
 On Nov 14, 2012, at 11:09 PM, Chris Evans cev...@chromium.org wrote:
 
 On Wed, Nov 14, 2012 at 8:59 PM, Ryosuke Niwa rn...@webkit.org wrote:
 On Wed, Nov 14, 2012 at 8:52 PM, Elliott Sprehn espr...@chromium.org wrote:
 I was present for one of the discussions about the exploit and how an arena 
 like allocator could have helped at Google. One proposed solution was to 
 allocate all the JS typed buffers in an arena.
 
 Is there a reason we can't just do that? It's much less intrusive to 
 allocate ArrayBuffer in an arena than to allocate all DOM objects in one.
 
 I don’t think allocating all JS objects in an arena is good enough because 
 attackers can inject nearly arbitrary sequence of bytes into DOM objects 
 (e.g. text node).
 
 Yeah, pretty much this. The worry is that it's very hard to be sure you've 
 identified all cases / classes where the attacker has reasonable control 
 over the size and exact content of the allocation. You have to start looking 
 at the buffers backing ArrayBuffers, the buffers backing WTF::Vectors, the 
 buffers backing the (multitude of) string classes, and even then, you're 
 left worrying about objects that simply have a bunch of consecutive ints 
 that the attacker can set as properties, etc. And even in the unlikely event 
 you catch everything in WebKit, you still have other very 
 attacker-controllable allocations going on in the same process such as audio 
 and video packets; canvas buffers; rendered image buffers; the list goes on. 
 I don't think it's a battle than can be won.
 
 So we think the problem is best approached from another observation:
 
 - Use-after-free of certain classes is either very lethal, or very common, 
 or both.
 
 Use-after-free is pretty common for the RenderObject hierarchy and the Node 
 hierarchy. Inferno ran a quick script for use-after-free stats in automated 
 ClusterFuzz reports and it was approximately 332 Render and 134 Node. Due to 
 historical accident, we already have a protection for RenderObject.
 
 The most lethal use-after-frees, though, are DOM objects. A freed DOM object 
 is often wired directly into Javascript and the attacker can prod all sorts 
 of methods and properties on the freed object in order to cause a chosen set 
 of accesses (corresponding to reads, writes and vtable usages under the 
 covers) in a chosen order.
 
 I'm not comfortable sharing it verbatim on a public list, but happy to send 
 you a copy of the Pinkie Pie exploit if you're interested. It relies on a 
 lethal DOM use-after-free.
 
 Because use-after-free in the Node hierarchy is both common and lethal, a 
 separate allocation area seems a profitable path forward.
 
 It still seems to me like the key difference is vtable vs no vtable,
 
 It's an important difference, but if we partitioned in to two based on that 
 difference alone, we'd have the following issues:
 
 1) Classes with multiple vtables would spoil our day. The sheer number of 
 classes in the vtable partition would practically ensure that an attacker 
 could overlap data of their choosing on top of a secondary vtable.
 Overlap of attacker data onto a secondary vtable is also a concern with the 
 DOM partition approach, but the chances of it being possible are much much 
 lower. In the Pinkie Pie exploit case, the DOM arena solution made it 
 impossible.
 
 2) The Pinkie Pie exploit wasn't exclusively about the vtable.
 Before you can abuse an overlap with a freed vtable pointer, you need to 
 defeat ASLR. This was partly achieved by overlapping the pointer and size of 
 a freed WTF::Vector member with arbitrary data. (This is highly dangerous for 
 DOM objects as it can lead to a direct arbitrary memory read or write 
 primitive from Javascript!) Again, the sheer number of classes in a vtable 
 partition would make a collision profitable to the attacker a statistical 
 likelihood. Again, with a strict DOM arena, possibilities are really shut 
 down. Back to the Pinkie Pie case, there wasn't any immediately useful 
 overlap in either the 32-bit or 64-bit memory layout.

I would probably need to know more about it to comment intelligently. Maybe we 
could discuss offline or on the security list.

 
 rather than DOM vs. not DOM. Also having a per-document arena for DOM nodes 
 (as is done for render objects via RenderArena) seems irrelevant to the 
 security goal and likely to cause bad memory fragmentation.
 
 My read on the Arena is that it's fragmentation resistant (i.e. it will not 
 repurpose a larger free chunk to satisfy a smaller allocation.) However, 
 memory usage at any given time is defined by peak usage since it cannot 
 release pages back to the system without ruining its security guarantee.

I'm talking about external fragmentation, not internal fragmentation. 
RenderArena is not resistant to it in any way.

 Interestingly,

Re: [webkit-dev] RenderArena: Teaching an old dog new tricks

On Thu, Nov 15, 2012 at 12:34 AM, Elliott Sprehn espr...@chromium.orgwrote:


 On Thu, Nov 15, 2012 at 3:22 AM, Chris Evans cev...@chromium.org wrote:


 ...


 My read on the Arena is that it's fragmentation resistant (i.e. it will
 not repurpose a larger free chunk to satisfy a smaller allocation.)
 However, memory usage at any given time is defined by peak usage since it
 cannot release pages back to the system without ruining its security
 guarantee. Interestingly, it can't be super bad: we already bite this
 bullet for RenderArena as used by RenderObjects. The RenderArena lifetime
 is the same as the document / DOM and I was surprised to recently be told
 that we don't throw away the RenderArena on a full layout.


 My understanding is that the render tree is rather small compared to the
 DOM in terms of memory usage on most pages


This does not seem to necessarily be the case.

Loading Gmail to the inbox view, sizes in bytes for all live allocations:
- 541k in the DOM arena
- At least a 918k plus a 49k render arena. May have failed to count a few
smaller ones that are still live.

Would you like similar measurements for a simpler page? Or some other
complicated page you think might be instructive?


Cheers
Chris

so not releasing it may not be so bad, but never releasing the memory used
 from DOM nodes seems like it'd be bad for large web apps.

 - E

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] RenderArena: Teaching an old dog new tricks

2012-11-15 Thread Oliver Hunt

Since a common theme people are bringing up is vtable overrides, I do recall 
reading about vtable masking being available in some compilers.  I'm wondering 
if we should push for support for such in compilers we use - I'm not sure what 
the vcall perf hit is in such cases, but it would knock kill off vtable 
overwrites as an attack vector.  We can also trivially add vtable poisoning to 
dealloc if we felt it necessary - i'm not sure here if the problem is 
specifically use-after-free, use-after-free-and-overwritten-with-evil-data, or 
use-after-free-and-overwritten-with-another-object-with-a-different-type.

--Oliver

On Nov 15, 2012, at 2:08 AM, Chris Evans cev...@chromium.org wrote:

 On Thu, Nov 15, 2012 at 12:34 AM, Elliott Sprehn espr...@chromium.org wrote:
 
 On Thu, Nov 15, 2012 at 3:22 AM, Chris Evans cev...@chromium.org wrote:
 
 ...
 
 My read on the Arena is that it's fragmentation resistant (i.e. it will not 
 repurpose a larger free chunk to satisfy a smaller allocation.) However, 
 memory usage at any given time is defined by peak usage since it cannot 
 release pages back to the system without ruining its security guarantee. 
 Interestingly, it can't be super bad: we already bite this bullet for 
 RenderArena as used by RenderObjects. The RenderArena lifetime is the same as 
 the document / DOM and I was surprised to recently be told that we don't 
 throw away the RenderArena on a full layout.
 
 
 My understanding is that the render tree is rather small compared to the DOM 
 in terms of memory usage on most pages
 
 This does not seem to necessarily be the case.
 
 Loading Gmail to the inbox view, sizes in bytes for all live allocations:
 - 541k in the DOM arena
 - At least a 918k plus a 49k render arena. May have failed to count a few 
 smaller ones that are still live.
 
 Would you like similar measurements for a simpler page? Or some other 
 complicated page you think might be instructive?
 
 
 Cheers
 Chris
 
 so not releasing it may not be so bad, but never releasing the memory used 
 from DOM nodes seems like it'd be bad for large web apps.
 
 - E
 
 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] RenderArena: Teaching an old dog new tricks

2012-11-15 Thread Geoffrey Garen

On Nov 14, 2012, at 3:19 PM, Chris Evans cev...@chromium.org wrote:

 A first step might be to make it a platform define. For the Chromium platform 
 we'd leave the define on -- there are some nice security properties we get 
 from having the RenderObjects in their own spot. I'm happy to go in to more 
 details if you want, but it's similar (although not identical) to the blog 
 post linked by Brendan regarding Firefox.
 
 Not all WebKit consumers need weight things the same way as the Chromium port 
 of course, but at least for us, the security win outweighs any quirks of 
 RenderArena.

r-

Don't do this.

I have removed the please to aid clarity.

Geoff
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] RenderArena: Teaching an old dog new tricks

2012-11-15 Thread Geoffrey Garen

On Nov 14, 2012, at 3:27 PM, Ojan Vafai o...@chromium.org wrote:

 As someone outside all these discussions, this seems like a completely unfair 
 characterization of what happened. Sam voiced an objection, then there was a 
 bunch of discussion in which Chris made an argument that Eric found 
 compelling. Many days passed, then Eric r+'ed. Unless there was other 
 discussion not on the bug that I missed, your objection came after Eric's r+. 
 I don't see the process problem here.

I want to call this issue out specifically, because it has come up more than 
once now.

The burden of proof for a patch is on the submitter, not on the reviewer. If a 
reviewer says a patch is a bad idea, it's up to the submitter to convince the 
reviewer otherwise. Saying more things and then letting time pass is not the 
same as convincing the reviewer, and it does not give one automatic rights to 
commit code, just because the reviewer has not kept up with line-by-line 
refutation.

It is an impossible burden for good reviewers to argue, line-by-line, against 
all bad ideas. I tried to do this in just one bug 
(http://webkit.org/b/88834), saying no to O(N^2) algorithms, followed by 
leaky algorithms, followed by use-after-free algorithms, with specific details, 
and the full process took three months. It is simply not practical to assume 
that reviewers can do this for all patches.

During those three months, one of the reasons I kept up with line-by-line 
refutation was that I feared that another reviewer might use the same logic 
you've used here in order to r+ a bad patch. This has to stop.

The process problem here was three-fold:

(1) Chris argued with Sam's objection, but did not resolve it

(2) Eric r+ed a patch with an unresolved objection from a reviewer

(3) Eric announced a new direction for WebKit, after two reviewers had objected 
to it

On Nov 14, 2012, at 3:18 PM, Adam Barth aba...@webkit.org wrote:

 One thing that I notice about these two comments is that they say
 please don't make this change but they don't explain why.  I realize
 that it takes more time and energy to explain the why, but it's
 helpful for folks like me (and I suspect Chris and Cris as well) who
 don't know these issues as well as you.
 
 Thanks for taking the time to explain the issue in your previous
 email.  That helped me understand this issue much better than before.

I'm happy to participate, and say why, when I have time. I don't always have 
time.

Please don't take my participation as consent to the crazy notion that 
line-by-line refutation is required to r- a bad patch.

Geoff

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] RenderArena: Teaching an old dog new tricks

On Thu, Nov 15, 2012 at 11:49 AM, Geoffrey Garen gga...@apple.com wrote:

 On Nov 14, 2012, at 3:19 PM, Chris Evans cev...@chromium.org wrote:

 A first step might be to make it a platform define. For the Chromium
 platform we'd leave the define on -- there are some nice security
 properties we get from having the RenderObjects in their own spot. I'm
 happy to go in to more details if you want, but it's similar (although not
 identical) to the blog post linked by Brendan regarding Firefox.

 Not all WebKit consumers need weight things the same way as the Chromium
 port of course, but at least for us, the security win outweighs any quirks
 of RenderArena.


 r-

 Don't do this.


Ok, no platform define for RenderArena. There's also an implicit r- on
removing the thing, though, as we'd regress security(!!) and performance.
Seems we're stuck with the thing.


Cheers
Chris



 I have removed the please to aid clarity.

 Geoff

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] RenderArena: Teaching an old dog new tricks


On Nov 15, 2012, at 2:16 PM, Chris Evans cev...@chromium.org wrote:

 On Thu, Nov 15, 2012 at 11:49 AM, Geoffrey Garen gga...@apple.com wrote:
 On Nov 14, 2012, at 3:19 PM, Chris Evans cev...@chromium.org wrote:
 
 A first step might be to make it a platform define. For the Chromium 
 platform we'd leave the define on -- there are some nice security 
 properties we get from having the RenderObjects in their own spot. I'm happy 
 to go in to more details if you want, but it's similar (although not 
 identical) to the blog post linked by Brendan regarding Firefox.
 
 Not all WebKit consumers need weight things the same way as the Chromium 
 port of course, but at least for us, the security win outweighs any quirks 
 of RenderArena.
 
 r-
 
 Don't do this.
 
 Ok, no platform define for RenderArena. There's also an implicit r- on 
 removing the thing, though, as we'd regress security(!!) and performance. 
 Seems we're stuck with the thing.

I don't think anyone is asking for immediate removal. At the very least we'd 
need a way to get the same performance - this has also been clear. Your new 
info also highlights the security benefits, and we'd have to address that too. 
Perhaps as we explore ways to improve robustness against use-after-free attacks 
for other, non-render-tree objects, we will find a solution that would be as 
effective as RenderArena even for renderers.

Regards,
Maciej

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] RenderArena: Teaching an old dog new tricks

2012-11-15 Thread Ryosuke Niwa

On Thu, Nov 15, 2012 at 2:16 PM, Chris Evans cev...@chromium.org wrote:

 On Thu, Nov 15, 2012 at 11:49 AM, Geoffrey Garen gga...@apple.com wrote:

 On Nov 14, 2012, at 3:19 PM, Chris Evans cev...@chromium.org wrote:

 A first step might be to make it a platform define. For the Chromium
 platform we'd leave the define on -- there are some nice security
 properties we get from having the RenderObjects in their own spot. I'm
 happy to go in to more details if you want, but it's similar (although not
 identical) to the blog post linked by Brendan regarding Firefox.

 Not all WebKit consumers need weight things the same way as the Chromium
 port of course, but at least for us, the security win outweighs any quirks
 of RenderArena.


 r-

 Don't do this.


 Ok, no platform define for RenderArena. There's also an implicit r- on
 removing the thing, though, as we'd regress security(!!) and performance.
 Seems we're stuck with the thing.


While I don’t want to further agitate the issue or go off on a tangent, and
agree that we must address the security aspect before getting rid of
RenderArena, only WebKit reviewers can r- patches written by other
contributors. You’re not even supposed to set r- on your own patches. See
http://www.webkit.org/coding/commit-review-policy.html

- R. Niwa
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] RenderArena: Teaching an old dog new tricks


I had a few more thoughts on this email besides the fragmentation aspect.

On Nov 15, 2012, at 12:22 AM, Chris Evans cev...@chromium.org wrote:

 
 It still seems to me like the key difference is vtable vs no vtable,
 
 It's an important difference, but if we partitioned in to two based on that 
 difference alone, we'd have the following issues:
 
 1) Classes with multiple vtables would spoil our day. The sheer number of 
 classes in the vtable partition would practically ensure that an attacker 
 could overlap data of their choosing on top of a secondary vtable.
 Overlap of attacker data onto a secondary vtable is also a concern with the 
 DOM partition approach, but the chances of it being possible are much much 
 lower. In the Pinkie Pie exploit case, the DOM arena solution made it 
 impossible.

Are you thinking about classes with multiple inheritence? A couple of thoughts 
on this:

a) I think the majority of uses of multiple inheritence from classes with 
virtual methods (thereby leading to multiple vtables) is in subclasses of Node, 
e.g. in SVG. I don't see how this is much more a problem for broader-scope 
segregated allocation.

b) This could be addressed by allocating classes with multiple vtables 
separately from ones with single vtables, if the cost is worth it. That seems 
to address the issue more directly than making Node subclasses the cutoff.

Your comments on this make me worry that this mitigation might be overly 
tailored to the one specific exploit.

 
 2) The Pinkie Pie exploit wasn't exclusively about the vtable.
 Before you can abuse an overlap with a freed vtable pointer, you need to 
 defeat ASLR. This was partly achieved by overlapping the pointer and size of 
 a freed WTF::Vector member with arbitrary data. (This is highly dangerous for 
 DOM objects as it can lead to a direct arbitrary memory read or write 
 primitive from Javascript!) Again, the sheer number of classes in a vtable 
 partition would make a collision profitable to the attacker a statistical 
 likelihood. Again, with a strict DOM arena, possibilities are really shut 
 down. Back to the Pinkie Pie case, there wasn't any immediately useful 
 overlap in either the 32-bit or 64-bit memory layout.

Is defeating ASLR via this type of information disclosure useful in ways other 
than following up with an attack on a vtable? It seems like a lot of Node 
subclasses


Another thought is that the CSSOM is likely to be pretty vulnerable to these 
kinds of attacks too. So a DOM-specific solution may be too narrow, but at the 
same time, adding more and more different arenas is likely to cause bad 
fragmentation in pathological cases. This is why I think doing the segregation 
at a coarser granularity is a promosing approach.

Cheers,
Maciej


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] RenderArena: Teaching an old dog new tricks


On Nov 15, 2012, at 2:59 PM, Ryosuke Niwa rn...@webkit.org wrote:

 On Thu, Nov 15, 2012 at 2:16 PM, Chris Evans cev...@chromium.org wrote:
 On Thu, Nov 15, 2012 at 11:49 AM, Geoffrey Garen gga...@apple.com wrote:
 On Nov 14, 2012, at 3:19 PM, Chris Evans cev...@chromium.org wrote:
 
 A first step might be to make it a platform define. For the Chromium 
 platform we'd leave the define on -- there are some nice security 
 properties we get from having the RenderObjects in their own spot. I'm happy 
 to go in to more details if you want, but it's similar (although not 
 identical) to the blog post linked by Brendan regarding Firefox.
 
 Not all WebKit consumers need weight things the same way as the Chromium 
 port of course, but at least for us, the security win outweighs any quirks 
 of RenderArena.
 
 r-
 
 Don't do this.
 
 Ok, no platform define for RenderArena. There's also an implicit r- on 
 removing the thing, though, as we'd regress security(!!) and performance. 
 Seems we're stuck with the thing.
 
 While I don’t want to further agitate the issue or go off on a tangent, and 
 agree that we must address the security aspect before getting rid of 
 RenderArena, only WebKit reviewers can r- patches written by other 
 contributors. You’re not even supposed to set r- on your own patches. See 
 http://www.webkit.org/coding/commit-review-policy.html

I think Chris merely meant that removing RenderArena without fixing those 
problems would appear to violate the project goals, and therefore a responsible 
reviewer would likely r- it. I don't think he intended to literally r- a patch.

Cheers,
Maciej


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] RenderArena: Teaching an old dog new tricks

On Thu, Nov 15, 2012 at 3:05 PM, Maciej Stachowiak m...@apple.com wrote:


 I had a few more thoughts on this email besides the fragmentation aspect.

 On Nov 15, 2012, at 12:22 AM, Chris Evans cev...@chromium.org wrote:


 It still seems to me like the key difference is vtable vs no vtable,


 It's an important difference, but if we partitioned in to two based on
 that difference alone, we'd have the following issues:

 1) Classes with multiple vtables would spoil our day. The sheer number of
 classes in the vtable partition would practically ensure that an attacker
 could overlap data of their choosing on top of a secondary vtable.
 Overlap of attacker data onto a secondary vtable is also a concern with
 the DOM partition approach, but the chances of it being possible are much
 much lower. In the Pinkie Pie exploit case, the DOM arena solution made it
 impossible.


 Are you thinking about classes with multiple inheritence? A couple of
 thoughts on this:

 a) I think the majority of uses of multiple inheritence from classes with
 virtual methods (thereby leading to multiple vtables) is in subclasses of
 Node, e.g. in SVG. I don't see how this is much more a problem for
 broader-scope segregated allocation.

 b) This could be addressed by allocating classes with multiple vtables
 separately from ones with single vtables, if the cost is worth it. That
 seems to address the issue more directly than making Node subclasses the
 cutoff.


One reason to make Node the cutoff is that UAF of DOM objects is often more
dangerous because of the rich JS API the attacker can play with to take the
effect of the memory corruption in so many different ways. So the
non-vtable aspects of the exploitation get defended a little better due to
fewer attacker opportunities.

That said, I like the elegance of your single vtable suggestion. Let me
think about it.



 Your comments on this make me worry that this mitigation might be overly
 tailored to the one specific exploit.


It's a fair comment; and I did see your request for other examples in a
previous mail. Actual UAF exploits are hard to come by because many people
who write them are not incentivized to ever make them public. And the great
example I do have cost $60k ;-)

I'll work on getting more. Or, send me a bug which leaves a JS handle
pointing to a freed underlying DOM node and I could see about weaponizing
it.

There are bunch of historical examples such as
https://code.google.com/p/chromium/issues/detail?id=118273 which show
attackers are going after freed DOM node pointers, though. We only have the
bug and not an exploit.



 2) The Pinkie Pie exploit wasn't exclusively about the vtable.
 Before you can abuse an overlap with a freed vtable pointer, you need to
 defeat ASLR. This was partly achieved by overlapping the pointer and size
 of a freed WTF::Vector member with arbitrary data. (This is highly
 dangerous for DOM objects as it can lead to a direct arbitrary memory read
 or write primitive from Javascript!) Again, the sheer number of classes in
 a vtable partition would make a collision profitable to the attacker a
 statistical likelihood. Again, with a strict DOM arena, possibilities are
 really shut down. Back to the Pinkie Pie case, there wasn't any immediately
 useful overlap in either the 32-bit or 64-bit memory layout.


 Is defeating ASLR via this type of information disclosure useful in ways
 other than following up with an attack on a vtable?


Yeah. Once you have a valid address, you can use that (e.g. set the
appropriate ArrayBuffer bytes at the right index) to be the basis for e.g.
a WTF::Vector. And set the length to 0x. Now, assuming there's a JS
API to make queries that are backed by the Vector, you get to read and
possibly write arbitrary memory.

If you can read arbitrary memory, you have a UXSS-class of vulnerability in
most setups.

If you can read or write arbitrary memory, you can target vtables, function
pointers, stacks... bad news at this stage.

The DOM arena approach does lead to less liklihood of being able to cause
useful overlap to do any of the above steps. If you like, you could imagine
the probability of useful attack for each step as being some function of
the number of different classes in the same partition. The vision behind
the DOM arena is to take the most dangerous UAFs and put them in a small
partition.


Cheers
Chris

It seems like a lot of Node subclasses


 Another thought is that the CSSOM is likely to be pretty vulnerable to
 these kinds of attacks too. So a DOM-specific solution may be too narrow,
 but at the same time, adding more and more different arenas is likely to
 cause bad fragmentation in pathological cases. This is why I think doing
 the segregation at a coarser granularity is a promosing approach.

 Cheers,
 Maciej



___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] RenderArena: Teaching an old dog new tricks

On Wed, Nov 14, 2012 at 3:27 PM, Ojan Vafai o...@chromium.org wrote:

On Wed, Nov 14, 2012 at 2:48 PM, Geoffrey Garen gga...@apple.com wrote:

Hi Eric.

Here are some problems in RenderArena that I know of:

- Grows memory without bound
- Duplicates the functionality of FastMalloc
- Makes allocation error-prone (allocation in the wrong arena is
sometimes a leak, sometimes a use-after-free, and sometimes a heizenbug of
the two)
- Makes allocation verbose (you have to thread an arena pointer
everywhere)
- Makes object lifetime complicated (all objects are implicitly tied to a
single owner they may outlive)
- Uses C-style macros and manual initialization and destruction, instead
of modern WebKit C++ style

You didn't mention any of these problems in your email, so I'll assume
you weren't aware of them.

Considering these problems now, please don't use RenderArena in more
places.

Slab-allocators (i.e. RenderArena) hand out memory all from a single
region, guaranteeing (among other things) that free'd objects can only
be ever overwritten by other objects from the same pool. This makes
it much harder, for example to find a Use-After-Free of a RenderBlock
and then fill that RenderBlock's memory (and vtable pointer) with
arbitrary memory (like the contents of a javascript array).
http://en.wikipedia.org/wiki/Slab_allocation

This is magical thinking. RenderArena is no different from FastMalloc.

(1) RenderArena recycles by object size, just like FastMalloc.

(2) FastMalloc is a slab allocator, just like RenderArena.

(3) RenderArena grows by calling FastMalloc.

Isolating object types from each other -- and specifically isolating
objects of arbitrary size and contents like arrays -- is an interesting
idea. RenderArena is neither necessary nor sufficient for implementing this
feature.

The only reason RenderArena seems isolated from other object types is
social, not technical: we actively discourage using RenderArena, so few
object types currently use it.

Since RenderArena is generic, the current plan to move it to WTF (as
by Chris Marrin suggested back in
http://www.mail-archive.com/webkit-dev@lists.webkit.org/msg12672.html),
clean up the code further, and investigate wider deployment (like to
the DOM tree) for the security benefit and possible perf win.
https://bugs.webkit.org/show_bug.cgi?id=101087

Having dealt with the specific technical question of RenderArena, I'd
like to briefly discuss the meta-level of how the WebKit project works.

Sam Weinig and I both provided review feedback saying that using
RenderArena more was a bad idea
(https://bugs.webkit.org/show_bug.cgi?idhttps://bugs.webkit.org/show_bug.cgi?id=101087#c9

*This seems completely unfair *

*
*

*Geoff*

*
*

*___*

*webkit-dev mailing list*

*webkit-dev@lists.webkit.org*

*http://lists.webkit.org/mailman/listinfo/webkit-dev*

=101087#c9 https://bugs.webkit.org/show_bug.cgi?id=101087#c9,
https://bugs.webkit.org/show_bug.cgi?id=101087#c18). Nonetheless, you
r+ed a patch to move in that direction, and you describe it here as the
current plan for WebKit.

I'm a little disappointed that, as individual contributors, Chris Neklar
and Chris Evans didn't realize or understand the problems listed above, and
didn't tackle them. However, the mistake is understandable: Chris and Chris
are new to WebKit. The WebKit project has a mechanism for resolving
mistakes like this: patch review.

Your job as a reviewer is to understand the zeitgeist of the project, to
use good judgement, and to r- patches that make mistakes like this. A bad
patch is only a small nuisance. But the small nuisance turns into a major
problem when you, as a reviewer, take a bad patch, mark it r+, and declare
it the current direction of the project, despite the objections of two
other reviewers who are senior members of the project.

As someone outside all these discussions, this seems like a completely
unfair characterization of what happened. Sam voiced an objection, then
there was a bunch of discussion in which Chris made an argument that Eric
found compelling. Many days passed, then Eric r+'ed. Unless there was other
discussion not on the bug that I missed, your objection came after Eric's
r+. I don't see the process problem here.

I think there's still a massive misunderstanding here. The bug in question,
https://bugs.webkit.org/show_bug.cgi?id=101087, is simply renaming / moving
a generic class into a more generic location. There's nothing render
specific about RenderArena.

Moving RenderArena to a more generic location, based on the observation
that it isn't render specific, has been floated as early as 2010 by Chris
Marrin:
http://mac-os-forge.2317878.n4.nabble.com/Arena-is-crufty-td178154.html

Whether or not we use RenderArena for additional things, or even remove it,
seems independent of the clean-up covered by

Re: [webkit-dev] RenderArena: Teaching an old dog new tricks

On Wed, Nov 14, 2012 at 3:19 PM, Chris Evans cev...@chromium.org wrote:

 On Tue, Nov 13, 2012 at 11:14 PM, Ryosuke Niwa rn...@webkit.org wrote:

 On Tue, Nov 13, 2012 at 10:23 PM, Eric Seidel e...@webkit.org wrote:

 RenderArena was a perf optimization for the rendering tree, which
 hyatt imported from Mozilla 10 years ago:
 http://trac.webkit.org/changeset/2635

 The prevailing lore has long been that RenderArena is no longer
 useful, ugly, and should be killed!
 http://www.mail-archive.com/webkit-dev@lists.webkit.org/msg12681.html

 The (unfortunate?) reality is that we've failed to do so, despite
 trying several times.
 http://www.mail-archive.com/webkit-dev@lists.webkit.org/msg12682.html


 I don't think we have failed. The messages posted on the thread don't
 indicate anyone has tried to delete the render arena recently. I support
 any attempts to remove it.


 A first step might be to make it a platform define. For the Chromium
 platform we'd leave the define on -- there are some nice security
 properties we get from having the RenderObjects in their own spot. I'm
 happy to go in to more details if you want, but it's similar (although not
 identical) to the blog post linked by Brendan regarding Firefox.


If we like the properties of RenderArena, then why don’t we modify
FastMalloc to have the same property, possibly behind a build flag? We can
then make all render objects to be ref-counted and get rid of RenderArena.
That’ll give us two layers of defense (for all objects; DOM, CSS, etc...)
by making it 1. hard to free prematurely and 2. hard to override vtable
pointers.

Having two independent ways of allocating an arbitrary object imposes a
significant maintenance burden on the project much like having multiple
build systems.

- R. Niwa
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] RenderArena: Teaching an old dog new tricks

On Wed, Nov 14, 2012 at 2:48 PM, Geoffrey Garen gga...@apple.com wrote:

 Hi Eric.

 Here are some problems in RenderArena that I know of:

 - Grows memory without bound
 - Duplicates the functionality of FastMalloc
 - Makes allocation error-prone (allocation in the wrong arena is sometimes
 a leak, sometimes a use-after-free, and sometimes a heizenbug of the two)
 - Makes allocation verbose (you have to thread an arena pointer everywhere)
 - Makes object lifetime complicated (all objects are implicitly tied to a
 single owner they may outlive)
 - Uses C-style macros and manual initialization and destruction, instead
 of modern WebKit C++ style

 You didn't mention any of these problems in your email, so I'll assume you
 weren't aware of them.

 Considering these problems now, please don't use RenderArena in more
 places.

  Slab-allocators (i.e. RenderArena) hand out memory all from a single
  region, guaranteeing (among other things) that free'd objects can only
  be ever overwritten by other objects from the same pool.  This makes
  it much harder, for example to find a Use-After-Free of a RenderBlock
  and then fill that RenderBlock's memory (and vtable pointer) with
  arbitrary memory (like the contents of a javascript array).
  http://en.wikipedia.org/wiki/Slab_allocation

 This is magical thinking. RenderArena is no different from FastMalloc.


It's actually different enough to be interesting:
- The bucket granularity is different, which affects exploitation
significantly.
- The packing is perfect, which might even lead to better memory usage
under some patterns, although, yes, it's true that there are possible
pathological conditions as noted by Maciej.
- free() is more expensive in fastMalloc because of various indirections to
work out if we're freeing a page range or a slab slot.
- RenderArena has freelist poisoning. I'm not sure if freelist poisoning
made it into upstream tcmalloc yet.
- There are very significant differences (from the attackers, and WebKit
consumer's point of view) on page reclaim.


 (1) RenderArena recycles by object size, just like FastMalloc.

 (2) FastMalloc is a slab allocator, just like RenderArena.

 (3) RenderArena grows by calling FastMalloc.

 Isolating object types from each other -- and specifically isolating
 objects of arbitrary size and contents like arrays -- is an interesting
 idea. RenderArena is neither necessary nor sufficient for implementing this
 feature.

 The only reason RenderArena seems isolated from other object types is
 social, not technical: we actively discourage using RenderArena, so few
 object types currently use it.

  Since RenderArena is generic, the current plan to move it to WTF (as
  by Chris Marrin suggested back in
  http://www.mail-archive.com/webkit-dev@lists.webkit.org/msg12672.html),
  clean up the code further, and investigate wider deployment (like to
  the DOM tree) for the security benefit and possible perf win.
  https://bugs.webkit.org/show_bug.cgi?id=101087

 Having dealt with the specific technical question of RenderArena, I'd like
 to briefly discuss the meta-level of how the WebKit project works.

 Sam Weinig and I both provided review feedback saying that using
 RenderArena more was a bad idea (
 https://bugs.webkit.org/show_bug.cgi?id=101087#c9,
 https://bugs.webkit.org/show_bug.cgi?id=101087#c18). Nonetheless, you
 r+ed a patch to move in that direction, and you describe it here as the
 current plan for WebKit.

 I'm a little disappointed that, as individual contributors, Chris Neklar
 and Chris Evans didn't realize or understand the problems listed above, and
 didn't tackle them.


We realize RenderArena tradeoffs and have not tackled them in more detail
because https://bugs.webkit.org/show_bug.cgi?id=101087 is not the place to
do it. https://bugs.webkit.org/show_bug.cgi?id=101087 is a simple clean-up
changeset. Eric started this thread in order to start discussing some of
the trade-offs with a DOM slab -- something for which we haven't yet
uploaded any patches for anyone to r+ or r- :-)


Cheers
Chris


 However, the mistake is understandable: Chris and Chris are new to WebKit.
 The WebKit project has a mechanism for resolving mistakes like this: patch
 review.

 Your job as a reviewer is to understand the zeitgeist of the project, to
 use good judgement, and to r- patches that make mistakes like this. A bad
 patch is only a small nuisance. But the small nuisance turns into a major
 problem when you, as a reviewer, take a bad patch, mark it r+, and declare
 it the current direction of the project, despite the objections of two
 other reviewers who are senior members of the project.

 Geoff

 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] RenderArena: Teaching an old dog new tricks

On Wed, Nov 14, 2012 at 3:27 PM, Ojan Vafai o...@chromium.org wrote:

 On Wed, Nov 14, 2012 at 2:48 PM, Geoffrey Garen gga...@apple.com wrote:

 Having dealt with the specific technical question of RenderArena, I'd
 like to briefly discuss the meta-level of how the WebKit project works.

 Sam Weinig and I both provided review feedback saying that using
 RenderArena more was a bad idea (
 https://bugs.webkit.org/show_bug.cgi?id=101087#c9,
 https://bugs.webkit.org/show_bug.cgi?id=101087#c18). Nonetheless, you
 r+ed a patch to move in that direction, and you describe it here as the
 current plan for WebKit.

 I'm a little disappointed that, as individual contributors, Chris Neklar
 and Chris Evans didn't realize or understand the problems listed above, and
 didn't tackle them. However, the mistake is understandable: Chris and Chris
 are new to WebKit. The WebKit project has a mechanism for resolving
 mistakes like this: patch review.


 Your job as a reviewer is to understand the zeitgeist of the project, to
 use good judgement, and to r- patches that make mistakes like this. A bad
 patch is only a small nuisance. But the small nuisance turns into a major
 problem when you, as a reviewer, take a bad patch, mark it r+, and declare
 it the current direction of the project, despite the objections of two
 other reviewers who are senior members of the project.


 As someone outside all these discussions, this seems like a completely
 unfair characterization of what happened. Sam voiced an objection, then
 there was a bunch of discussion in which Chris made an argument that Eric
 found compelling. Many days passed, then Eric r+'ed. Unless there was other
 discussion not on the bug that I missed, your objection came after Eric's
 r+. I don't see the process problem here.


I agree that Eric’s r+ seems reasonable after reading the comments on the
bug. I think the miscommunication comes from how people interpreted Sam’s
objection on the bug:

 I would rather not generalize this.  Our long term plan is to stop using
 the RenderArena, so changes to use it in more places is probably not a
 great idea.

 If you have specific other uses where this type of allocator would be
 useful that would be interesting, but we should have those use cases
 established before moving forward.


One might consider this comment as simply asking to define the use case, in
which case Eric’s following comment adequately addressed the concern:

 cevans would like to use it for allocating DOM Nodes.  He has a prototype
 patch.  I agree we should probably wait to see how that bears fruit before
 generalizing.


Others might take it as objecting to the change until we can establish that
the use case behind the change is overall benefit to the project, in which
case, nobody has provided enough information anywhere thus far.

Now, it is my understanding that many members of the WebKit community
strongly felt the need to get rid of RenderArena eventually for some time.
Thus I was surprised when I learned that Chris (and possibly other members
of the community) wants to keep RenderArena. I would have preferred that
the change of this nature to be discussed on webkit-dev before the patch is
posted. e.g. Geoff was able to give us quite a few reasons we might not
want to proceed with the proposed generalization of RenderArena.

With respect to the specific patch being posted to the bug, while I agree
that the patch doesn’t deploy RenderArena in more places, the fact
RenderArena implementation isn’t specific to render objects doesn’t
necessarily mean we need to move it to WTF. If a class is only used in some
component of WebKit and we don’t intend to use it elsewhere, then it’s not
necessarily a bad idea to confine the visibility of the class in that
component so as not to pollute the WTF namespace.

Also, was there a reason why this change had to be made on trunk before
gathering more data? It seems like Chris already generalized RenderArena
locally and got some pref. data. Why can't we proceed with gathering more
data to reach a consensus as the community before landing patches given
that many of members of the community feel strongly that we should get rid
of RenderArena?

The way I see it, adding a build flag for this sort of things should be our
last report when we can’t reach a consensus. Adding a new build flag always
incurs a maintenance cost and forces us to know more. Adding a build flag
to change memory allocations is particularly because that will
fundamentally change the nature of WebKit’s performance and security model.
e.g. how do we decide whether to take a performance optimization when the
optimization favors one memory management model over the other?

- R. Niwa
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] RenderArena: Teaching an old dog new tricks

On Wed, Nov 14, 2012 at 3:46 PM, Chris Evans cev...@chromium.org wrote:

 On Wed, Nov 14, 2012 at 2:48 PM, Geoffrey Garen gga...@apple.com wrote:

 Hi Eric.

 Here are some problems in RenderArena that I know of:

 - Grows memory without bound
 - Duplicates the functionality of FastMalloc
 - Makes allocation error-prone (allocation in the wrong arena is
 sometimes a leak, sometimes a use-after-free, and sometimes a heizenbug of
 the two)
 - Makes allocation verbose (you have to thread an arena pointer
 everywhere)
 - Makes object lifetime complicated (all objects are implicitly tied to a
 single owner they may outlive)
 - Uses C-style macros and manual initialization and destruction, instead
 of modern WebKit C++ style

 You didn't mention any of these problems in your email, so I'll assume
 you weren't aware of them.

 Considering these problems now, please don't use RenderArena in more
 places.

  Slab-allocators (i.e. RenderArena) hand out memory all from a single
  region, guaranteeing (among other things) that free'd objects can only
  be ever overwritten by other objects from the same pool.  This makes
  it much harder, for example to find a Use-After-Free of a RenderBlock
  and then fill that RenderBlock's memory (and vtable pointer) with
  arbitrary memory (like the contents of a javascript array).
  http://en.wikipedia.org/wiki/Slab_allocation

 This is magical thinking. RenderArena is no different from FastMalloc.


 It's actually different enough to be interesting:
 - The bucket granularity is different, which affects exploitation
 significantly.


How big of a difference does this (and other characteristics of
RenderArena) make in making WebCore secure? e.g. do you have data (i.e.
esti as to what percentage of security vulnerabilities we’ve had in the
past one year could have been less severe if we had used RenderArena for
DOM?

Also, can you post the complete diff somewhere so that those of us
following at home may gather data ourselves as well?

- R. Niwa.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] RenderArena: Teaching an old dog new tricks

2012-11-14 Thread Elliott Sprehn

I was present for one of the discussions about the exploit and how an arena
like allocator could have helped at Google. One proposed solution was to
allocate all the JS typed buffers in an arena.

Is there a reason we can't just do that? It's much less intrusive to
allocate ArrayBuffer in an arena than to allocate all DOM objects in one.

- E


On Wed, Nov 14, 2012 at 11:16 PM, Maciej Stachowiak m...@apple.com wrote:


 Following up on a bugzilla discussion:

  It is indeed significant that only a small number of classes are
 allocated within RenderArena. For example, when there's a RenderObject
 which has been freed but is still in use, it's not possible for the
 attacker to allocate (e.g.) a raw ArrayBuffer contents on top of the freed
 slot. So the attacker cannot place arbitrary bytes on top of where the
 vtable pointer is expected to be. We've made sure that the first
 sizeof(void*) bytes either point to a valid vtable pointer, or are a
 poisoned freelist pointer, or are an invalid pointer in the case that one
 of the (few) non-virtual classes is overlaid.
 
  It's essentially about limiting the attacker's possibilities when the
 inevitable rendering use-after-frees occur. There are some other non vtable
 related possibilities that get limited too.

 Interesting! How much value would be derived from allocating all classes
 with a virtual method in a separate heap from classes with no vtable and
 raw data buffers? And how much is specifically due to a more limited range
 of classes that may be present in the RenderArena? If classes with vtables
 getting overwritten by arbitrary data are the top fear, then there is
 probably a more modest solution than RenderArena. It could still cause
 additional memory fragmentation, but probably less so than RenderArenas.

 Regards,
 Maciej

 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] RenderArena: Teaching an old dog new tricks

On Wed, Nov 14, 2012 at 9:59 PM, Adam Barth aba...@webkit.org wrote:


 On Nov 14, 2012 8:59 PM, Ryosuke Niwa rn...@webkit.org wrote:
 
  On Wed, Nov 14, 2012 at 8:52 PM, Elliott Sprehn espr...@chromium.org
 wrote:
 
  I was present for one of the discussions about the exploit and how an
 arena like allocator could have helped at Google. One proposed solution was
 to allocate all the JS typed buffers in an arena.
 
  Is there a reason we can't just do that? It's much less intrusive to
 allocate ArrayBuffer in an arena than to allocate all DOM objects in one.
 
 
  I don’t think allocating all JS objects in an arena is good enough
 because attackers can inject nearly arbitrary sequence of bytes into DOM
 objects (e.g. text node).

 The text for a text node is stored in a String, not in the Node object
 itself.

Yeah, I guess text node was not a good example. Now that I think about it,
we can probably get most of security benefits of using RenderArena for DOM
if we can allocate all strings  js objects from arena.

- R. Niwa.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] RenderArena: Teaching an old dog new tricks


On Nov 14, 2012, at 9:59 PM, Adam Barth aba...@webkit.org wrote:

 
 On Nov 14, 2012 8:59 PM, Ryosuke Niwa rn...@webkit.org wrote:
 
  On Wed, Nov 14, 2012 at 8:52 PM, Elliott Sprehn espr...@chromium.org 
  wrote:
 
  I was present for one of the discussions about the exploit and how an 
  arena like allocator could have helped at Google. One proposed solution 
  was to allocate all the JS typed buffers in an arena.
 
  Is there a reason we can't just do that? It's much less intrusive to 
  allocate ArrayBuffer in an arena than to allocate all DOM objects in one.
 
 
  I don’t think allocating all JS objects in an arena is good enough because 
  attackers can inject nearly arbitrary sequence of bytes into DOM objects 
  (e.g. text node).
 
Just to be precise here, I imagine the thing you'd probably want to segregate 
is the void* m_data member of ArrayBufferContents.
 The text for a text node is stored in a String, not in the Node object itself
 

Is the buffer for a String likely to be materially easier or harder to use for 
this than the buffer for an ArrayBuffer? It seems like const UChar* m_data16 
is likely to be nearly as dangerous (ok, so it's not mutable, this might make a 
difference).

I feel like, to the extent there is a material distinction being made here, it 
is between objects starting with a vtable pointer (which is trusted by the C++ 
runtime to point to code) vs. allocations of the same size class that don't 
start with a vtable and where memory in that initial word can be freely 
controlled (so you can make it point to a fake vtable which points to your 
attack code).

Of course, I am talking without having actually seen any detail on the analysis 
folks are talking about, so I may be missing the point.

Cheers,
Maciej


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] RenderArena: Teaching an old dog new tricks

2012-11-14 Thread Cris Neckar

On Wed, Nov 14, 2012 at 10:05 PM, Ryosuke Niwa rn...@webkit.org wrote:

 On Wed, Nov 14, 2012 at 9:59 PM, Adam Barth aba...@webkit.org wrote:


 On Nov 14, 2012 8:59 PM, Ryosuke Niwa rn...@webkit.org wrote:
 
  On Wed, Nov 14, 2012 at 8:52 PM, Elliott Sprehn espr...@chromium.org
 wrote:
 
  I was present for one of the discussions about the exploit and how an
 arena like allocator could have helped at Google. One proposed solution was
 to allocate all the JS typed buffers in an arena.
 
  Is there a reason we can't just do that? It's much less intrusive to
 allocate ArrayBuffer in an arena than to allocate all DOM objects in one.
 
 
  I don’t think allocating all JS objects in an arena is good enough
 because attackers can inject nearly arbitrary sequence of bytes into DOM
 objects (e.g. text node).

 The text for a text node is stored in a String, not in the Node object
 itself.

 Yeah, I guess text node was not a good example. Now that I think about it,
 we can probably get most of security benefits of using RenderArena for DOM
 if we can allocate all strings  js objects from arena.

 - R. Niwa.


 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo/webkit-dev



You still have every other object type available to find useful overlapping
attributes. At best taking away JS objects is an annoyance and will
probably never make something impossible to exploit. On the other hand
DOMobjects in their own size class you would likely have many situations
where exploitation is actually not possible. I would much prefer that we
find a way to accomplish this with specific dangerous types rather than
just take away objects that make exploitation trivial.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] RenderArena: Teaching an old dog new tricks


On Nov 14, 2012, at 10:05 PM, Ryosuke Niwa rn...@webkit.org wrote:

 On Wed, Nov 14, 2012 at 9:59 PM, Adam Barth aba...@webkit.org wrote:
 
 On Nov 14, 2012 8:59 PM, Ryosuke Niwa rn...@webkit.org wrote:
 
  On Wed, Nov 14, 2012 at 8:52 PM, Elliott Sprehn espr...@chromium.org 
  wrote:
 
  I was present for one of the discussions about the exploit and how an 
  arena like allocator could have helped at Google. One proposed solution 
  was to allocate all the JS typed buffers in an arena.
 
  Is there a reason we can't just do that? It's much less intrusive to 
  allocate ArrayBuffer in an arena than to allocate all DOM objects in one.
 
 
  I don’t think allocating all JS objects in an arena is good enough because 
  attackers can inject nearly arbitrary sequence of bytes into DOM objects 
  (e.g. text node).
 
 The text for a text node is stored in a String, not in the Node object itself.
 
 Yeah, I guess text node was not a good example. Now that I think about it, we 
 can probably get most of security benefits of using RenderArena for DOM if we 
 can allocate all strings  js objects from arena.

Actual JS objects are already allocated on the GC heap instead of in the malloc 
heap. The question is where the underlying buffer for ArrayBuffer (and for 
String) goes.

Regards,
Maciej


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] RenderArena: Teaching an old dog new tricks

2012-11-14 Thread Elliott Sprehn

On Thu, Nov 15, 2012 at 1:29 AM, Ryosuke Niwa rn...@webkit.org wrote:

 ...
 In other words, why are you interested in using the proposed allocation
 mechanism for only DOM nodes/objects instead of everything in
 WebCore/WebKit?


This was my concern as well. It would seem you'd need many different
arenas, and that would only make it really annoying to get use after frees
since they have to be in the same arena, not impossible.

The major danger is really ArrayBuffer (and I suppose String) which lets
you allocate an object of a specific size and aligned the same as the freed
object. You can then create thousands of them until you get one on top of
the freed location and fill in the buffer with the malicious vtable and ptr.

How hard would it be to allocate the void* buffer and the String UChar*
with an arena?

- E
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] RenderArena: Teaching an old dog new tricks

On Wed, Nov 14, 2012 at 8:59 PM, Ryosuke Niwa rn...@webkit.org wrote:

 On Wed, Nov 14, 2012 at 8:52 PM, Elliott Sprehn espr...@chromium.orgwrote:

 I was present for one of the discussions about the exploit and how an
 arena like allocator could have helped at Google. One proposed solution was
 to allocate all the JS typed buffers in an arena.

 Is there a reason we can't just do that? It's much less intrusive to
 allocate ArrayBuffer in an arena than to allocate all DOM objects in one.


 I don’t think allocating all JS objects in an arena is good enough because
 attackers can inject nearly arbitrary sequence of bytes into DOM objects
 (e.g. text node).


Yeah, pretty much this. The worry is that it's very hard to be sure you've
identified all cases / classes where the attacker has reasonable control
over the size and exact content of the allocation. You have to start
looking at the buffers backing ArrayBuffers, the buffers backing
WTF::Vectors, the buffers backing the (multitude of) string classes, and
even then, you're left worrying about objects that simply have a bunch of
consecutive ints that the attacker can set as properties, etc. And even in
the unlikely event you catch everything in WebKit, you still have other
very attacker-controllable allocations going on in the same process such as
audio and video packets; canvas buffers; rendered image buffers; the list
goes on. I don't think it's a battle than can be won.

So we think the problem is best approached from another observation:

- Use-after-free of certain classes is either very lethal, or very common,
or both.

Use-after-free is pretty common for the RenderObject hierarchy and the Node
hierarchy. Inferno ran a quick script for use-after-free stats in automated
ClusterFuzz reports and it was approximately 332 Render and 134 Node. Due
to historical accident, we already have a protection for RenderObject.

The most lethal use-after-frees, though, are DOM objects. A freed DOM
object is often wired directly into Javascript and the attacker can prod
all sorts of methods and properties on the freed object in order to cause a
chosen set of accesses (corresponding to reads, writes and vtable usages
under the covers) in a chosen order.

I'm not comfortable sharing it verbatim on a public list, but happy to send
you a copy of the Pinkie Pie exploit if you're interested. It relies on a
lethal DOM use-after-free.

Because use-after-free in the Node hierarchy is both common and lethal, a
separate allocation area seems a profitable path forward.


Cheers
Chris


 - R. Niwa


 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo/webkit-dev


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] RenderArena: Teaching an old dog new tricks


On Nov 14, 2012, at 10:36 PM, Elliott Sprehn espr...@chromium.org wrote:

 
 On Thu, Nov 15, 2012 at 1:29 AM, Ryosuke Niwa rn...@webkit.org wrote:
 ...
 In other words, why are you interested in using the proposed allocation 
 mechanism for only DOM nodes/objects instead of everything in WebCore/WebKit?
 
  
 This was my concern as well. It would seem you'd need many different arenas, 
 and that would only make it really annoying to get use after frees since they 
 have to be in the same arena, not impossible.
 
 The major danger is really ArrayBuffer (and I suppose String) which lets you 
 allocate an object of a specific size and aligned the same as the freed 
 object. You can then create thousands of them until you get one on top of the 
 freed location and fill in the buffer with the malicious vtable and ptr.
 
 How hard would it be to allocate the void* buffer and the String UChar* with 
 an arena?

I don't think you specifically want something with RenderArena's behavior for 
those, just something disjoint from the space where anything with a vtable 
pointer goes. There's no point having separate pools per document for these, or 
tighter size classes than what you get with normal malloc, or anything like 
that.

Regards,
Maciej


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] RenderArena: Teaching an old dog new tricks


On Nov 14, 2012, at 11:09 PM, Chris Evans cev...@chromium.org wrote:

 On Wed, Nov 14, 2012 at 8:59 PM, Ryosuke Niwa rn...@webkit.org wrote:
 On Wed, Nov 14, 2012 at 8:52 PM, Elliott Sprehn espr...@chromium.org wrote:
 I was present for one of the discussions about the exploit and how an arena 
 like allocator could have helped at Google. One proposed solution was to 
 allocate all the JS typed buffers in an arena.
 
 Is there a reason we can't just do that? It's much less intrusive to allocate 
 ArrayBuffer in an arena than to allocate all DOM objects in one.
 
 I don’t think allocating all JS objects in an arena is good enough because 
 attackers can inject nearly arbitrary sequence of bytes into DOM objects 
 (e.g. text node).
 
 Yeah, pretty much this. The worry is that it's very hard to be sure you've 
 identified all cases / classes where the attacker has reasonable control over 
 the size and exact content of the allocation. You have to start looking at 
 the buffers backing ArrayBuffers, the buffers backing WTF::Vectors, the 
 buffers backing the (multitude of) string classes, and even then, you're left 
 worrying about objects that simply have a bunch of consecutive ints that the 
 attacker can set as properties, etc. And even in the unlikely event you catch 
 everything in WebKit, you still have other very attacker-controllable 
 allocations going on in the same process such as audio and video packets; 
 canvas buffers; rendered image buffers; the list goes on. I don't think it's 
 a battle than can be won.
 
 So we think the problem is best approached from another observation:
 
 - Use-after-free of certain classes is either very lethal, or very common, or 
 both.
 
 Use-after-free is pretty common for the RenderObject hierarchy and the Node 
 hierarchy. Inferno ran a quick script for use-after-free stats in automated 
 ClusterFuzz reports and it was approximately 332 Render and 134 Node. Due to 
 historical accident, we already have a protection for RenderObject.
 
 The most lethal use-after-frees, though, are DOM objects. A freed DOM object 
 is often wired directly into Javascript and the attacker can prod all sorts 
 of methods and properties on the freed object in order to cause a chosen set 
 of accesses (corresponding to reads, writes and vtable usages under the 
 covers) in a chosen order.
 
 I'm not comfortable sharing it verbatim on a public list, but happy to send 
 you a copy of the Pinkie Pie exploit if you're interested. It relies on a 
 lethal DOM use-after-free.
 
 Because use-after-free in the Node hierarchy is both common and lethal, a 
 separate allocation area seems a profitable path forward.

It still seems to me like the key difference is vtable vs no vtable, rather 
than DOM vs. not DOM. Also having a per-document arena for DOM nodes (as is 
done for render objects via RenderArena) seems irrelevant to the security goal 
and likely to cause bad memory fragmentation.

Allocating everything with a vtable separately seems like it would achieve the 
security goals, avoid the need to thread arena pointers everywhere, and avoid 
poor behavior in pathological situations, among the many flaws of RenderArenas 
as they exist today (see e.g. Geoff's list). Classes with a vtable method can 
if necessary be identified at compile/link time so we could check that they are 
given the right allocation behavior. That's much easier to do reliably than 
trying to identify buffers that are easily writable from JavaScript. And it 
makes more sense than segregating Node subclasses. (Note also that lots of 
classes that don't inherit from Node are exposed to JS too!)

Regards,
Maciej



___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] RenderArena: Teaching an old dog new tricks