From 95800c144d2ab2af95cdc8f08df0518c496a579a Mon Sep 17 00:00:00 2001

From: Cathy Avery <cavery@redhat.com>

Date: Thu, 12 Nov 2020 09:01:08 -0500

Subject: [PATCH] Fix memory leaks.

RH-Author: Cathy Avery (cavery)

RH-MergeRequest: 2: Fix memory leaks.

RH-Commit: [1/1] 79ac85f5e8c31cc48b7b0834682c6320afcc2288 (cavery/open-vm-tools)

RH-Bugzilla: 1896804

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1896804

Brew: https://brewweb.engineering.redhat.com/brew/taskinfo?taskID=33050983

Tested: By QE

Upstream Status: devel branch

A Coverity scan of open-vm-tools reported a number of memory leaks

on error code paths.  Fix seven reported leaks, and modify code

to address two false positives in order to make the code clearer

and/or keep Coverity from reporting the issues.  Also fix additional

leaks found in the routine Proto_TextContents during code review.

(cherry picked from commit e18e67f727d0354b08a55b685178fd05f542c6da)

Signed-off-by: Cathy Avery <cavery@redhat.com>

---

 open-vm-tools/libvmtools/vmtoolsLog.c         |  6 ++---

 .../plugins/guestInfo/guestInfoServer.c       |  2 +-

 open-vm-tools/services/vmtoolsd/pluginMgr.c   |  1 +

 open-vm-tools/vgauth/lib/proto.c              | 23 +++++++++++++++----

 open-vm-tools/vgauth/serviceImpl/alias.c      |  4 ++++

 5 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/open-vm-tools/libvmtools/vmtoolsLog.c b/open-vm-tools/libvmtools/vmtoolsLog.c

index a991b49f..bea5abd4 100644

--- a/open-vm-tools/libvmtools/vmtoolsLog.c

+++ b/open-vm-tools/libvmtools/vmtoolsLog.c

@@ -2395,7 +2395,6 @@ VMTools_ChangeLogFilePath(const gchar *delimiter,     // IN

 {

    gchar key[128];

    gchar *path = NULL;

-   gchar *userLogTemp = NULL;

    gchar **tokens;

    gboolean retVal = FALSE;

@@ -2412,8 +2411,9 @@ VMTools_ChangeLogFilePath(const gchar *delimiter,     // IN

    tokens = g_strsplit(path, delimiter, 2);

    if (tokens != NULL && *tokens != NULL){

-      userLogTemp = g_strjoin(appendString, *tokens, " ", NULL);

-      userLogTemp = g_strchomp (userLogTemp);

+      char *userLogTemp = g_strjoin(appendString, *tokens, " ", NULL);

+

+      g_strchomp(userLogTemp);

       if (*(tokens+1) != NULL){

          gchar *userLog;

          userLog = g_strjoin(delimiter, userLogTemp, *(tokens+1), NULL);

diff --git a/open-vm-tools/services/plugins/guestInfo/guestInfoServer.c b/open-vm-tools/services/plugins/guestInfo/guestInfoServer.c

index c1ab6962..ab6725fe 100644

--- a/open-vm-tools/services/plugins/guestInfo/guestInfoServer.c

+++ b/open-vm-tools/services/plugins/guestInfo/guestInfoServer.c

@@ -1298,12 +1298,12 @@ GuestInfoSendDiskInfoV1(ToolsAppCtx *ctx,             // IN

                          b64name,

                          pdi->partitionList[i].freeBytes,

                          pdi->partitionList[i].totalBytes);

+      g_free(b64name);

       if (len <= 0) {

          goto exit;

       }

       DynBuf_Append(&dynBuffer, tmpBuf, len);

-      g_free(b64name);

       if (pdi->partitionList[i].fsType[0] != '\0') {

          len = Str_Snprintf(tmpBuf, sizeof tmpBuf, jsonPerDiskFsTypeFmt,

diff --git a/open-vm-tools/services/vmtoolsd/pluginMgr.c b/open-vm-tools/services/vmtoolsd/pluginMgr.c

index 53b91f7a..d5f2c0ef 100644

--- a/open-vm-tools/services/vmtoolsd/pluginMgr.c

+++ b/open-vm-tools/services/vmtoolsd/pluginMgr.c

@@ -512,6 +512,7 @@ ToolsCoreLoadDirectory(ToolsAppCtx *ctx,

    dir = g_dir_open(pluginPath, 0, &err;;

    if (dir == NULL) {

       g_warning("Error opening dir: %s\n", err->message);

+      g_clear_error(&err;;

       goto exit;

    }

diff --git a/open-vm-tools/vgauth/lib/proto.c b/open-vm-tools/vgauth/lib/proto.c

index 12386918..01df9df7 100644

--- a/open-vm-tools/vgauth/lib/proto.c

+++ b/open-vm-tools/vgauth/lib/proto.c

@@ -830,8 +830,10 @@ Proto_TextContents(GMarkupParseContext *parseContext,

          g_set_error(error, G_MARKUP_ERROR_PARSE, VGAUTH_E_INVALID_ARGUMENT,

                      "Found pipeName in reply type %d",

                      reply->expectedReplyType);

+         g_free(val);

+      } else {

+         reply->replyData.sessionReq.pipeName = val;

       }

-      reply->replyData.sessionReq.pipeName = val;

       break;

    case PARSE_STATE_TICKET:

@@ -839,8 +841,10 @@ Proto_TextContents(GMarkupParseContext *parseContext,

          g_set_error(error, G_MARKUP_ERROR_PARSE, VGAUTH_E_INVALID_ARGUMENT,

                      "Found ticket in reply type %d",

                      reply->expectedReplyType);

+         g_free(val);

+      } else {

+         reply->replyData.createTicket.ticket = val;

       }

-      reply->replyData.createTicket.ticket = val;

       break;

    case PARSE_STATE_TOKEN:

@@ -853,6 +857,7 @@ Proto_TextContents(GMarkupParseContext *parseContext,

          g_set_error(error, G_MARKUP_ERROR_PARSE, VGAUTH_E_INVALID_ARGUMENT,

                      "Found token in reply type %d",

                      reply->expectedReplyType);

+         g_free(val);

       }

       break;

@@ -863,6 +868,7 @@ Proto_TextContents(GMarkupParseContext *parseContext,

          g_set_error(error, G_MARKUP_ERROR_PARSE, VGAUTH_E_INVALID_ARGUMENT,

                      "Found token in reply type %d",

                      reply->expectedReplyType);

+         g_free(val);

       }

       break;

@@ -878,6 +884,7 @@ Proto_TextContents(GMarkupParseContext *parseContext,

          g_set_error(error, G_MARKUP_ERROR_PARSE, VGAUTH_E_INVALID_ARGUMENT,

                      "Found username in reply type %d",

                      reply->expectedReplyType);

+         g_free(val);

       }

       break;

@@ -890,6 +897,7 @@ Proto_TextContents(GMarkupParseContext *parseContext,

          g_set_error(error, G_MARKUP_ERROR_PARSE, VGAUTH_E_INVALID_ARGUMENT,

                      "Found pemCert in reply type %d",

                      reply->expectedReplyType);

+         g_free(val);

       }

       break;

    case PARSE_STATE_CERTCOMMENT:

@@ -899,6 +907,7 @@ Proto_TextContents(GMarkupParseContext *parseContext,

          g_set_error(error, G_MARKUP_ERROR_PARSE, VGAUTH_E_INVALID_ARGUMENT,

                      "Found cert comment in reply type %d",

                      reply->expectedReplyType);

+         g_free(val);

       }

       break;

@@ -923,6 +932,7 @@ Proto_TextContents(GMarkupParseContext *parseContext,

          g_set_error(error, G_MARKUP_ERROR_PARSE, VGAUTH_E_INVALID_ARGUMENT,

                      "Found SAMLSubject in reply type %d",

                      reply->expectedReplyType);

+         g_free(val);

       }

       break;

    case PARSE_STATE_USERHANDLETYPE:

@@ -968,6 +978,7 @@ Proto_TextContents(GMarkupParseContext *parseContext,

          g_set_error(error, G_MARKUP_ERROR_PARSE, VGAUTH_E_INVALID_ARGUMENT,

                      "Found NamedSubject in reply type %d",

                      reply->expectedReplyType);

+         g_free(val);

       }

       break;

    case PARSE_STATE_ANYSUBJECT:

@@ -990,6 +1001,7 @@ Proto_TextContents(GMarkupParseContext *parseContext,

                      "Found AnySubject in reply type %d",

                      reply->expectedReplyType);

       }

+      g_free(val);

       break;

    case PARSE_STATE_COMMENT:

       if (PROTO_REPLY_QUERYALIASES == reply->expectedReplyType) {

@@ -1005,11 +1017,13 @@ Proto_TextContents(GMarkupParseContext *parseContext,

          g_set_error(error, G_MARKUP_ERROR_PARSE, VGAUTH_E_INVALID_ARGUMENT,

                      "Found comment in reply type %d",

                      reply->expectedReplyType);

+         g_free(val);

       }

       break;

    default:

       g_warning("Unexpected value '%s' in unhandled parseState %d in %s\n",

                 val, reply->parseState, __FUNCTION__);

+      g_free(val);

       ASSERT(0);

    }

 }

@@ -1200,7 +1214,6 @@ VGAuth_ReadAndParseResponse(VGAuthContext *ctx,

    VGAuthError err = VGAUTH_E_OK;

    GMarkupParseContext *parseContext;

    gsize len;

-   gchar *rawReply = NULL;

    ProtoReply *reply;

    gboolean bRet;

    GError *gErr = NULL;

@@ -1217,6 +1230,8 @@ VGAuth_ReadAndParseResponse(VGAuthContext *ctx,

     * transport.

     */

    while (!reply->complete) {

+      gchar *rawReply = NULL;

+

       err = VGAuth_CommReadData(ctx, &len, &rawReply);

       if (0 == len) {      // EOF -- not expected

          err = VGAUTH_E_COMM;

@@ -1237,6 +1252,7 @@ VGAuth_ReadAndParseResponse(VGAuthContext *ctx,

                                           rawReply,

                                           len,

                                           &gErr);

+      g_free(rawReply);

       if (!bRet) {

          /*

           * XXX Could drain the wire here, but since this should

@@ -1252,7 +1268,6 @@ VGAuth_ReadAndParseResponse(VGAuthContext *ctx,

        * XXX need some way to break out if packet never completed

        * yet socket left valid.  timer?

        */

-      g_free(rawReply);

    }

 #if VGAUTH_PROTO_TRACE

diff --git a/open-vm-tools/vgauth/serviceImpl/alias.c b/open-vm-tools/vgauth/serviceImpl/alias.c

index f6cde02c..0a43811e 100644

--- a/open-vm-tools/vgauth/serviceImpl/alias.c

+++ b/open-vm-tools/vgauth/serviceImpl/alias.c

@@ -3158,6 +3158,9 @@ ServiceIDVerifyStoreContents(void)

              * a blacklist of bad files and keep going.  but that's

              * a lot of risky work that's very hard to test, so punt for now.

              */

+            g_free(badFileName);

+            g_free(fullFileName);

+            g_dir_close(dir);

             return VGAUTH_E_FAIL;

          } else {

             Audit_Event(TRUE,

@@ -3408,6 +3411,7 @@ ServiceAliasInitAliasStore(void)

                          "Failed to rename suspect Alias store directory '%s' to '%s'"),

                      aliasStoreRootDir, badRootDirName);

          // XXX making this fatal for now.  can we do anything better?

+         g_free(badRootDirName);

          return VGAUTH_E_FAIL;

       }

       g_free(badRootDirName);

-- 

2.18.4