Fix SVACE and C++ issues 14/193814/3
authorKrzysztof Jackiewicz <k.jackiewicz@samsung.com>
Mon, 26 Nov 2018 13:59:10 +0000 (14:59 +0100)
committerKrzysztof Jackiewicz <k.jackiewicz@samsung.com>
Mon, 3 Dec 2018 14:38:08 +0000 (14:38 +0000)
Change-Id: I34adcda30e84fd0622f912d9ce0288e719c43199

server/engine/encryption/dmcrypt-engine.cpp
server/internal-encryption.cpp
server/key-manager/key-store.cpp
server/upgrade-support.cpp

index e8d8d9e..b0a5eae 100644 (file)
@@ -21,6 +21,8 @@
 #include <fcntl.h>
 #include <errno.h>
 
+#include <memory>
+
 #include <klay/error.h>
 #include <klay/exception.h>
 #include <klay/filesystem.h>
@@ -103,6 +105,8 @@ const std::string createCryptoBlkDev(const std::string &realBlkDev,
                throw runtime::Exception("Cannot open device-mapper");
        }
 
+       std::unique_ptr<int, void(*)(int*)> fdPtr(&fd, [](int* fd){ if (fd) ::close(*fd); });
+
        /*
         * dmBuf         |-------------------------------------------------| DM_MAX_BUFFER_SIZE(4096)
         * dmIo          |----------| size of dm_ioctl
@@ -112,14 +116,12 @@ const std::string createCryptoBlkDev(const std::string &realBlkDev,
        // Create Device (mount_name)
        initDMIoctl(dmBuf, DM_MAX_BUFFER_SIZE, mountName, 0);
        if (ioctl(fd, DM_DEV_CREATE, dmBuf)) {
-               close(fd);
                throw runtime::Exception("Cannot create dm-crypt device");
        }
 
        // Get the device status, in particular, the mount_name of it's device file
        initDMIoctl(dmBuf, DM_MAX_BUFFER_SIZE, mountName, 0);
        if (ioctl(fd, DM_DEV_STATUS, dmBuf)) {
-               close(fd);
                throw runtime::Exception("Cannot retrieve dm-crypt device status");
        }
 
@@ -154,7 +156,19 @@ const std::string createCryptoBlkDev(const std::string &realBlkDev,
        char *cryptParams = dmBuf + sizeof(struct dm_ioctl) + sizeof(struct dm_target_spec);
 
        // Store cryptParams
-       snprintf(cryptParams, DM_MAX_BUFFER_SIZE - (cryptParams - dmBuf), "%s %s 0 %s 0", cryptoTypeName.c_str(), convertToHex(key).c_str(), realBlkDev.c_str());
+       size_t cryptParamsSize = DM_MAX_BUFFER_SIZE - (cryptParams - dmBuf);
+       int ret = snprintf(cryptParams,
+                          cryptParamsSize,
+                          "%s %s 0 %s 0",
+                          cryptoTypeName.c_str(),
+                          convertToHex(key).c_str(),
+                          realBlkDev.c_str());
+       if (ret < 0) {
+               throw runtime::Exception("snprintf() failed");
+       }
+       if (static_cast<size_t>(ret) >= cryptParamsSize) {
+               throw runtime::Exception("Crypto params didn't fit the device mapper buffer");
+       }
 
        cryptParams += strlen(cryptParams) + 1;
        // Align to an 8 byte boundary
@@ -163,19 +177,15 @@ const std::string createCryptoBlkDev(const std::string &realBlkDev,
 
        // Table load
        if (ioctl(fd, DM_TABLE_LOAD, dmBuf) < 0) {
-               close(fd);
                throw runtime::Exception("Cannot load dm-crypt mapping table.");
        }
 
        // Resume this device to activate it
        initDMIoctl(dmBuf, DM_MAX_BUFFER_SIZE, mountName, 0);
        if (ioctl(fd, DM_DEV_SUSPEND, dmBuf)) {
-               close(fd);
                throw runtime::Exception("Cannot resume the dm-crypt device");
        }
 
-       close(fd);
-
        return cryptoBlkDev;
 }
 
index 5e86b0c..0abbf17 100644 (file)
@@ -52,6 +52,7 @@ namespace ode {
 namespace {
 
 const char *PRIVILEGE_PLATFORM = "http://tizen.org/privilege/internal/default/platform";
+const mode_t MODE_0640 = S_IRUSR | S_IWUSR | S_IRGRP;
 
 // watches systemd jobs
 class JobWatch {
@@ -408,7 +409,7 @@ int InternalEncryptionServer::encrypt(const std::string& password, unsigned int
                        ::sleep(1);
 
                        runtime::File file("/opt/etc/.odeprogress");
-                       file.create(0640);
+                       file.create(MODE_0640);
 
                        INFO(SINK, "Closing all known systemd services that might be using internal storage.");
                        stopKnownSystemdUnits();
@@ -467,7 +468,7 @@ int InternalEncryptionServer::decrypt(const std::string& password)
                        ::sleep(1);
 
                        runtime::File file("/opt/etc/.odeprogress");
-                       file.create(0640);
+                       file.create(MODE_0640);
 
                        if (engine->isMounted()) {
                                INFO(SINK, "Closing all known systemd services that might be using internal storage.");
@@ -520,7 +521,7 @@ int InternalEncryptionServer::recovery()
                Ext4Tool::mkfs(engine->getSource());
 
        runtime::File file("/opt/.factoryreset");
-       file.create(0640);
+       file.create(MODE_0640);
 
        ::sync();
        try {
index 497461b..c25e25b 100644 (file)
@@ -71,23 +71,26 @@ const std::string KeyStore::getHashSpec() const
 
 void KeyStore::setCipherName(const std::string &name)
 {
-       char *buf = (char *)luks.cipherName;
-       ::strncpy(buf, name.c_str(), sizeof(luks.cipherName) - 2);
-       buf[sizeof(luks.cipherName) - 1] = '\0';
+       if (name.size() >= sizeof(luks.cipherName)) {
+               throw runtime::Exception("Cipher name is too long");
+       }
+       ::strcpy(luks.cipherName, name.c_str());
 }
 
 void KeyStore::setCipherMode(const std::string &mode)
 {
-       char *buf = (char *)luks.cipherMode;
-       ::strncpy(buf, mode.c_str(), sizeof(luks.cipherMode) - 2);
-       buf[sizeof(luks.cipherMode) - 1] = '\0';
+       if (mode.size() >= sizeof(luks.cipherMode)) {
+               throw runtime::Exception("Cipher mode is too long");
+       }
+       ::strcpy(luks.cipherMode, mode.c_str());
 }
 
 void KeyStore::setHashSpec(const std::string &spec)
 {
-       char *buf = (char *)luks.hashSpec;
-       ::strncpy(buf, spec.c_str(), sizeof(luks.hashSpec) - 2);
-       buf[sizeof(luks.hashSpec) - 1] = '\0';
+       if (spec.size() >= sizeof(luks.hashSpec)) {
+               throw runtime::Exception("Hash spec is too long");
+       }
+       ::strcpy(luks.hashSpec, spec.c_str());
 }
 
 unsigned int KeyStore::getMasterKeyLength() const
index 11c49b8..27c9aac 100644 (file)
@@ -184,7 +184,7 @@ void writeToken(runtime::File &file, const BinaryData& token)
 {
        size_t tokenSize(token.size());
 
-       file.create(0600);
+       file.create(S_IRUSR | S_IWUSR);
 
        file.write(&tokenSize, sizeof(tokenSize));
        file.write(token.data(), token.size());