VectorClass:instantiated_types needs to be per-Domain

VERIFIED FIXED in flash10.1

Status

P2
normal
VERIFIED FIXED
9 years ago
9 years ago

People

(Reporter: stejohns, Assigned: stejohns)

Tracking

unspecified
flash10.1
Bug Flags:
flashplayer-qrb +

Details

Attachments

(1 attachment)

(Assignee)

Description

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

Comment 1

9 years ago
Created attachment 426789 [details] [diff] [review]
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.
Assignee: nobody → stejohns
Attachment #426789 - Flags: superreview?(edwsmith)
Attachment #426789 - Flags: review?
(Assignee)

Updated

9 years ago
Attachment #426789 - Flags: review? → review?(tierney)

Updated

9 years ago
Attachment #426789 - Flags: review?(tierney) → review+

Comment 2

9 years ago
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?
Attachment #426789 - Flags: superreview?(edwsmith) → superreview-
(Assignee)

Comment 3

9 years ago
(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.

Updated

9 years ago
Attachment #426789 - Flags: superreview- → superreview+

Updated

9 years ago
Status: NEW → ASSIGNED
Flags: flashplayer-qrb+
Priority: -- → P2
Target Milestone: --- → flash10.1
(Assignee)

Comment 4

9 years ago
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)
(Assignee)

Comment 5

9 years ago
http://hg.mozilla.org/tamarin-redux/rev/fb13df575e8c
http://hg.mozilla.org/tamarin-redux/rev/83c11703715b
Group: tamarin-security
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED

Comment 6

9 years ago
Engineering work item.  Marking verified fixed.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.