[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    branches/KDE/3.5/kdelibs/kioslave/http/kcookiejar
From:       Dawit Alemayehu <adawit () kde ! org>
Date:       2007-09-01 17:19:15
Message-ID: 1188667155.701496.24165.nullmailer () svn ! kde ! org
[Download RAW message or body]

SVN commit 707375 by adawit:

- Fix BR# 134421 and as a result fix a cross-site cookie injection vulnerability.
- Simplyfy the code that saves the contents of the cookiejar in KCookieServer.


 M  +10 -27    kcookiejar.cpp  
 M  +16 -23    kcookieserver.cpp  
 M  +5 -0      tests/cookie.test  


--- branches/KDE/3.5/kdelibs/kioslave/http/kcookiejar/kcookiejar.cpp #707374:707375
@@ -981,44 +981,27 @@
 //
 KCookieAdvice KCookieJar::cookieAdvice(KHttpCookiePtr cookiePtr)
 {
-    QStringList domains;
-
     if (m_rejectCrossDomainCookies && cookiePtr->isCrossDomain())
        return KCookieReject;
 
-    if (m_autoAcceptSessionCookies && (cookiePtr->expireDate() == 0 ||
-        m_ignoreCookieExpirationDate))
-       return KCookieAccept;
+    QStringList domains;
 
     extractDomains(cookiePtr->host(), domains);
 
-    // If the cookie specifies a domain, check whether it is valid and
-    // correct otherwise.
+    // If the cookie specifies a domain, check whether it is valid. Otherwise,
+    // accept the cookie anyways but removes the domain="" value to prevent
+    // cross-site cookie injection.
     if (!cookiePtr->domain().isEmpty())
     {
-       bool valid = false;
-
-       // This checks whether the cookie is valid based on
-       // what ::extractDomains returns
-       if (!valid)
-       {
-          if (domains.contains(cookiePtr->domain()))
-             valid = true;
-       }
-
-       if (!valid)
-       {
-          // Maybe it points to a sub-domain
-          if (cookiePtr->domain().endsWith("."+cookiePtr->host()))
-             valid = true;
-       }
-
-       if (!valid)
-       {
+      if (!domains.contains(cookiePtr->domain()) && 
+          !cookiePtr->domain().endsWith("."+cookiePtr->host()))
           cookiePtr->fixDomain(QString::null);
-       }
     }
 
+    if (m_autoAcceptSessionCookies && (cookiePtr->expireDate() == 0 ||
+        m_ignoreCookieExpirationDate))
+       return KCookieAccept;
+
     KCookieAdvice advice = KCookieDunno;
     bool isFQDN = true; // First is FQDN
     QStringList::Iterator it = domains.begin(); // Start with FQDN which first in the list.
--- branches/KDE/3.5/kdelibs/kioslave/http/kcookiejar/kcookieserver.cpp #707374:707375
@@ -86,7 +86,8 @@
    mPendingCookies->setAutoDelete(true);
    mRequestList = new RequestList;
    mAdvicePending = false;
-   mTimer = 0;
+   mTimer = new QTimer();
+   connect( mTimer, SIGNAL( timeout()), SLOT( slotSave()));
    mConfig = new KConfig("kcookiejarrc");
    mCookieJar->loadConfig( mConfig );
 
@@ -186,6 +187,7 @@
     KHttpCookiePtr cookie = list->first();
     while (cookie)
     {
+        kdDebug(7104) << "checkCookies: Asking cookie advice for " << cookie->host() << endl;
         KCookieAdvice advice = mCookieJar->cookieAdvice(cookie);
         switch(advice)
         {
@@ -256,7 +258,7 @@
                break;
 
            default:
-               qWarning(__FILE__":%d Problen!", __LINE__);
+               qWarning(__FILE__":%d Problem!", __LINE__);
                cookie = mPendingCookies->next();
                break;
            }
@@ -292,26 +294,22 @@
           request = mRequestList->next();
         }
     }
-    if (mCookieJar->changed() && !mTimer)
+    if (mCookieJar->changed())
         saveCookieJar();
 }
 
 void KCookieServer::slotSave()
 {
-   delete mTimer;
-   mTimer = 0;
    QString filename = locateLocal("data", "kcookiejar/cookies");
    mCookieJar->saveCookies(filename);
 }
 
 void KCookieServer::saveCookieJar()
 {
-    if( mTimer )
+    if( mTimer->isActive() )
         return;
 
-    mTimer = new QTimer();
-    connect( mTimer, SIGNAL( timeout()), SLOT( slotSave()));
-    mTimer->start( 1000*60*SAVE_DELAY );
+    mTimer->start( 1000*60*SAVE_DELAY, true );
 }
 
 void KCookieServer::putCookie( QStringList& out, KHttpCookie *cookie,
@@ -391,12 +389,12 @@
       mRequestList->append( request );
       return QString::null; // Talk to you later :-)
    }
-   
+
    QString cookies = mCookieJar->findCookies(url, false, windowId);
-   
-   if (mCookieJar->changed() && !mTimer)
+
+   if (mCookieJar->changed())
       saveCookieJar();
-   
+
    return cookies;
 }
 
@@ -491,8 +489,7 @@
          if( cookieMatches(it.current(), domain, fqdn, path, name) )
          {
             mCookieJar->eatCookie( it.current() );
-            if (!mTimer)
-               saveCookieJar();
+            saveCookieJar();
             break;
          }
       }
@@ -504,8 +501,7 @@
 KCookieServer::deleteCookiesFromDomain(QString domain)
 {
    mCookieJar->eatCookiesForDomain(domain);
-   if (!mTimer)
-      saveCookieJar();
+   saveCookieJar();
 }
 
 
@@ -521,16 +517,14 @@
 KCookieServer::deleteSessionCookies( long windowId )
 {
   mCookieJar->eatSessionCookies( windowId );
-  if(!mTimer)
-    saveCookieJar();
+  saveCookieJar();
 }
 
 void
 KCookieServer::deleteSessionCookiesFor(QString fqdn, long windowId)
 {
   mCookieJar->eatSessionCookies( fqdn, windowId );
-  if(!mTimer)
-    saveCookieJar();
+  saveCookieJar();
 }
 
 // DCOP function
@@ -538,8 +532,7 @@
 KCookieServer::deleteAllCookies()
 {
    mCookieJar->eatAllCookies();
-   if (!mTimer)
-      saveCookieJar();
+   saveCookieJar();
 }
 
 // DCOP function
--- branches/KDE/3.5/kdelibs/kioslave/http/kcookiejar/tests/cookie.test #707374:707375
@@ -144,3 +144,8 @@
 CHECK http://z.foobar.com/
 CHECK http://www.foobar.com/
 CHECK http://foobar.com/
+CLEAR COOKIES
+## Check domain restrictions #8
+CONFIG AcceptSessionCookies true
+COOKIE ACCEPT http://www.foobar.com Set-Cookie: from=foobar.com; domain=bar.com; Path="/"
+CHECK http://bar.com
[prev in list] [next in list] [prev in thread] [next in thread]