Sanitization

common/ConfigUtil.cpp

@@ -12,11 +12,13 @@
 #include <config.h>
 
 #include <ConfigUtil.hpp>
+
 #include <Util.hpp>
 
 #include <cassert>
 #include <string>
 #include <sstream>
+#include <unordered_map>
 
 #include <Poco/Util/AbstractConfiguration.h>
 #include <Poco/Util/XMLConfiguration.h>
@@ -42,7 +44,7 @@ RuntimeConstant<bool> SslTermination;
 // NOTE: This is sorted, please keep it sorted as it's friendlier to readers,
 //       except for properties, which are sorted before the value, e.g.
 //       "setting[@name]" before "setting", which is more readable.
-static const std::map<std::string, std::string> DefAppConfig = {
+static const std::unordered_map<std::string, std::string> DefAppConfig = {
     { "accessibility.enable", "false" },
     { "admin_console.enable", "true" },
     { "admin_console.enable_pam", "false" },
@@ -319,7 +321,7 @@ void initialize(const std::string& xml)
 
 bool isInitialized() { return Config != nullptr; }
 
-const std::map<std::string, std::string>& getDefaultAppConfig() { return DefAppConfig; }
+const std::unordered_map<std::string, std::string>& getDefaultAppConfig() { return DefAppConfig; }
 
 /// Recursively extract the sub-keys of the given parent key.
 void extract(const std::string& parentKey, const Poco::Util::AbstractConfiguration& config,

common/ConfigUtil.hpp

@@ -26,14 +26,16 @@
 #include <atomic>
 #include <string>
 #include <map>
+#include <unordered_map>
 
 namespace ConfigUtil
 {
 /// Helper class to hold default configuration entries.
 class AppConfigMap final : public Poco::Util::MapConfiguration
 {
 public:
-    AppConfigMap(const std::map<std::string, std::string>& map)
+    AppConfigMap() = default;
+    AppConfigMap(const std::unordered_map<std::string, std::string>& map)
     {
         for (const auto& pair : map)
         {
@@ -109,7 +111,7 @@ void initialize(const Poco::Util::AbstractConfiguration* config);
 bool isInitialized();
 
 /// Returns the default config.
-const std::map<std::string, std::string>& getDefaultAppConfig();
+const std::unordered_map<std::string, std::string>& getDefaultAppConfig();
 
 /// Extract all entries as key-value pairs. We use map to have the entries sorted.
 std::map<std::string, std::string> extractAll(const Poco::Util::AbstractConfiguration& config);

common/FileUtil.hpp

@@ -189,10 +189,10 @@ namespace FileUtil
     {
     public:
         /// Stat the given path. Symbolic links are stat'ed when @link is true.
-        Stat(const std::string& file, bool link = false)
-            : _path(file)
+        Stat(std::string file, bool link = false)
+            : _path(std::move(file))
             , _sb{}
-            , _res(link ? lstat(file.c_str(), &_sb) : stat(file.c_str(), &_sb))
+            , _res(link ? lstat(_path.c_str(), &_sb) : stat(_path.c_str(), &_sb))
             , _stat_errno(errno)
         {
         }
@@ -201,7 +201,7 @@ namespace FileUtil
         bool bad() const { return !good(); }
         const struct ::stat& sb() const { return _sb; }
 
-        const std::string path() const { return _path; }
+        const std::string& path() const { return _path; }
 
         bool isDirectory() const { return S_ISDIR(_sb.st_mode); }
         bool isFile() const { return S_ISREG(_sb.st_mode); }

common/Protocol.hpp

@@ -311,8 +311,8 @@ namespace COOLProtocol
         std::string ret(message, abbrevLen);
         for (size_t i = abbrevLen; i < messageLen; ++i)
         {
-            const uint8_t unit = message[i];
-            const bool continuation = (unit & 0xC0) == 0x80;
+            const char unit = message[i];
+            const bool continuation = (static_cast<uint8_t>(unit) & 0xC0) == 0x80;
             if (!continuation) // likely
                 break;
             ret.push_back(unit);

kit/ForKit.cpp

@@ -406,16 +406,16 @@ static void cleanupChildren()
 
     // Now delete the jails.
     auto i = cleanupJailPaths.size();
-    while (i-- > 0)
+    while (i > 0)
     {
+        --i;
         const std::string path = cleanupJailPaths[i];
 
-        // don't delete jails where there was a crash until it ~3 minutes old
-        std::string noteCrashFile(path + "/tmp/kit-crashed");
-        auto noteStat = FileUtil::Stat(noteCrashFile);
+        // Don't delete jails where there was a crash until it's ~3 minutes old.
+        const FileUtil::Stat noteStat(path + "/tmp/kit-crashed");
         if (noteStat.good())
         {
-            time_t modifiedTimeSec = noteStat.modifiedTimeMs() / 1000;
+            const time_t modifiedTimeSec = noteStat.modifiedTimeMs() / 1000;
             if (time(nullptr) < modifiedTimeSec + 180)
                 continue;
         }

net/Socket.hpp

@@ -1312,7 +1312,7 @@ class StreamSocket : public Socket,
                     _inBuffer.append(&buf[0], len);
                 }
                 // else poll will handle errors.
-            } while (len == (sizeof(buf)));
+            } while (len == static_cast<ssize_t>(sizeof(buf)));
 
             // Restore errno from the read call.
             errno = last_errno;

net/WebSocketHandler.hpp

@@ -749,22 +749,22 @@ class WebSocketHandler : public ProtocolHandlerInterface
         char scratch[16];
 
         // All unfragmented frames must have the Fin bit.
-        scratch[slen++] = WSFrameMask::Fin | flags;
+        scratch[slen++] = static_cast<char>(WSFrameMask::Fin | flags);
 
         int maskFlag = _isMasking ? 0x80 : 0;
         if (len < 126)
         {
-            scratch[slen++] = (char)(len | maskFlag);
+            scratch[slen++] = static_cast<char>(len | maskFlag);
         }
         else if (len <= 0xffff)
         {
-            scratch[slen++] = (char)(126 | maskFlag);
+            scratch[slen++] = static_cast<char>(126 | maskFlag);
             scratch[slen++] = static_cast<char>((len >> 8) & 0xff);
             scratch[slen++] = static_cast<char>((len >> 0) & 0xff);
         }
         else
         {
-            scratch[slen++] = (char)(127 | maskFlag);
+            scratch[slen++] = static_cast<char>(127 | maskFlag);
             scratch[slen++] = static_cast<char>((len >> 56) & 0xff);
             scratch[slen++] = static_cast<char>((len >> 48) & 0xff);
             scratch[slen++] = static_cast<char>((len >> 40) & 0xff);

wsd/Admin.cpp

@@ -674,18 +674,22 @@ void Admin::pollingThread()
             lastNet = now;
         }
 
-        int cleanupWait = _cleanupIntervalMs;
+        std::chrono::milliseconds cleanupWait(_cleanupIntervalMs);
         if (_defDocProcSettings.getCleanupSettings().getEnable())
         {
-            cleanupWait
-                -= std::chrono::duration_cast<std::chrono::milliseconds>(now - lastCleanup).count();
-            if (cleanupWait <= MinStatsIntervalMs / 2) // Close enough
+            if (now > lastCleanup)
+            {
+                cleanupWait -=
+                    std::chrono::duration_cast<std::chrono::milliseconds>(now - lastCleanup);
+            }
+
+            if (cleanupWait <= std::chrono::milliseconds(MinStatsIntervalMs / 2)) // Close enough
             {
                 cleanupResourceConsumingDocs();
                 if (_defDocProcSettings.getCleanupSettings().getLostKitGracePeriod())
                     cleanupLostKits();
 
-                cleanupWait += _cleanupIntervalMs;
+                cleanupWait += std::chrono::milliseconds(_cleanupIntervalMs);
                 lastCleanup = now;
             }
         }
@@ -714,7 +718,7 @@ void Admin::pollingThread()
 
         // Handle websockets & other work.
         const auto timeout = std::chrono::milliseconds(capAndRoundInterval(
-            std::min(std::min(std::min(cpuWait, memWait), netWait), cleanupWait)));
+            std::min<int>(std::min(std::min(cpuWait, memWait), netWait), cleanupWait.count())));
         LOGA_TRC(Admin, "Admin poll for " << timeout);
         poll(timeout); // continue with ms for admin, settings etc.
     }

wsd/COOLWSD.cpp

@@ -192,7 +192,8 @@ static std::chrono::milliseconds careerSpanMs(std::chrono::milliseconds::zero())
 #endif
 
 /// The timeout for a child to spawn, initially high, then reset to the default.
-int ChildSpawnTimeoutMs = CHILD_SPAWN_TIMEOUT_MS;
+std::atomic<std::chrono::milliseconds> ChildSpawnTimeoutMs =
+    std::chrono::milliseconds(CHILD_SPAWN_TIMEOUT_MS);
 std::atomic<unsigned> COOLWSD::NumConnections;
 std::unordered_set<std::string> COOLWSD::EditFileExtensions;
 
@@ -602,7 +603,7 @@ static void rebalanceChildren(const std::string& configId, int balance)
 
     const auto duration = (std::chrono::steady_clock::now() - LastForkRequestTimes[configId]);
     const auto durationMs = std::chrono::duration_cast<std::chrono::milliseconds>(duration);
-    if (OutstandingForks[configId] != 0 && durationMs >= std::chrono::milliseconds(ChildSpawnTimeoutMs))
+    if (OutstandingForks[configId] != 0 && durationMs >= ChildSpawnTimeoutMs.load())
     {
         // Children taking too long to spawn.
         // Forget we had requested any, and request anew.
@@ -660,7 +661,7 @@ static size_t addNewChild(std::shared_ptr<ChildProcess> child)
     {
         // Reset the child-spawn timeout to the default, now that we're set.
         // But only when mounting is enabled. Otherwise, copying is always slow.
-        ChildSpawnTimeoutMs = CHILD_TIMEOUT_MS;
+        ChildSpawnTimeoutMs = std::chrono::milliseconds(CHILD_TIMEOUT_MS);
     }
 
     LOG_TRC("Adding a new child " << pid << " with config " << configId
@@ -890,7 +891,7 @@ std::shared_ptr<ChildProcess> getNewChild_Blocks(SocketPoll &destPoll, const std
 #if !MOBILEAPP
     assert(mobileAppDocId == 0 && "Unexpected to have mobileAppDocId in the non-mobile build");
 
-    int spawnTimeoutMs = ChildSpawnTimeoutMs / 2;
+    std::chrono::milliseconds spawnTimeoutMs = ChildSpawnTimeoutMs.load() / 2;
 
     if (configId.empty() || SubForKitProcs.contains(configId))
     {
@@ -902,11 +903,11 @@ std::shared_ptr<ChildProcess> getNewChild_Blocks(SocketPoll &destPoll, const std
     else
     {
         // configId exists, and no SubForKitProcs for it seen yet, be more generous for startup time.
-        spawnTimeoutMs = CHILD_SPAWN_TIMEOUT_MS;
+        spawnTimeoutMs = std::chrono::milliseconds(CHILD_SPAWN_TIMEOUT_MS);
         LOG_DBG("getNewChild: awaiting subforkit[" << configId << "], timeout of " << spawnTimeoutMs << "ms");
     }
 
-    const auto timeout = std::chrono::milliseconds(spawnTimeoutMs);
+    const std::chrono::milliseconds timeout = spawnTimeoutMs;
     LOG_TRC("Waiting for a new child for a max of " << timeout);
 #else // MOBILEAPP
     const auto timeout = std::chrono::hours(100);
@@ -1273,10 +1274,11 @@ void COOLWSD::innerInitialize(Poco::Util::Application& self)
     // Initialize the config subsystem.
     LayeredConfiguration& conf = config();
 
-    const std::map<std::string, std::string> DefAppConfig = ConfigUtil::getDefaultAppConfig();
+    const std::unordered_map<std::string, std::string> defAppConfig =
+        ConfigUtil::getDefaultAppConfig();
 
     // Set default values, in case they are missing from the config file.
-    Poco::AutoPtr<ConfigUtil::AppConfigMap> defConfig(new ConfigUtil::AppConfigMap(DefAppConfig));
+    Poco::AutoPtr<ConfigUtil::AppConfigMap> defConfig(new ConfigUtil::AppConfigMap(defAppConfig));
     conf.addWriteable(defConfig, PRIO_SYSTEM); // Lowest priority
 
 #if !MOBILEAPP
@@ -1711,7 +1713,7 @@ void COOLWSD::innerInitialize(Poco::Util::Application& self)
 
     // Copy and serialize the config into XML to pass to forkit.
     KitXmlConfig.reset(new Poco::Util::XMLConfiguration);
-    for (const auto& pair : DefAppConfig)
+    for (const auto& pair : defAppConfig)
     {
         try
         {
@@ -2697,7 +2699,7 @@ bool COOLWSD::createForKit()
     SigUtil::addActivity("spawning new forkit");
 
     // Creating a new forkit is always a slow process.
-    ChildSpawnTimeoutMs = CHILD_SPAWN_TIMEOUT_MS;
+    ChildSpawnTimeoutMs = std::chrono::milliseconds(CHILD_SPAWN_TIMEOUT_MS);
 
     std::unique_lock<std::mutex> newChildrenLock(NewChildrenMutex);
 
@@ -3305,7 +3307,7 @@ class COOLWSDServer
            << "\n  NewChildren: " << NewChildren.size() << " (" << NewChildren.capacity() << ')'
            << "\n  OutstandingForks: " << TotalOutstandingForks
            << "\n  NumPreSpawnedChildren: " << COOLWSD::NumPreSpawnedChildren
-           << "\n  ChildSpawnTimeoutMs: " << ChildSpawnTimeoutMs
+           << "\n  ChildSpawnTimeoutMs: " << ChildSpawnTimeoutMs.load()
            << "\n  Document Brokers: " << DocBrokers.size()
 #if !MOBILEAPP
            << "\n  of which ConvertTo: " << ConvertToBroker::getInstanceCount()
@@ -3667,7 +3669,7 @@ int COOLWSD::innerMain()
         else
         {
             int retry = (COOLWSD::NoCapsForKit ? 150 : 50);
-            const auto timeout = std::chrono::milliseconds(ChildSpawnTimeoutMs);
+            const auto timeout = ChildSpawnTimeoutMs.load();
             while (retry-- > 0 && !SigUtil::getShutdownRequestFlag())
             {
                 LOG_INF("Waiting for a new child for a max of " << timeout);

wsd/COOLWSD.hpp

@@ -310,7 +310,7 @@ class COOLWSD final : public Poco::Util::ServerApplication,
     virtual std::string getJailRoot(int pid) override;
 
     /// Settings passed from the command-line to override those in the config file.
-    std::map<std::string, std::string> _overrideSettings;
+    std::unordered_map<std::string, std::string> _overrideSettings;
 
 #if MOBILEAPP
 public:

wsd/ProofKey.cpp

@@ -92,7 +92,7 @@ std::vector<unsigned char> Proof::Base64ToBytes(const std::string &str)
     char c = 0;
     std::vector<unsigned char> vec;
     while (decoder.get(c))
-        vec.push_back(c);
+        vec.push_back(static_cast<unsigned char>(c));
 
     return vec;
 }

wsd/RemoteConfig.hpp

@@ -71,7 +71,7 @@ class RemoteConfigPoll : public RemoteJSONPoll
         : RemoteJSONPoll(config, "remote_config.remote_url", "remoteconfig_poll", "configuration")
     {
         constexpr int PRIO_JSON = -200; // highest priority
-        _persistConfig = new ConfigUtil::AppConfigMap(std::map<std::string, std::string>{});
+        _persistConfig = new ConfigUtil::AppConfigMap();
         _conf.addWriteable(_persistConfig, PRIO_JSON);
     }