From 0406ab355c2c5d04817f22f67f05c5543829c92f Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Mon, 26 Nov 2018 14:59:10 +0100 Subject: [PATCH] Fix SVACE and C++ issues Change-Id: I34adcda30e84fd0622f912d9ce0288e719c43199 --- server/engine/encryption/dmcrypt-engine.cpp | 24 +++++++++++++++------ server/internal-encryption.cpp | 7 +++--- server/key-manager/key-store.cpp | 21 ++++++++++-------- server/upgrade-support.cpp | 2 +- 4 files changed, 34 insertions(+), 20 deletions(-) diff --git a/server/engine/encryption/dmcrypt-engine.cpp b/server/engine/encryption/dmcrypt-engine.cpp index e8d8d9e..b0a5eae 100644 --- a/server/engine/encryption/dmcrypt-engine.cpp +++ b/server/engine/encryption/dmcrypt-engine.cpp @@ -21,6 +21,8 @@ #include #include +#include + #include #include #include @@ -103,6 +105,8 @@ const std::string createCryptoBlkDev(const std::string &realBlkDev, throw runtime::Exception("Cannot open device-mapper"); } + std::unique_ptr 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(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; } diff --git a/server/internal-encryption.cpp b/server/internal-encryption.cpp index 5e86b0c..0abbf17 100644 --- a/server/internal-encryption.cpp +++ b/server/internal-encryption.cpp @@ -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 { diff --git a/server/key-manager/key-store.cpp b/server/key-manager/key-store.cpp index 497461b..c25e25b 100644 --- a/server/key-manager/key-store.cpp +++ b/server/key-manager/key-store.cpp @@ -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 diff --git a/server/upgrade-support.cpp b/server/upgrade-support.cpp index 11c49b8..27c9aac 100644 --- a/server/upgrade-support.cpp +++ b/server/upgrade-support.cpp @@ -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()); -- 2.34.1