From kfm-devel Thu Oct 13 00:35:52 2005 From: Dawit Alemayehu Date: Thu, 13 Oct 2005 00:35:52 +0000 To: kfm-devel Subject: PATCH: XMLHttpRequest Message-Id: <200510122035.53336.adawit () kde ! org> X-MARC-Message: https://marc.info/?l=kfm-devel&m=112916380707309 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--Boundary-00=_pvaTDQB3AlJIbyx" --Boundary-00=_pvaTDQB3AlJIbyx Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline 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" --Boundary-00=_pvaTDQB3AlJIbyx Content-Type: text/x-diff; charset="iso-8859-1"; name="xmlhttprequest.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="xmlhttprequest.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 --Boundary-00=_pvaTDQB3AlJIbyx--