From 499aa4ad13b3993ea46682062bd7f3e1a61676cd Mon Sep 17 00:00:00 2001
From: Stephen Birarda <commit@birarda.com>
Date: Wed, 18 Jul 2018 15:17:19 -0700
Subject: [PATCH] 404 malformed agent script requests, check doc root

---
 domain-server/src/DomainServer.cpp            |  9 +++--
 .../embedded-webserver/src/HTTPManager.cpp    | 34 ++++++++++++++-----
 2 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp
index eccd1c1e20..86a9a58058 100644
--- a/domain-server/src/DomainServer.cpp
+++ b/domain-server/src/DomainServer.cpp
@@ -1916,14 +1916,16 @@ bool DomainServer::handleHTTPRequest(HTTPConnection* connection, const QUrl& url
 
         // don't handle if we don't have a matching node
         if (!matchingNode) {
-            return false;
+            connection->respond(HTTPConnection::StatusCode404, "Resource not found.");
+            return true;
         }
 
         auto nodeData = static_cast<DomainServerNodeData*>(matchingNode->getLinkedData());
 
         // don't handle if we don't have node data for this node
         if (!nodeData) {
-            return false;
+            connection->respond(HTTPConnection::StatusCode404, "Resource not found.");
+            return true;
         }
 
         SharedAssignmentPointer matchingAssignment = _allAssignments.value(nodeData->getAssignmentUUID());
@@ -1944,7 +1946,8 @@ bool DomainServer::handleHTTPRequest(HTTPConnection* connection, const QUrl& url
         }
 
         // request not handled
-        return false;
+        connection->respond(HTTPConnection::StatusCode404, "Resource not found.");
+        return true;
     }
 
     // check if this is a request for our domain ID
diff --git a/libraries/embedded-webserver/src/HTTPManager.cpp b/libraries/embedded-webserver/src/HTTPManager.cpp
index 4a75994e12..ccebeaf9cc 100644
--- a/libraries/embedded-webserver/src/HTTPManager.cpp
+++ b/libraries/embedded-webserver/src/HTTPManager.cpp
@@ -48,6 +48,13 @@ void HTTPManager::incomingConnection(qintptr socketDescriptor) {
 }
 
 bool HTTPManager::handleHTTPRequest(HTTPConnection* connection, const QUrl& url, bool skipSubHandler) {
+    // Reject paths with embedded NULs
+    if (url.path().contains(QChar(0x00))) {
+        connection->respond(HTTPConnection::StatusCode400, "Embedded NULs not allowed in requests");
+        qCWarning(embeddedwebserver) << "Received a request with embedded NULs";
+        return true;
+    }
+
     if (!skipSubHandler && requestHandledByRequestHandler(connection, url)) {
         // this request was handled by our request handler object
         // so we don't need to attempt to do so in the document root
@@ -57,17 +64,27 @@ bool HTTPManager::handleHTTPRequest(HTTPConnection* connection, const QUrl& url,
     if (!_documentRoot.isEmpty()) {
         // check to see if there is a file to serve from the document root for this path
         QString subPath = url.path();
-        
+
         // remove any slash at the beginning of the path
         if (subPath.startsWith('/')) {
             subPath.remove(0, 1);
         }
-        
+
+        QString absoluteDocumentRoot { QFileInfo(_documentRoot).absolutePath() };
         QString filePath;
-        
-        if (QFileInfo(_documentRoot + subPath).isFile()) {
-            filePath = _documentRoot + subPath;
-        } else if (subPath.size() > 0 && !subPath.endsWith('/')) {
+        QFileInfo pathFileInfo { _documentRoot + subPath };
+        QString absoluteFilePath { pathFileInfo.absoluteFilePath() };
+
+        // The absolute path for this file isn't under the document root
+        if (absoluteFilePath.indexOf(absoluteDocumentRoot) != 0) {
+            qCWarning(embeddedwebserver) << absoluteFilePath << "is outside the document root";
+            connection->respond(HTTPConnection::StatusCode400, "Requested path outside document root");
+            return true;
+        }
+
+        if (pathFileInfo.isFile()) {
+            filePath = absoluteFilePath;
+        } else if (subPath.size() > 0 && !subPath.endsWith('/') && pathFileInfo.isDir()) {
             // this could be a directory with a trailing slash
             // send a redirect to the path with a slash so we can
             QString redirectLocation = '/' + subPath + '/';
@@ -80,6 +97,7 @@ bool HTTPManager::handleHTTPRequest(HTTPConnection* connection, const QUrl& url,
             redirectHeader.insert(QByteArray("Location"), redirectLocation.toUtf8());
             
             connection->respond(HTTPConnection::StatusCode302, "", HTTPConnection::DefaultContentType, redirectHeader);
+            return true;
         }
         
         // if the last thing is a trailing slash then we want to look for index file
@@ -87,8 +105,8 @@ bool HTTPManager::handleHTTPRequest(HTTPConnection* connection, const QUrl& url,
             QStringList possibleIndexFiles = QStringList() << "index.html" << "index.shtml";
             
             foreach (const QString& possibleIndexFilename, possibleIndexFiles) {
-                if (QFileInfo(_documentRoot + subPath + possibleIndexFilename).exists()) {
-                    filePath = _documentRoot + subPath + possibleIndexFilename;
+                if (QFileInfo(absoluteFilePath + possibleIndexFilename).exists()) {
+                    filePath = absoluteFilePath + possibleIndexFilename;
                     break;
                 }
             }