Fix various issues reported by address sanitizer reported from the unit tests

src/common/commonutils/CommandUtils.c

@@ -285,6 +285,7 @@ int ExecuteCommand(void* context, const char* command, bool replaceEol, bool for
     // Read the text result from the output of the command, if any, whether command succeeded or failed
     if (NULL != textResult)
     {
+        *textResult = NULL;
         resultsFile = fopen(commandTextResultFile, "r");
         if (resultsFile)
         {
@@ -339,7 +340,7 @@ int ExecuteCommand(void* context, const char* command, bool replaceEol, bool for
         OsConfigLogInfo(log, "Context: '%p'", context);
         OsConfigLogInfo(log, "Command: '%s'", command);
         OsConfigLogInfo(log, "Status: %d (errno: %d)", status, errno);
-        OsConfigLogInfo(log, "Text result: '%s'", (NULL != textResult) ? (*textResult) : "");
+        OsConfigLogInfo(log, "Text result: '%s'", (NULL != textResult && NULL != *textResult) ? (*textResult) : "");
     }
 
     return status;
@@ -377,5 +378,6 @@ char* HashCommand(const char* source, void* log)
         OsConfigLogError(log, "HashCommand: out of memory");
     }
 
+    FREE_MEMORY(command);
     return (0 == status) ? hash : NULL;
 }

src/common/commonutils/DeviceInfoUtils.c

@@ -26,7 +26,7 @@ void RemovePrefixBlanks(char* target)
         i += 1;
     }
 
-    memcpy(target, target + i, targetLength - i);
+    memmove(target, target + i, targetLength - i);
     target[targetLength - i] = 0;
 }
 
@@ -43,7 +43,7 @@ void RemovePrefixUpTo(char* target, char marker)
     if (equalSign)
     {
         targetLength = strlen(equalSign + 1);
-        memcpy(target, equalSign + 1, targetLength);
+        memmove(target, equalSign + 1, targetLength);
         target[targetLength] = 0;
     }
 }
@@ -61,7 +61,7 @@ void RemovePrefixUpToString(char* target, const char* marker)
     if (equalSign)
     {
         targetLength = (int)strlen(equalSign);
-        memcpy(target, equalSign, targetLength);
+        memmove(target, equalSign, targetLength);
         target[targetLength] = 0;
     }
 }
@@ -354,6 +354,7 @@ long GetTotalMemory(void* log)
     if (NULL != textResult)
     {
         totalMemory = atol(textResult);
+        FREE_MEMORY(textResult);
     }
 
     if (IsFullLoggingEnabled())
@@ -373,6 +374,7 @@ long GetFreeMemory(void* log)
     if (NULL != textResult)
     {
         freeMemory = atol(textResult);
+        FREE_MEMORY(textResult);
     }
 
     if (IsFullLoggingEnabled())
@@ -391,6 +393,7 @@ char* GetProductName(void* log)
 
     if ((NULL == textResult) || (0 == strlen(textResult)))
     {
+        FREE_MEMORY(textResult);
         textResult = GetHardwareProperty(osProductNameAlternateCommand, false, log);
     }
 
@@ -410,6 +413,7 @@ char* GetProductVendor(void* log)
 
     if ((NULL == textResult) || (0 == strlen(textResult)))
     {
+        FREE_MEMORY(textResult);
         textResult = GetHardwareProperty(osProductVendorAlternateCommand, false, log);
     }
 
@@ -429,6 +433,7 @@ char* GetProductVersion(void* log)
 
     if ((NULL == textResult) || (0 == strlen(textResult)))
     {
+        FREE_MEMORY(textResult);
         textResult = GetHardwareProperty(osProductVersionAlternateCommand, false, log);
     }
 
@@ -697,6 +702,8 @@ int CheckLoginUmask(const char* desired, char** reason, void* log)
             OsConfigCaptureReason(reason, "Current login UMASK '%s' does not match desired '%s'", current, desired);
             status = ENOENT;
         }
+
+        FREE_MEMORY(current);
     }
 
     return status;

src/common/commonutils/FileUtils.c

@@ -309,6 +309,11 @@ bool ConcatenateFiles(const char* firstFileName, const char* secondFileName, boo
 
 int RestrictFileAccessToCurrentAccountOnly(const char* fileName)
 {
+    if (NULL == fileName)
+    {
+        return EINVAL;
+    }
+
     // S_ISUID (4000): Set user ID on execution
     // S_ISGID (2000): Set group ID on execution
     // S_IRUSR (0400): Read permission, owner
@@ -1547,14 +1552,13 @@ static int FindTextInCommandOutput(const char* command, const char* text, void*
             status = ENOENT;
             OsConfigLogInfo(log, "FindTextInCommandOutput: '%s' not found in '%s' output", text, command);
         }
-
-        FREE_MEMORY(results);
     }
     else
     {
         OsConfigLogInfo(log, "FindTextInCommandOutput: command '%s' failed with %d", command, status);
     }
 
+    FREE_MEMORY(results);
     return status;
 }
 
@@ -1630,10 +1634,9 @@ char* GetStringOptionFromBuffer(const char* buffer, const char* option, char sep
         {
             OsConfigLogError(log, "GetStringOptionFromBuffer: failed to duplicate result string (%d)", errno);
         }
-
-        FREE_MEMORY(internal);
     }
 
+    FREE_MEMORY(internal);
     return result;
 }
 

src/common/commonutils/PassUtils.c

@@ -62,6 +62,7 @@ static char* FindPamModule(const char* pamModule, void* log)
 int CheckEnsurePasswordReuseIsLimited(int remember, char** reason, void* log)
 {
     int status = ENOENT;
+    char* pamModule = NULL;
 
     if (0 == CheckFileExists(g_etcPamdCommonPassword, NULL, log))
     {
@@ -81,9 +82,13 @@ int CheckEnsurePasswordReuseIsLimited(int remember, char** reason, void* log)
             g_etcPamdCommonPassword, g_etcPamdSystemAuth, g_remember);
     }
 
-    if (status && (false == FindPamModule(g_pamUnixSo, log)))
+    if (status)
     {
-        OsConfigCaptureReason(reason, "The PAM module '%s' is not available. Automatic remediation is not possible", g_pamUnixSo);
+        if (NULL == (pamModule = FindPamModule(g_pamUnixSo, log)))
+        {
+            OsConfigCaptureReason(reason, "The PAM module '%s' is not available. Automatic remediation is not possible", g_pamUnixSo);
+        }
+        FREE_MEMORY(pamModule);
     }
 
     return status;
@@ -219,7 +224,7 @@ int CheckLockoutForFailedPasswordAttempts(const char* fileName, const char* pamS
                 continue;
             }
             else if ((NULL != strstr(line, auth)) && (NULL != strstr(line, pamSo)) && 
-                (NULL != (authValue = GetStringOptionFromBuffer(line, auth, ' ', log))) && (0 == strcmp(authValue, required)) && FreeAndReturnTrue(authValue) &&
+                (NULL != (authValue = GetStringOptionFromBuffer(line, auth, ' ', log))) && (0 == strcmp(authValue, required)) &&
                 (0 <= (deny = GetIntegerOptionFromBuffer(line, "deny", '=', log))) && (deny <= 5) &&
                 (0 < (unlockTime = GetIntegerOptionFromBuffer(line, "unlock_time", '=', log))))
             {
@@ -229,10 +234,12 @@ int CheckLockoutForFailedPasswordAttempts(const char* fileName, const char* pamS
                     auth, required, pamSo, deny, unlockTime, fileName);
 
                 status = 0;
+                FREE_MEMORY(authValue);
                 break;
             }
             else
             {
+                FREE_MEMORY(authValue);
                 status = ENOENT;
             }
 

src/common/commonutils/SshUtils.c

@@ -275,7 +275,7 @@ static int IsSshConfigIncludeSupported(void* log)
 
 static int CheckOnlyApprovedMacAlgorithmsAreUsed(const char* macs, char** reason, void* log)
 {
-    char* sshMacs = DuplicateStringToLowercase(g_sshMacs);
+    char* sshMacs = NULL;
     char* macsValue = NULL;
     char* value = NULL;
     size_t macsValueLength = 0;
@@ -291,6 +291,11 @@ static int CheckOnlyApprovedMacAlgorithmsAreUsed(const char* macs, char** reason
     {
         return status;
     }
+    else if (NULL == (sshMacs = DuplicateStringToLowercase(g_sshMacs)))
+    {
+        OsConfigLogError(log, "CheckOnlyApprovedMacAlgorithmsAreUsed: failed to duplicate string to lowercase");
+        return ENOMEM;
+    }
 
     if (NULL == (macsValue = GetSshServerState(sshMacs, log)))
     {
@@ -343,7 +348,7 @@ static int CheckOnlyApprovedMacAlgorithmsAreUsed(const char* macs, char** reason
 
 static int CheckAppropriateCiphersForSsh(const char* ciphers, char** reason, void* log)
 {
-    char* sshCiphers = DuplicateStringToLowercase(g_sshCiphers);
+    char* sshCiphers = NULL;
     char* ciphersValue = NULL;
     char* value = NULL;
     size_t ciphersValueLength = 0;
@@ -359,6 +364,11 @@ static int CheckAppropriateCiphersForSsh(const char* ciphers, char** reason, voi
     {
         return status;
     }
+    else if (NULL == (sshCiphers = DuplicateStringToLowercase(g_sshCiphers)))
+    {
+        OsConfigLogError(log, "CheckAppropriateCiphersForSsh: failed to duplicate string to lowercase");
+        return ENOMEM;
+    }
 
     if (NULL == (ciphersValue = GetSshServerState(sshCiphers, log)))
     {

src/common/commonutils/UrlUtils.c

@@ -12,7 +12,7 @@ char* UrlEncode(char* target)
 
     int i = 0, j = 0;
     int targetLength = (int)strlen(target);
-    int encodedLength = 3 * targetLength;
+    int encodedLength = (3 * targetLength) + 1;
     char* encodedTarget = (char*)malloc(encodedLength);
     if (NULL != encodedTarget)
     {

src/modules/commandrunner/tests/CommandRunnerTests.cpp

@@ -127,6 +127,7 @@ namespace Tests
 
         EXPECT_EQ(MMI_OK, m_commandRunner->Get(m_component, m_reportedObject, &reportedPayload, &payloadSizeBytes));
         EXPECT_TRUE(IsJsonEq(Command::Status::Serialize(status), std::string(reportedPayload, payloadSizeBytes)));
+        delete[] reportedPayload;
     }
 
     TEST_F(CommandRunnerTests, RunCommandTimeout)
@@ -145,6 +146,7 @@ namespace Tests
 
         EXPECT_EQ(MMI_OK, m_commandRunner->Get(m_component, m_reportedObject, &reportedPayload, &payloadSizeBytes));
         EXPECT_TRUE(IsJsonEq(Command::Status::Serialize(status), std::string(reportedPayload, payloadSizeBytes)));
+        delete[] reportedPayload;
     }
 
     TEST_F(CommandRunnerTests, RunCommandSingleLineTextResult)
@@ -163,6 +165,7 @@ namespace Tests
 
         EXPECT_EQ(MMI_OK, m_commandRunner->Get(m_component, m_reportedObject, &reportedPayload, &payloadSizeBytes));
         EXPECT_TRUE(IsJsonEq(Command::Status::Serialize(status), std::string(reportedPayload, payloadSizeBytes)));
+        delete[] reportedPayload;
     }
 
     TEST_F(CommandRunnerTests, RunCommandLimitedPayloadSize)
@@ -190,6 +193,7 @@ namespace Tests
 
         EXPECT_EQ(MMI_OK, commandRunner.Get(m_component, m_reportedObject, &reportedPayload, &payloadSizeBytes));
         EXPECT_TRUE(IsJsonEq(Command::Status::Serialize(status), std::string(reportedPayload, payloadSizeBytes)));
+        delete[] reportedPayload;
     }
 
     TEST_F(CommandRunnerTests, RunCommandMaximumCacheSize)
@@ -222,6 +226,7 @@ namespace Tests
             EXPECT_EQ(MMI_OK, m_commandRunner->Set(m_component, m_desiredObject, (MMI_JSON_STRING)(refresh.c_str()), refresh.size()));
             EXPECT_EQ(MMI_OK, m_commandRunner->Get(m_component, m_reportedObject, &reportedPayload, &payloadSizeBytes));
             EXPECT_TRUE(IsJsonEq(Command::Status::Serialize(expectedResult.second), std::string(reportedPayload, payloadSizeBytes)));
+            delete[] reportedPayload;
         }
 
         // Add one more command to the cache
@@ -237,6 +242,7 @@ namespace Tests
         // Get the last command from the cache
         EXPECT_EQ(MMI_OK, m_commandRunner->Get(m_component, m_reportedObject, &reportedPayload, &payloadSizeBytes));
         EXPECT_TRUE(IsJsonEq(Command::Status::Serialize(lastStatus), std::string(reportedPayload, payloadSizeBytes)));
+        delete[] reportedPayload;
 
         // The first command should have been removed from the cache
         Command::Arguments refreshFirstCommand(expectedResults[0].first, "", Command::Action::RefreshCommandStatus, 0, false);
@@ -246,6 +252,7 @@ namespace Tests
         // The last command should still be reported (set as command to report) and in the cache
         EXPECT_EQ(MMI_OK, m_commandRunner->Get(m_component, m_reportedObject, &reportedPayload, &payloadSizeBytes));
         EXPECT_TRUE(IsJsonEq(Command::Status::Serialize(lastStatus), std::string(reportedPayload, payloadSizeBytes)));
+        delete[] reportedPayload;
     }
 
     TEST_F(CommandRunnerTests, RefreshCommand)
@@ -273,6 +280,7 @@ namespace Tests
         // The last run command should be reported
         EXPECT_EQ(MMI_OK, m_commandRunner->Get(m_component, m_reportedObject, &reportedPayload, &payloadSizeBytes));
         EXPECT_TRUE(IsJsonEq(Command::Status::Serialize(status2), std::string(reportedPayload, payloadSizeBytes)));
+        delete[] reportedPayload;
 
         // Refresh the command
         Command::Arguments refresh(id1, "", Command::Action::RefreshCommandStatus, 0, false);
@@ -284,6 +292,7 @@ namespace Tests
         // The refreshed command should be reported
         EXPECT_EQ(MMI_OK, m_commandRunner->Get(m_component, m_reportedObject, &reportedPayload, &payloadSizeBytes));
         EXPECT_TRUE(IsJsonEq(Command::Status::Serialize(status1), std::string(reportedPayload, payloadSizeBytes)));
+        delete[] reportedPayload;
     }
 
     TEST_F(CommandRunnerTests, CancelCommandInProgress)
@@ -306,6 +315,7 @@ namespace Tests
 
         EXPECT_EQ(MMI_OK, m_commandRunner->Get(m_component, m_reportedObject, &reportedPayload, &payloadSizeBytes));
         EXPECT_TRUE(IsJsonEq(Command::Status::Serialize(status), std::string(reportedPayload, payloadSizeBytes)));
+        delete[] reportedPayload;
     }
 
     TEST_F(CommandRunnerTests, RepeatCommandId)
@@ -327,6 +337,7 @@ namespace Tests
 
         EXPECT_EQ(MMI_OK, m_commandRunner->Get(m_component, m_reportedObject, &reportedPayload, &payloadSizeBytes));
         EXPECT_TRUE(IsJsonEq(Command::Status::Serialize(status), std::string(reportedPayload, payloadSizeBytes)));
+        delete[] reportedPayload;
     }
 
     TEST_F(CommandRunnerTests, RepeatCommand)
@@ -346,6 +357,7 @@ namespace Tests
 
         EXPECT_EQ(MMI_OK, m_commandRunner->Get(m_component, m_reportedObject, &reportedPayload, &payloadSizeBytes));
         EXPECT_TRUE(IsJsonEq(Command::Status::Serialize(status), std::string(reportedPayload, payloadSizeBytes)));
+        delete[] reportedPayload;
     }
 
     TEST_F(CommandRunnerTests, ExecuteCommand)

src/modules/configuration/src/lib/Configuration.c

@@ -90,6 +90,8 @@ static char* LoadConfigurationFromFile(const char* fileName)
         g_iotHubManagementEnabled = IsIotHubManagementEnabledInJsonConfig(jsonConfiguration);
         g_iotHubProtocol = GetIotHubProtocolFromJsonConfig(jsonConfiguration, ConfigurationGetLog());
         g_gitManagementEnabled = GetGitManagementFromJsonConfig(jsonConfiguration, ConfigurationGetLog());
+
+        FREE_MEMORY(g_gitBranch);
         g_gitBranch = GetGitBranchFromJsonConfig(jsonConfiguration, ConfigurationGetLog());
     }
     else
@@ -597,7 +599,7 @@ int ConfigurationMmiSet(MMI_HANDLE clientSession, const char* componentName, con
                 }
                 else if (0 == strcmp(objectName, g_desiredGitBranchObject))
                 {
-                    if (NULL != (jsonString = (char*)json_value_get_string(jsonValue)))
+                    if (NULL != (jsonString = json_value_get_string(jsonValue)))
                     {
                         if (jsonString)
                         {

src/platform/MpiServer.c

@@ -199,7 +199,7 @@ HTTP_STATUS HandleMpiCall(const char* uri, const char* requestBody, char** respo
     const char* client = NULL;
     const char* component = NULL;
     const char* object = NULL;
-    const char* payload = NULL;
+    char* payload = NULL;
     int maxPayloadSizeBytes = 0;
     int estimatedSize = 0;
     const char* responseFormat = "\"%s\"";
@@ -424,6 +424,7 @@ HTTP_STATUS HandleMpiCall(const char* uri, const char* requestBody, char** respo
         }
     }
 
+    json_free_serialized_string(payload);
     json_value_free(rootValue);
 
     return status;