545973 - VectorClass:instantiated_types needs to be per-Domain

Status: VERIFIED FIXED

(Tamarin Graveyard :: Virtual Machine, defect, P2)

Description

[Steven Johnson]

Having this list be in VectorClass means that types can outlive their Domains, leaking things unpleasantly. This should be moved to the Domain.

Comment 1

[Steven Johnson]

Attachment: Patch

This seems to mitigate the problem. 

Note that the change in VTable.cpp mitigates a latent bug we had before: we were using the wrong Traits as the base, but as long as sizeof(VectorClass)>=sizeof(ObjectVectorClass) we were safe.

Marking with security bit pending review -- I don't think this is exploitable (just a leak), but I want other thoughts on it and also to ponder over the weekend.

Comment 2

[Edwin Smith]

Comment on attachment 426789 [details] [diff] [review]
Patch

it doesn't look like the protocol for creating & searching for parameterized types mirrors the protocol for scalar types.

or... is it safe to bypass that because we stuff Vector<T> into whatever domain owns T?

Comment 3

[Steven Johnson]

(In reply to comment #2)
> (From update of attachment 426789 [details] [diff] [review])
> it doesn't look like the protocol for creating & searching for parameterized
> types mirrors the protocol for scalar types.
> 
> or... is it safe to bypass that because we stuff Vector<T> into whatever domain
> owns T?

That's the theory.

Comment 4

[Steven Johnson]

ok, I have r+ on this, now the question is, is this really a security issue or not? (I can land it in tr-sec if there's any doubt but it's on the large side for a security patch, so longterm maintenance will be easier if it's not necessary)

Comment 5

[Steven Johnson]

http://hg.mozilla.org/tamarin-redux/rev/fb13df575e8c
http://hg.mozilla.org/tamarin-redux/rev/83c11703715b

Comment 6

[Chris Peyer]

Engineering work item.  Marking verified fixed.