[PATCH] Security Bug in JS handling

Wed May 15 20:42:59 BST 2002

Hi, 

This is derived from a Bugtraq report about a similiar bug in Opera: 

<html><body>
<iframe name=cookie src="http://www.google.com/" height=0 width=0></iframe>
<a href="javascript:readCookie()">Read google cookie</a><br>
<script>
function readCookie(){
  cookie.location="javascript:alert(document.cookie);";                       
  //cookie.location="http://www.kde.org/";                       
}
</script>


We must not allow the "javascript:" url assignment to the sub-iframe if its 
domains do not match, because the assigned javascript code is executed in 
the sub-iframe's context. 

I'm not sure what the correct fix is. 

a) execute the Javascript in caller's context ?

b) avoid this situation by deny javascript:-Url assignments. 

I decided for the latter, although this is quite an unclean fix. however it 
seems to matches IE's behaviour (couldn't test with Crapzilla). Note that IE 
allows assignments of non-javascript: url's even if the domains do not 
match, which is weird, and makes me think that I've missed something. 

I fear that we have this problem in other places as well. Please 
review/comment. 


Thanks,

Dirk


-------------- next part --------------
Index: kjs_window.cpp
===================================================================
RCS file: /home/kde/kdelibs/khtml/ecma/kjs_window.cpp,v
retrieving revision 1.259
diff -u -5 -d -p -r1.259 kjs_window.cpp

--- kjs_window.cpp	2002/05/08 23:44:00	1.259
+++ kjs_window.cpp	2002/05/15 19:41:52
@@ -696,14 +696,18 @@ void Window::put(ExecState* exec, const 
       String s = value.toString(exec);
       m_part->setJSDefaultStatusBarText(s.value().qstring());
       return;
     }
     case _Location: {
-      QString str = value.toString(exec).qstring();
       KHTMLPart* p = Window::retrieveActive(exec)->m_part;
-      if ( p )
-        m_part->scheduleRedirection(0, p->htmlDocument().completeURL(str).string(), false/*don't lock history*/);
+      if (p) {
+        QString dstUrl = p->htmlDocument().completeURL(value.toString(exec).string()).string();
+        if (dstUrl.find("javascript:", 0, false) || isSafeScript(exec))
+          m_part->scheduleRedirection(0,
+                                      dstUrl,
+                                      false /*don't lock history*/);
+      }
       return;
     }
     case Onabort:
       if (isSafeScript(exec))
         setListener(exec, DOM::EventImpl::ABORT_EVENT,value);
@@ -859,10 +863,11 @@ bool Window::isSafeScript(ExecState *exe
   DOM::DOMString actDomain = actDocument.domain();
   DOM::DOMString thisDomain = thisDocument.domain();
   //kdDebug(6070) << "current domain:" << actDomain.string() << ", frame domain:" << thisDomain.string() << endl;
   if ( actDomain == thisDomain )
     return true;
+
   kdWarning(6070) << "Javascript: access denied for current frame '" << actDomain.string() << "' to frame '" << thisDomain.string() << "'" << endl;
   return false;
 }
 
 void Window::setListener(ExecState *exec, int eventId, Value func)
