[Libreoffice-commits] online.git: loolwsd/LOOLKit.cpp

[Tor Lillqvist]

 loolwsd/LOOLKit.cpp |   20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

New commits:
commit 3aadd910c6e32c0e557671effa5a4c606cd6e8bf
Author: Tor Lillqvist <tml at collabora.com>
Date:   Mon Apr 18 13:11:30 2016 +0300

    We can't remove jailPath when inside the chroot as it does not exist there
    
    When inside the chroot, what we would need to do is remove everything
    below / . But doing that is a bit too risky, in case some developer
    screws up some detail and that code happens to run outside the chroot
    after all, and the developer's machine gets trashed. So just remove
    paths we can reasonably assume won't exist as global pathnames on a
    developer machine: loSubPath and JAILED_DOCUMENT_ROOT.
    
    Currently the actual complete cleanup of loolkit jails happens in
    loolwsd when it is exiting. That is a bug and will have to be
    fixed. It should be done in loolforkit as soon as possible after the
    loolkit process has exited.

diff --git a/loolwsd/LOOLKit.cpp b/loolwsd/LOOLKit.cpp
index 4a2f94d..8a4b8e4 100644
--- a/loolwsd/LOOLKit.cpp
+++ b/loolwsd/LOOLKit.cpp
@@ -1054,10 +1054,26 @@ void lokit_main(const std::string& childRoot,
                     return TerminationFlag;
                 });
 
-        // Cleanup a jail if we created one
+        // Clean up jail if we created one
         if (bRunInsideJail && !jailPath.isRelative())
         {
-            Util::removeFile(jailPath, true);
+            // In theory we should here do Util::removeFile("/", true), because we are inside the
+            // chroot jail, and all of it can be removed now when we are exiting. (At least the root
+            // of the chroot jail probably would not be removed even if we tried, so we still would
+            // need to complete the cleanup in loolforkit.)
+
+            // But: It is way too risky to actually do that (effectively, "rm -rf /") as it would
+            // trash a developer's machine if something goes wrong while hacking and debugging and
+            // the process isn't in a chroot after all when it comes here.
+
+            // So just remove what we can reasonably safely assume won't exist as global pathnames
+            // on a developer's machine, loSubpath (typically "/lo") and JAILED_DOCUMENT_ROOT
+            // ("/user/docs/").
+
+            Log::info("Removing '/" + loSubPath + "'");
+            Util::removeFile("/" + loSubPath, true);
+            Log::info("Removing '" + std::string(JAILED_DOCUMENT_ROOT) + "'");
+            Util::removeFile(std::string(JAILED_DOCUMENT_ROOT), true);
         }
     }
     catch (const Exception& exc)