Changeset 97353 in webkit


Ignore:
Timestamp:
Oct 13, 2011 1:53:50 AM (13 years ago)
Author:
abarth@webkit.org
Message:

DOMWindow subobjects can be re-created after navigation
​https://bugs.webkit.org/show_bug.cgi?id=68849

Reviewed by Sam Weinig.

Source/WebCore:

Test: http/tests/security/xss-DENIED-getSelection-from-inactive-domwindow.html

  • page/DOMWindow.cpp:

(WebCore::DOMWindow::~DOMWindow):

  • Add ASSERTs to show that we're not recreating these objects.
  • Add a call to clear() as defense in depth in case we have any of these objects hanging around.

(WebCore::DOMWindow::clear):

  • Clear out a couple of objects that weren't getting cleared. These are actually not likely to cause problems, but clearing them out is the safe thing to do.

(WebCore::DOMWindow::isActive):

  • Add a concept of whether the DOMWindow is "active" in its frame. We had this concept in a couple places already, but centralizing it into a helper function make it easier to use and talk about.

(WebCore::DOMWindow::orientation):

  • Whitespace nit.

(WebCore::DOMWindow::screen):
(WebCore::DOMWindow::history):
(WebCore::DOMWindow::crypto):
(WebCore::DOMWindow::locationbar):
(WebCore::DOMWindow::menubar):
(WebCore::DOMWindow::personalbar):
(WebCore::DOMWindow::scrollbars):
(WebCore::DOMWindow::statusbar):
(WebCore::DOMWindow::toolbar):
(WebCore::DOMWindow::console):
(WebCore::DOMWindow::applicationCache):
(WebCore::DOMWindow::navigator):
(WebCore::DOMWindow::performance):
(WebCore::DOMWindow::location):
(WebCore::DOMWindow::sessionStorage):
(WebCore::DOMWindow::localStorage):
(WebCore::DOMWindow::webkitNotifications):
(WebCore::DOMWindow::webkitIndexedDB):
(WebCore::DOMWindow::getSelection):
(WebCore::DOMWindow::styleMedia):
(WebCore::DOMWindow::webkitURL):
(WebCore::DOMWindow::webkitStorageInfo):

  • Avoid creating these objects when we're not active. That can only lead to sadness.

(WebCore::DOMWindow::webkitRequestFileSystem):
(WebCore::DOMWindow::webkitResolveLocalFileSystemURL):
(WebCore::DOMWindow::openDatabase):
(WebCore::DOMWindow::postMessage):

  • While not techincally creating subobjects, these functions also seem unwise when the DOMWindow is inactive.

(WebCore::DOMWindow::find):
(WebCore::DOMWindow::length):
(WebCore::DOMWindow::getMatchedCSSRules):

  • These functions operate on the active Document. When we're not active, that's not us!

(WebCore::DOMWindow::document):

  • Update to use the new concept of being active rather than having this function roll its own implementation.

(WebCore::DOMWindow::webkitConvertPointFromNodeToPage):
(WebCore::DOMWindow::webkitConvertPointFromPageToNode):
(WebCore::DOMWindow::scrollBy):
(WebCore::DOMWindow::scrollTo):

  • These functions also look unwise to run when inactive because they're reading information from the active document.
  • I added a RefPtr for node because the call to updateLayoutIgnorePendingStylesheets() seems likely to be able to run script somehow.

(WebCore::DOMWindow::addEventListener):
(WebCore::DOMWindow::removeEventListener):
(WebCore::DOMWindow::dispatchLoadEvent):
(WebCore::DOMWindow::dispatchEvent):

  • I don't think these functions worked when inactive anyway, but explicitly blocking them seems wise.

(WebCore::DOMWindow::setLocation):
(WebCore::DOMWindow::isInsecureScriptAccess):
(WebCore::DOMWindow::open):
(WebCore::DOMWindow::showModalDialog):

  • These already have checks for being active, but it can't hurt to be explicit at the top of the function.
  • page/DOMWindow.h:

LayoutTests:

  • http/tests/security/xss-DENIED-getSelection-from-inactive-domwindow-expected.txt: Added.
  • http/tests/security/xss-DENIED-getSelection-from-inactive-domwindow.html: Added.
Location:
trunk
Files:
2 added
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r97352 r97353  
     12011-10-13  Adam Barth  <abarth@webkit.org>
     2
     3        DOMWindow subobjects can be re-created after navigation
     4        https://bugs.webkit.org/show_bug.cgi?id=68849
     5
     6        Reviewed by Sam Weinig.
     7
     8        * http/tests/security/xss-DENIED-getSelection-from-inactive-domwindow-expected.txt: Added.
     9        * http/tests/security/xss-DENIED-getSelection-from-inactive-domwindow.html: Added.
     10
    1112011-10-13  Kent Tamura  <tkent@chromium.org>
    212

  • trunk/Source/WebCore/ChangeLog

    r97351 r97353  
     12011-10-13  Adam Barth  <abarth@webkit.org>
     2
     3        DOMWindow subobjects can be re-created after navigation
     4        https://bugs.webkit.org/show_bug.cgi?id=68849
     5
     6        Reviewed by Sam Weinig.
     7
     8        Test: http/tests/security/xss-DENIED-getSelection-from-inactive-domwindow.html
     9
     10        * page/DOMWindow.cpp:
     11        (WebCore::DOMWindow::~DOMWindow):
     12            - Add ASSERTs to show that we're not recreating these objects.
     13            - Add a call to clear() as defense in depth in case we have any of
     14              these objects hanging around.
     15        (WebCore::DOMWindow::clear):
     16            - Clear out a couple of objects that weren't getting cleared.
     17              These are actually not likely to cause problems, but clearing
     18              them out is the safe thing to do.
     19        (WebCore::DOMWindow::isActive):
     20            - Add a concept of whether the DOMWindow is "active" in its frame.
     21              We had this concept in a couple places already, but centralizing
     22              it into a helper function make it easier to use and talk about.
     23        (WebCore::DOMWindow::orientation):
     24            - Whitespace nit.
     25        (WebCore::DOMWindow::screen):
     26        (WebCore::DOMWindow::history):
     27        (WebCore::DOMWindow::crypto):
     28        (WebCore::DOMWindow::locationbar):
     29        (WebCore::DOMWindow::menubar):
     30        (WebCore::DOMWindow::personalbar):
     31        (WebCore::DOMWindow::scrollbars):
     32        (WebCore::DOMWindow::statusbar):
     33        (WebCore::DOMWindow::toolbar):
     34        (WebCore::DOMWindow::console):
     35        (WebCore::DOMWindow::applicationCache):
     36        (WebCore::DOMWindow::navigator):
     37        (WebCore::DOMWindow::performance):
     38        (WebCore::DOMWindow::location):
     39        (WebCore::DOMWindow::sessionStorage):
     40        (WebCore::DOMWindow::localStorage):
     41        (WebCore::DOMWindow::webkitNotifications):
     42        (WebCore::DOMWindow::webkitIndexedDB):
     43        (WebCore::DOMWindow::getSelection):
     44        (WebCore::DOMWindow::styleMedia):
     45        (WebCore::DOMWindow::webkitURL):
     46        (WebCore::DOMWindow::webkitStorageInfo):
     47            - Avoid creating these objects when we're not active.  That can
     48              only lead to sadness.
     49        (WebCore::DOMWindow::webkitRequestFileSystem):
     50        (WebCore::DOMWindow::webkitResolveLocalFileSystemURL):
     51        (WebCore::DOMWindow::openDatabase):
     52        (WebCore::DOMWindow::postMessage):
     53            - While not techincally creating subobjects, these functions also
     54              seem unwise when the DOMWindow is inactive.
     55        (WebCore::DOMWindow::find):
     56        (WebCore::DOMWindow::length):
     57        (WebCore::DOMWindow::getMatchedCSSRules):
     58            - These functions operate on the active Document.  When we're not
     59              active, that's not us!
     60        (WebCore::DOMWindow::document):
     61            - Update to use the new concept of being active rather than having
     62              this function roll its own implementation.
     63        (WebCore::DOMWindow::webkitConvertPointFromNodeToPage):
     64        (WebCore::DOMWindow::webkitConvertPointFromPageToNode):
     65        (WebCore::DOMWindow::scrollBy):
     66        (WebCore::DOMWindow::scrollTo):
     67            - These functions also look unwise to run when inactive because
     68              they're reading information from the active document.
     69            - I added a RefPtr for node because the call to
     70              updateLayoutIgnorePendingStylesheets() seems likely to be able to
     71              run script somehow.
     72        (WebCore::DOMWindow::addEventListener):
     73        (WebCore::DOMWindow::removeEventListener):
     74        (WebCore::DOMWindow::dispatchLoadEvent):
     75        (WebCore::DOMWindow::dispatchEvent):
     76            - I don't think these functions worked when inactive anyway, but
     77              explicitly blocking them seems wise.
     78        (WebCore::DOMWindow::setLocation):
     79        (WebCore::DOMWindow::isInsecureScriptAccess):
     80        (WebCore::DOMWindow::open):
     81        (WebCore::DOMWindow::showModalDialog):
     82            - These already have checks for being active, but it can't hurt to
     83              be explicit at the top of the function.
     84        * page/DOMWindow.h:
     85
    1862011-10-13  Kent Tamura  <tkent@chromium.org>
    287

  • trunk/Source/WebCore/page/DOMWindow.cpp

    r95919 r97353  
    406406        m_frame->clearFormerDOMWindow(this);
    407407
     408    ASSERT(!m_screen);
     409    ASSERT(!m_selection);
     410    ASSERT(!m_history);
     411    ASSERT(!m_crypto);
     412    ASSERT(!m_locationbar);
     413    ASSERT(!m_menubar);
     414    ASSERT(!m_personalbar);
     415    ASSERT(!m_scrollbars);
     416    ASSERT(!m_statusbar);
     417    ASSERT(!m_toolbar);
     418    ASSERT(!m_console);
     419    ASSERT(!m_navigator);
     420#if ENABLE(WEB_TIMING)
     421    ASSERT(!m_performance);
     422#endif
     423    ASSERT(!m_location);
     424    ASSERT(!m_media);
     425#if ENABLE(DOM_STORAGE)
     426    ASSERT(!m_sessionStorage);
     427    ASSERT(!m_localStorage);
     428#endif
     429    ASSERT(!m_applicationCache);
     430#if ENABLE(NOTIFICATIONS)
     431    ASSERT(!m_notifications);
     432#endif
     433#if ENABLE(INDEXED_DATABASE)
     434    ASSERT(!m_idbFactory);
     435#endif
     436#if ENABLE(BLOB)
     437    ASSERT(!m_domURL);
     438#endif
     439#if ENABLE(QUOTA)
     440    ASSERT(!m_storageInfo);
     441#endif
     442
     443    // This clear should be unnessary given the ASSERTs above, but we don't
     444    // want any of these objects to hang around after we've been destroyed.
     445    clear();
     446
    408447    removeAllUnloadEventListeners(this);
    409448    removeAllBeforeUnloadEventListeners(this);
    … …  
    516555    m_idbFactory = 0;
    517556#endif
     557
     558#if ENABLE(BLOB)
     559    m_domURL = 0;
     560#endif
     561
     562#if ENABLE(QUOTA)
     563    m_storageInfo = 0;
     564#endif
     565}
     566
     567bool DOMWindow::isCurrentlyDisplayedInFrame() const
     568{
     569    return m_frame && m_frame->domWindow() == this;
    518570}
    519571
    … …  
    523575    if (!m_frame)
    524576        return 0;
    525    
     577
    526578    return m_frame->orientation();
    527579}
    … …  
    530582Screen* DOMWindow::screen() const
    531583{
    532     if (!m_screen)
     584    if (!m_screen && isCurrentlyDisplayedInFrame())
    533585        m_screen = Screen::create(m_frame);
    534586    return m_screen.get();
    … …  
    537589History* DOMWindow::history() const
    538590{
    539     if (!m_history)
     591    if (!m_history && isCurrentlyDisplayedInFrame())
    540592        m_history = History::create(m_frame);
    541593    return m_history.get();
    … …  
    544596Crypto* DOMWindow::crypto() const
    545597{
    546     if (!m_crypto)
     598    if (!m_crypto && isCurrentlyDisplayedInFrame())
    547599        m_crypto = Crypto::create();
    548600    return m_crypto.get();
    … …  
    551603BarInfo* DOMWindow::locationbar() const
    552604{
    553     if (!m_locationbar)
     605    if (!m_locationbar && isCurrentlyDisplayedInFrame())
    554606        m_locationbar = BarInfo::create(m_frame, BarInfo::Locationbar);
    555607    return m_locationbar.get();
    … …  
    558610BarInfo* DOMWindow::menubar() const
    559611{
    560     if (!m_menubar)
     612    if (!m_menubar && isCurrentlyDisplayedInFrame())
    561613        m_menubar = BarInfo::create(m_frame, BarInfo::Menubar);
    562614    return m_menubar.get();
    … …  
    565617BarInfo* DOMWindow::personalbar() const
    566618{
    567     if (!m_personalbar)
     619    if (!m_personalbar && isCurrentlyDisplayedInFrame())
    568620        m_personalbar = BarInfo::create(m_frame, BarInfo::Personalbar);
    569621    return m_personalbar.get();
    … …  
    572624BarInfo* DOMWindow::scrollbars() const
    573625{
    574     if (!m_scrollbars)
     626    if (!m_scrollbars && isCurrentlyDisplayedInFrame())
    575627        m_scrollbars = BarInfo::create(m_frame, BarInfo::Scrollbars);
    576628    return m_scrollbars.get();
    … …  
    579631BarInfo* DOMWindow::statusbar() const
    580632{
    581     if (!m_statusbar)
     633    if (!m_statusbar && isCurrentlyDisplayedInFrame())
    582634        m_statusbar = BarInfo::create(m_frame, BarInfo::Statusbar);
    583635    return m_statusbar.get();
    … …  
    586638BarInfo* DOMWindow::toolbar() const
    587639{
    588     if (!m_toolbar)
     640    if (!m_toolbar && isCurrentlyDisplayedInFrame())
    589641        m_toolbar = BarInfo::create(m_frame, BarInfo::Toolbar);
    590642    return m_toolbar.get();
    … …  
    593645Console* DOMWindow::console() const
    594646{
    595     if (!m_console)
     647    if (!m_console && isCurrentlyDisplayedInFrame())
    596648        m_console = Console::create(m_frame);
    597649    return m_console.get();
    … …  
    600652DOMApplicationCache* DOMWindow::applicationCache() const
    601653{
    602     if (!m_applicationCache)
     654    if (!m_applicationCache && isCurrentlyDisplayedInFrame())
    603655        m_applicationCache = DOMApplicationCache::create(m_frame);
    604656    return m_applicationCache.get();
    … …  
    607659Navigator* DOMWindow::navigator() const
    608660{
    609     if (!m_navigator)
     661    if (!m_navigator && isCurrentlyDisplayedInFrame())
    610662        m_navigator = Navigator::create(m_frame);
    611663    return m_navigator.get();
    … …  
    615667Performance* DOMWindow::performance() const
    616668{
    617     if (!m_performance)
     669    if (!m_performance && isCurrentlyDisplayedInFrame())
    618670        m_performance = Performance::create(m_frame);
    619671    return m_performance.get();
    … …  
    623675Location* DOMWindow::location() const
    624676{
    625     if (!m_location)
     677    if (!m_location && isCurrentlyDisplayedInFrame())
    626678        m_location = Location::create(m_frame);
    627679    return m_location.get();
    … …  
    631683Storage* DOMWindow::sessionStorage(ExceptionCode& ec) const
    632684{
    633     if (m_sessionStorage)
     685    if (m_sessionStorage || !isCurrentlyDisplayedInFrame())
    634686        return m_sessionStorage.get();
    635687
    … …  
    656708Storage* DOMWindow::localStorage(ExceptionCode& ec) const
    657709{
    658     if (m_localStorage)
     710    if (m_localStorage || !isCurrentlyDisplayedInFrame())
    659711        return m_localStorage.get();
    660712
    … …  
    686738NotificationCenter* DOMWindow::webkitNotifications() const
    687739{
    688     if (m_notifications)
     740    if (m_notifications || !isCurrentlyDisplayedInFrame())
    689741        return m_notifications.get();
    690742
    … …  
    738790        return 0;
    739791
    740     if (!m_idbFactory)
     792    if (!m_idbFactory && isCurrentlyDisplayedInFrame())
    741793        m_idbFactory = IDBFactory::create(page->group().idbFactory());
    742794    return m_idbFactory.get();
    … …  
    747799void DOMWindow::webkitRequestFileSystem(int type, long long size, PassRefPtr<FileSystemCallback> successCallback, PassRefPtr<ErrorCallback> errorCallback)
    748800{
     801    if (!isCurrentlyDisplayedInFrame())
     802        return;
     803
    749804    Document* document = this->document();
    750805    if (!document)
    … …  
    767822void DOMWindow::webkitResolveLocalFileSystemURL(const String& url, PassRefPtr<EntryCallback> successCallback, PassRefPtr<ErrorCallback> errorCallback)
    768823{
     824    if (!isCurrentlyDisplayedInFrame())
     825        return;
     826
    769827    Document* document = this->document();
    770828    if (!document)
    … …  
    805863void DOMWindow::postMessage(PassRefPtr<SerializedScriptValue> message, const MessagePortArray* ports, const String& targetOrigin, DOMWindow* source, ExceptionCode& ec)
    806864{
    807     if (!m_frame)
     865    if (!isCurrentlyDisplayedInFrame())
    808866        return;
    809867
    … …  
    857915DOMSelection* DOMWindow::getSelection()
    858916{
    859     if (!m_selection)
     917    if (!m_selection && isCurrentlyDisplayedInFrame())
    860918        m_selection = DOMSelection::create(m_frame);
    861919    return m_selection.get();
    … …  
    10451103bool DOMWindow::find(const String& string, bool caseSensitive, bool backwards, bool wrap, bool /*wholeWord*/, bool /*searchInFrames*/, bool /*showDialog*/) const
    10461104{
    1047     if (!m_frame)
     1105    if (!isCurrentlyDisplayedInFrame())
    10481106        return false;
    10491107
    … …  
    11641222unsigned DOMWindow::length() const
    11651223{
    1166     if (!m_frame)
     1224    if (!isCurrentlyDisplayedInFrame())
    11671225        return 0;
    11681226
    … …  
    12621320Document* DOMWindow::document() const
    12631321{
     1322    if (!isCurrentlyDisplayedInFrame())
     1323        return 0;
     1324
    12641325    // FIXME: This function shouldn't need a frame to work.
    1265     if (!m_frame)
    1266         return 0;
    1267 
    1268     // The m_frame pointer is not zeroed out when the window is put into b/f cache, so it can hold an unrelated document/window pair.
    1269     // FIXME: We should always zero out the frame pointer on navigation to avoid accidentally accessing the new frame content.
    1270     if (m_frame->domWindow() != this)
    1271         return 0;
    1272 
    12731326    ASSERT(m_frame->document());
    12741327    return m_frame->document();
    … …  
    12771330PassRefPtr<StyleMedia> DOMWindow::styleMedia() const
    12781331{
    1279     if (!m_media)
     1332    if (!m_media && isCurrentlyDisplayedInFrame())
    12801333        m_media = StyleMedia::create(m_frame);
    12811334    return m_media.get();
    … …  
    12921345PassRefPtr<CSSRuleList> DOMWindow::getMatchedCSSRules(Element* element, const String&, bool authorOnly) const
    12931346{
    1294     if (!m_frame)
     1347    if (!isCurrentlyDisplayedInFrame())
    12951348        return 0;
    12961349
    … …  
    13111364        return 0;
    13121365
    1313     m_frame->document()->updateLayoutIgnorePendingStylesheets();
     1366    if (!document())
     1367        return 0;
     1368
     1369    document()->updateLayoutIgnorePendingStylesheets();
    13141370
    13151371    FloatPoint pagePoint(p->x(), p->y());
    … …  
    13231379        return 0;
    13241380
    1325     m_frame->document()->updateLayoutIgnorePendingStylesheets();
     1381    if (!document())
     1382        return 0;
     1383
     1384    document()->updateLayoutIgnorePendingStylesheets();
    13261385
    13271386    FloatPoint nodePoint(p->x(), p->y());
    … …  
    13451404PassRefPtr<Database> DOMWindow::openDatabase(const String& name, const String& version, const String& displayName, unsigned long estimatedSize, PassRefPtr<DatabaseCallback> creationCallback, ExceptionCode& ec)
    13461405{
     1406    if (!isCurrentlyDisplayedInFrame())
     1407        return 0;
     1408
    13471409    RefPtr<Database> database = 0;
    13481410    if (m_frame && AbstractDatabase::isAvailable() && m_frame->document()->securityOrigin()->canAccessDatabase())
    … …  
    13581420void DOMWindow::scrollBy(int x, int y) const
    13591421{
    1360     if (!m_frame)
    1361         return;
    1362 
    1363     m_frame->document()->updateLayoutIgnorePendingStylesheets();
     1422    if (!isCurrentlyDisplayedInFrame())
     1423        return;
     1424
     1425    document()->updateLayoutIgnorePendingStylesheets();
    13641426
    13651427    FrameView* view = m_frame->view();
    … …  
    13721434void DOMWindow::scrollTo(int x, int y) const
    13731435{
    1374     if (!m_frame)
    1375         return;
    1376 
    1377     m_frame->document()->updateLayoutIgnorePendingStylesheets();
     1436    if (!isCurrentlyDisplayedInFrame())
     1437        return;
     1438
     1439    document()->updateLayoutIgnorePendingStylesheets();
    13781440
    13791441    RefPtr<FrameView> view = m_frame->view();
    … …  
    16521714void DOMWindow::setLocation(const String& urlString, DOMWindow* activeWindow, DOMWindow* firstWindow, SetLocationLocking locking)
    16531715{
    1654     if (!m_frame)
     1716    if (!isCurrentlyDisplayedInFrame())
    16551717        return;
    16561718
    … …  
    17121774        return false;
    17131775
    1714     // If m_frame->domWindow() != this, then |this| isn't the DOMWindow that's
    1715     // currently active in the frame and there's no way we should allow the
    1716     // access.
     1776    // If this DOMWindow isn't currently active in the Frame, then there's no
     1777    // way we should allow the access.
    17171778    // FIXME: Remove this check if we're able to disconnect DOMWindow from
    17181779    // Frame on navigation: https://bugs.webkit.org/show_bug.cgi?id=62054
    1719     if (m_frame->domWindow() == this) {
     1780    if (isCurrentlyDisplayedInFrame()) {
    17201781        // FIXME: Is there some way to eliminate the need for a separate "activeWindow == this" check?
    17211782        if (activeWindow == this)
    … …  
    17741835    DOMWindow* activeWindow, DOMWindow* firstWindow)
    17751836{
    1776     if (!m_frame)
     1837    if (!isCurrentlyDisplayedInFrame())
    17771838        return 0;
    17781839    Frame* activeFrame = activeWindow->frame();
    … …  
    18421903    DOMWindow* activeWindow, DOMWindow* firstWindow, PrepareDialogFunction function, void* functionContext)
    18431904{
    1844     if (!m_frame)
     1905    if (!isCurrentlyDisplayedInFrame())
    18451906        return;
    18461907    Frame* activeFrame = activeWindow->frame();
    … …  
    18651926DOMURL* DOMWindow::webkitURL() const
    18661927{
    1867     if (!m_domURL)
     1928    if (!m_domURL && isCurrentlyDisplayedInFrame())
    18681929        m_domURL = DOMURL::create(this->scriptExecutionContext());
    18691930    return m_domURL.get();
    … …  
    18741935StorageInfo* DOMWindow::webkitStorageInfo() const
    18751936{
    1876     if (!m_storageInfo)
     1937    if (!m_storageInfo && isCurrentlyDisplayedInFrame())
    18771938        m_storageInfo = StorageInfo::create();
    18781939    return m_storageInfo.get();

  • trunk/Source/WebCore/page/DOMWindow.h

    r95919 r97353  
    415415        DOMWindow(Frame*);
    416416
     417        // FIXME: When this DOMWindow is no longer the active DOMWindow (i.e.,
     418        // when its document is no longer the document that is displayed in its
     419        // frame), we would like to zero out m_frame to avoid being confused
     420        // by the document that is currently active in m_frame.
     421        bool isCurrentlyDisplayedInFrame() const;
     422
    417423        virtual void refEventTarget() { ref(); }
    418424        virtual void derefEventTarget() { deref(); }
Note: See TracChangeset for help on using the changeset viewer.