Summary: | Move the Selection object out of the JS bindings | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Sam Weinig <sam> | ||||||
Component: | WebCore JavaScript | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | ||||||||
Priority: | P2 | ||||||||
Version: | 523.x (Safari 3) | ||||||||
Hardware: | Mac | ||||||||
OS: | OS X 10.4 | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 14192 | ||||||||
Attachments: |
|
Description
Sam Weinig
2007-06-22 15:47:44 PDT
Created attachment 15189 [details]
patch
Assuming disconnectFrame() gets called somewhere, DOMSelection needs to NULL check m_frame in each of its member functions. +void DOMSelection::disconnectFrame() I don't see any callers to this function. That means that, once the frame is destroyed, referencing its DOMSelection will read/write freed memory, right? I believe this used to be called by Window::disconnectFrame(). Who should call it now? + mutable RefPtr<DOMSelection> m_selection; Should probably call this m_DOMSelection to distinguish form WebCore::Selection. There seems to be a cross-frame security problem here. Once I have a window's DOMSelection object, I can read the current selection in the window even if a new document from a new domain is loaded. I can't tell if your patch introduced this bug or not, but it's pretty serious. r- for everything but the rename. Created attachment 15210 [details]
updated patch
Comment on attachment 15210 [details]
updated patch
+ return "";
A different way to do this is to return String(), which gives you the null string. I'm not sure which we want here -- null string or empty string -- though. Empty seems fine.
This patch doesn't fix the security problem, but, like I said, I don't think it introduced the problem, either. Can you file a new bug about it?
One solution to the security problem would be to call DOMSelection::disconnectFrame inside Window::clear. Another solution would be to include isSafeScript checks in the JSSelection object.
|