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

List:       kfm-devel
Subject:    PATCH: XMLHttpRequest
From:       Dawit Alemayehu <adawit () kde ! org>
Date:       2005-10-13 0:35:52
Message-ID: 200510122035.53336.adawit () kde ! org
[Download RAW message or body]

Hello,

The attached patch addresses several issues with the current implementation of 
XMLHttpRequest object:

1.) Add a referrer by default as requested in bug 113962. If one is supplied 
through the "setRequestHeader" call, we sanitize it to make sure that it was 
not an attempt to spoof the referrer header, which can be a security issue.

2.) Deny all but "get" and "post" support through "setRequestHeader" and make 
sure the "get" and "post" requests are routed through the "open" function 
call to ensure that they are sanitized properly.

3.) Instead of blindly doing a get action when the requested method is other 
than http, simply abort the request. While we are at it, store the method 
type in lowercase.

Comments, feedback...

-- 
Regards,
Dawit A.
"Practice what you preach, preach what you practice"

["xmlhttprequest.diff" (text/x-diff)]

Index: xmlhttprequest.cpp
===================================================================
--- xmlhttprequest.cpp	(revision 468536)
+++ xmlhttprequest.cpp	(working copy)
@@ -257,6 +257,10 @@
 
 bool XMLHttpRequest::urlMatchesDocumentDomain(const KURL& _url) const
 {
+  // No need to do work if _url is not valid...
+  if (!_url.isValid())
+    return false;
+
   KURL documentURL(doc->URL());
 
   // a local file can load anything
@@ -297,7 +301,7 @@
   }
 
 
-  method = _method;
+  method = _method.lower();
   url = _url;
   async = _async;
 
@@ -308,9 +312,19 @@
 {
   aborted = false;
 
-  if (method.lower() == "post" && (url.protocol().lower().startsWith("http"))) {
-    // FIXME: determine post encoding correctly by looking in headers for charset
+  if (method == "post") {
+    QString protocol = url.protocol().lower();
 
+    // Do not blindly change a "post" request to a "get" when the protocol
+    // is other than "http". Instead abondon the request.
+    if (!protocol.startsWith("http") && !protocol.startsWith("webdav"))
+    {
+      abort();
+      return;
+    }
+
+    // FIXME: determine post encoding correctly by looking in headers
+    // for charset.
     QByteArray buf;
     buf.duplicate(_body.utf8().data(), _body.length());
 
@@ -333,10 +347,22 @@
         rh += "\r\n";
       rh += i.key() + ": " + i.data();
     }
+
     job->addMetaData("customHTTPHeader", rh);
   }
-  job->addMetaData( "PropagateHttpHeader", "true" );
 
+  job->addMetaData("PropagateHttpHeader", "true");
+
+  // Set the default referrer if one is not already supplied
+  // through setRequestHeader. NOTE: the user can still disable
+  // this feature at the protocol level (kio_http).
+  if (requestHeaders.find("Referer") == requestHeaders.end()) {
+    KURL documentURL(doc->URL());
+    documentURL.setPass(QString::null);
+    documentURL.setUser(QString::null);
+    job->addMetaData("referrer", documentURL.url());
+  }
+
   if (!async) {
     QByteArray data;
     KURL finalURL;
@@ -385,17 +411,46 @@
   aborted = true;
 }
 
-void XMLHttpRequest::setRequestHeader(const QString& name, const QString &value)
+void XMLHttpRequest::setRequestHeader(const QString& _name, const QString &value)
 {
+  QString name = _name.lower();
+
   // Content-type needs to be set seperately from the other headers
-  if(name.lower() == "content-type") {
+  if(name == "content-type") {
     contentType = "Content-type: " + value;
     return;
   }
-  if(name.lower() == "content-length") {
-    return; // Denied - we set it ourselves.
+
+  // Sanitize the referrer header to protect against spoofing...
+  else if(name == "referer") {
+    KURL referrerURL(value);
+    if (urlMatchesDocumentDomain(referrerURL))
+      requestHeaders[name] = referrerURL.url();
+    return;
   }
-  requestHeaders[name] = value;
+
+  // Sanitize the request headers below and handle them as if they are
+  // calls to open. Otherwise, we will end up ignoring them all together!
+  // TODO: Do something about "put" which kio_http sort of supports and
+  // the webDAV headers such as PROPFIND etc...
+  else if (name == "get"  || name == "post") {
+    KURL reqURL (doc->URL(), value.stripWhiteSpace());
+    open(name, reqURL, async);
+    return;
+  }
+
+  // Put all headers we want to ignore here. We ignore these for one reason
+  // or another. NOTE: if you think any of these headers should be supported
+  // make sure you properly sanitize them before hand. See above...
+  else if (name == "authorization" || name == "proxy-authorization" ||
+           name == "content-length" || name == "host" || name == "connect" ||
+           name == "copy" || name == "move" || name == "delete" ||
+           name == "head" || name == "trace" || name == "put" ||
+           name == "propfind" || name == "proppatch" || name == "mkcol" ||
+           name == "lock" || name == "unlock")
+    return;   // Denied
+
+  requestHeaders[name] = value.stripWhiteSpace();
 }
 
 Value XMLHttpRequest::getAllResponseHeaders() const


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