[Libreoffice-commits] online.git: 3 commits - loleaflet/src loolwsd/LOOLWSD.cpp

[Pranav Kant]

 loleaflet/src/layer/tile/TileLayer.js |    8 +++--
 loolwsd/LOOLWSD.cpp                   |   54 ++++++++++++++++++++++++----------
 2 files changed, 45 insertions(+), 17 deletions(-)

New commits:
commit 7e57ae5dce1a3ed7e1f8951b4fc73f2fee5c10ec
Author: Pranav Kant <pranavk at collabora.co.uk>
Date:   Mon Oct 3 11:16:09 2016 +0530

    loleaflet: Don't add these layers in readonly mode
    
    Change-Id: I17437f53921dc3d5036be4a9650213a7eb0895dd

diff --git a/loleaflet/src/layer/tile/TileLayer.js b/loleaflet/src/layer/tile/TileLayer.js
index 061db21..62ee1a9 100644
--- a/loleaflet/src/layer/tile/TileLayer.js
+++ b/loleaflet/src/layer/tile/TileLayer.js
@@ -144,11 +144,15 @@ L.TileLayer = L.GridLayer.extend({
 		this._initContainer();
 		this._getToolbarCommandsValues();
 		this._selections = new L.LayerGroup();
-		map.addLayer(this._selections);
+		if (this.options.permission !== 'readonly') {
+			map.addLayer(this._selections);
+		}
 
 		// This layergroup contains all the layers corresponding to other's view
 		this._viewLayerGroup = new L.LayerGroup();
-		map.addLayer(this._viewLayerGroup);
+		if (this.options.permission !== 'readonly') {
+			map.addLayer(this._viewLayerGroup);
+		}
 
 		this._debug = map.options.debug;
 		this._debugInit();
commit 3fa2115fa18ca8b53a9f3f4659eb0f280c0f5125
Author: Pranav Kant <pranavk at collabora.co.uk>
Date:   Mon Oct 3 10:51:20 2016 +0530

    loolwsd: security: Cleanup HTTP download request
    
    Sanitize for some funny inputs.
    
    Change-Id: I450cb5ed6e03e9809308e8f763af2c2a66fcecb0

diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 5372b19..77a0399 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -555,15 +555,8 @@ private:
         {
             Log::info("File download request.");
             //TODO: Check that the user in question has access to this file!
-            const std::string dirPath = LOOLWSD::ChildRoot + tokens[3]
-                                      + JAILED_DOCUMENT_ROOT + tokens[4];
-            std::string fileName;
-            URI::decode(tokens[5], fileName);
-            const std::string filePath = dirPath + "/" + fileName;
-            Log::info("HTTP request for: " + filePath);
-            File file(filePath);
 
-            // Validate the dockey
+            // 1. Validate the dockey
             std::string decodedUri;
             URI::decode(tokens[2], decodedUri);
             const auto docKey = DocumentBroker::getDocKey(DocumentBroker::sanitizeURI(decodedUri));
@@ -573,24 +566,55 @@ private:
             {
                 throw BadRequestException("DocKey [" + docKey + "] is invalid.");
             }
+
+            // 2. Cross-check if received child id is correct
+            if (docBrokerIt->second->getJailId() != tokens[3])
+            {
+                throw BadRequestException("ChildId does not correspond to docKey");
+            }
+
+            // 3. Don't let user download the file in main doc directory containing
+            // the document being edited otherwise we will end up deleting main directory
+            // after download finishes
+            if (docBrokerIt->second->getJailId() == tokens[4])
+            {
+                throw BadRequestException("RandomDir cannot be equal to ChildId");
+            }
             docBrokersLock.unlock();
 
-            if (file.exists())
+            std::string fileName;
+            bool responded = false;
+            URI::decode(tokens[5], fileName);
+            const Path filePath(LOOLWSD::ChildRoot + tokens[3]
+                                + JAILED_DOCUMENT_ROOT + tokens[4] + "/" + fileName);
+            Log::info("HTTP request for: " + filePath.toString());
+            if (filePath.isAbsolute() && File(filePath).exists())
             {
                 response.set("Access-Control-Allow-Origin", "*");
                 HTMLForm form(request);
                 const std::string mimeType = form.has("mime_type")
                                            ? form.get("mime_type")
                                            : "application/octet-stream";
-                response.sendFile(filePath, mimeType);
-                //TODO: Cleanup on error.
-                Util::removeFile(dirPath, true);
-                return true;
+                try
+                {
+                    response.sendFile(filePath.toString(), mimeType);
+                    responded = true;
+                }
+                catch (const Exception& exc)
+                {
+                    Log::error() << "Error sending file to client. PocoException: " << exc.displayText()
+                                 << (exc.nested() ? " (" + exc.nested()->displayText() + ")" : "")
+                                 << Log::end;
+                }
+
+                Util::removeFile(File(filePath.parent()).path(), true);
             }
             else
             {
-                Log::error("Download file [" + filePath + "] not found.");
+                Log::error("Download file [" + filePath.toString() + "] not found.");
             }
+
+            return responded;
         }
 
         throw BadRequestException("Invalid or unknown request.");
commit 700a310d359541191554ecad9c6608617cd92eb1
Author: Pranav Kant <pranavk at collabora.co.uk>
Date:   Sat Oct 1 14:59:57 2016 +0530

    loolwsd: Fix incorrect log messsage
    
    This can be a POST request too.
    
    Change-Id: I673fb94fc8a4b0cc09ed166f04dbad94ca2a041a

diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index a54378a..5372b19 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -938,7 +938,7 @@ public:
         const auto id = LOOLWSD::GenSessionId();
 
         Poco::URI requestUri(request.getURI());
-        Log::debug("Handling GET: " + request.getURI());
+        Log::debug("Handling: " + request.getURI());
 
         StringTokenizer reqPathTokens(request.getURI(), "/?", StringTokenizer::TOK_IGNORE_EMPTY | StringTokenizer::TOK_TRIM);