Merge ~paelzer/ubuntu/+source/open-vm-tools:fix-1847157-vix-mem-leak-bionic into ubuntu/+source/open-vm-tools:ubuntu/bionic-devel

[Christian Ehrhardt ]

[Christian Ehrhardt  (paelzer)]

Test build: https://launchpad.net/~paelzer/+archive/ubuntu/bug-1847157-ovm-memloak

[Andreas Hasenack (ahasenack)]

Same change as disco and eoan

d/changelog version ok, considering upgrades from xenial, and to (new) disco.

+1

review: Approve

[Christian Ehrhardt  (paelzer)]

Tagged and uploaded

diff --git a/debian/changelog b/debian/changelog
index 82edc10..f7e8a4f 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,12 @@
+open-vm-tools (2:10.3.10-1~ubuntu0.18.04.2) bionic; urgency=medium
+
+  * Fix memory leaks in vix plugin (LP: #1847157)
+    - d/p/lp-1847157-End-VGAuth-impersonation-in-the-case-of-error.patch
+    - d/p/lp-1847157-Fix-leaks-in-ListAliases-and-ListMappedAliases-9bc72.patch
+    - d/p/lp-1847157-Fix-memory-leaks-in-vix-tools-plugin.patch
+
+ -- Christian Ehrhardt <[e-mail]>  Wed, 09 Oct 2019 14:06:46 +0200
+
 open-vm-tools (2:10.3.10-1~ubuntu0.18.04.1) bionic; urgency=medium
 
   * Backport recent open-vm-tools (LP: #1822204)
diff --git a/debian/control b/debian/control
index 11b0b7c..9858935 100644
--- a/debian/control
+++ b/debian/control
@@ -1,7 +1,8 @@
 Source: open-vm-tools
 Section: admin
 Priority: extra
-Maintainer: Bernd Zeimetz <[e-mail]>
+Maintainer: Ubuntu Developers <[e-mail]>
+XSBC-Original-Maintainer: Bernd Zeimetz <[e-mail]>
 Build-Depends:
  debhelper (>= 11~), autotools-dev, doxygen, libcunit1-dev,
  libdumbnet-dev, libfuse-dev, libgtkmm-3.0-dev, libgtk-3-dev,
diff --git a/debian/patches/lp-1847157-End-VGAuth-impersonation-in-the-case-of-error.patch b/debian/patches/lp-1847157-End-VGAuth-impersonation-in-the-case-of-error.patch
new file mode 100644
index 0000000..9a96fed
--- /dev/null
+++ b/debian/patches/lp-1847157-End-VGAuth-impersonation-in-the-case-of-error.patch
@@ -0,0 +1,100 @@
+From 7b874f37f970aab2adddb063a8363594f47abf70 Mon Sep 17 00:00:00 2001
+From: Oliver Kurth <[e-mail]>
+Date: Tue, 4 Sep 2018 15:40:58 -0700
+Subject: [PATCH] End VGAuth impersonation in the case of error.
+
+* In GuestAuthPasswordAuthenticateImpersonate():
+When VGAuth_UserHandleAccessToken fails, unimpersonation is not
+being done. This can cause issues. Fixed it.
+
+* In GuestAuthSAMLAuthenticateAndImpersonate(), fixed the following issues:
+The 'newHandle' is not being freed which causes a memory leak.
+When VGAuth_UserHandleAccessToken fails, unimpersonation is not
+being done.
+
+Origin: upstream, https://github.com/vmware/open-vm-tools/commit/7b874f37f970aab2adddb063a8363594f47abf70
+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1847157
+Last-Update: 2019-10-09
+
+---
+ open-vm-tools/services/plugins/vix/vixTools.c | 25 +++++++++++++++++--
+ 1 file changed, 23 insertions(+), 2 deletions(-)
+
+diff --git a/open-vm-tools/services/plugins/vix/vixTools.c b/open-vm-tools/services/plugins/vix/vixTools.c
+index 44369444c..00b40b1ce 100644
+--- a/open-vm-tools/services/plugins/vix/vixTools.c
++++ b/open-vm-tools/services/plugins/vix/vixTools.c
+@@ -11550,6 +11550,7 @@ GuestAuthPasswordAuthenticateImpersonate(
+    VGAuthError vgErr;
+    VGAuthUserHandle *newHandle = NULL;
+    VGAuthExtraParams extraParams[1];
++   Bool impersonated = FALSE;
+ 
+    extraParams[0].name = VGAUTH_PARAM_LOAD_USER_PROFILE;
+    extraParams[0].value = VGAUTH_PARAM_VALUE_TRUE;
+@@ -11585,6 +11586,8 @@ GuestAuthPasswordAuthenticateImpersonate(
+       goto done;
+    }
+ 
++   impersonated = TRUE;
++
+ #ifdef _WIN32
+    // this is making a copy of the token, be sure to close it
+    vgErr = VGAuth_UserHandleAccessToken(ctx, newHandle, userToken);
+@@ -11604,6 +11607,10 @@ done:
+    Util_ZeroFreeString(password);
+ 
+    if (VIX_OK != err) {
++      if (impersonated) {
++         vgErr = VGAuth_EndImpersonation(ctx);
++         ASSERT(vgErr == VGAUTH_E_OK);
++      }
+       VGAuth_UserHandleFree(newHandle);
+       newHandle = NULL;
+    }
+@@ -11638,12 +11645,13 @@ GuestAuthSAMLAuthenticateAndImpersonate(
+ {
+ #if SUPPORT_VGAUTH
+    VixError err;
+-   char *token;
+-   char *username;
++   char *token = NULL;
++   char *username = NULL;
+    VGAuthContext *ctx = NULL;
+    VGAuthError vgErr;
+    VGAuthUserHandle *newHandle = NULL;
+    VGAuthExtraParams extraParams[1];
++   Bool impersonated = FALSE;
+ 
+    extraParams[0].name = VGAUTH_PARAM_LOAD_USER_PROFILE;
+    extraParams[0].value = VGAUTH_PARAM_VALUE_TRUE;
+@@ -11735,6 +11743,8 @@ impersonate:
+       goto done;
+    }
+ 
++   impersonated = TRUE;
++
+ #ifdef _WIN32
+    // this is making a copy of the token, be sure to close it
+    vgErr = VGAuth_UserHandleAccessToken(ctx, newHandle, userToken);
+@@ -11750,6 +11760,17 @@ impersonate:
+    err = VIX_OK;
+ 
+ done:
++   Util_ZeroFreeString(token);
++   Util_ZeroFreeString(username);
++
++   if (VIX_OK != err) {
++      if (impersonated) {
++         vgErr = VGAuth_EndImpersonation(ctx);
++         ASSERT(vgErr == VGAUTH_E_OK);
++      }
++      VGAuth_UserHandleFree(newHandle);
++      newHandle = NULL;
++   }
+ 
+    return err;
+ #else
+-- 
+2.23.0
+
diff --git a/debian/patches/lp-1847157-Fix-leaks-in-ListAliases-and-ListMappedAliases-9bc72.patch b/debian/patches/lp-1847157-Fix-leaks-in-ListAliases-and-ListMappedAliases-9bc72.patch
new file mode 100644
index 0000000..0050f47
--- /dev/null
+++ b/debian/patches/lp-1847157-Fix-leaks-in-ListAliases-and-ListMappedAliases-9bc72.patch
@@ -0,0 +1,74 @@
+From 26b9edbeb79d1c67b9ae73a0c97c48999c1fb503 Mon Sep 17 00:00:00 2001
+From: Oliver Kurth <[e-mail]>
+Date: Wed, 2 Oct 2019 17:48:35 -0700
+Subject: [PATCH] Fix leaks in ListAliases and ListMappedAliases
+ (9bc72f0b09702754b429115658a85223cb3058bd from devel)
+
+
+Origin: upstream, https://github.com/vmware/open-vm-tools/commit/26b9edbeb79d1c67b9ae73a0c97c48999c1fb503
+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1847157
+Last-Update: 2019-10-09
+
+---
+ open-vm-tools/services/plugins/vix/vixTools.c | 10 ++++++++--
+ 1 file changed, 8 insertions(+), 2 deletions(-)
+
+diff --git a/open-vm-tools/services/plugins/vix/vixTools.c b/open-vm-tools/services/plugins/vix/vixTools.c
+index 00b40b1ce..b0bd4d023 100644
+--- a/open-vm-tools/services/plugins/vix/vixTools.c
++++ b/open-vm-tools/services/plugins/vix/vixTools.c
+@@ -9621,7 +9621,6 @@ VixToolsListAuthAliases(VixCommandRequestHeader *requestMsg, // IN
+    char *endDestPtr;
+    char *tmpBuf = NULL;
+    char *tmpBuf2 = NULL;
+-   char *recordBuf;
+    size_t recordSize;
+    char *escapedStr = NULL;
+    char *escapedStr2 = NULL;
+@@ -9680,6 +9679,8 @@ VixToolsListAuthAliases(VixCommandRequestHeader *requestMsg, // IN
+    destPtr += Str_Sprintf(destPtr, endDestPtr - destPtr, "%s",
+                           VIX_XML_ESCAPED_TAG);
+    for (i = 0; i < num; i++) {
++      char *recordBuf = NULL;
++
+       escapedStr = VixToolsEscapeXMLString(uaList[i].pemCert);
+       if (escapedStr == NULL) {
+          err = VIX_E_OUT_OF_MEMORY;
+@@ -9750,6 +9751,8 @@ VixToolsListAuthAliases(VixCommandRequestHeader *requestMsg, // IN
+          Log("%s: ListAuth list results too large, truncating", __FUNCTION__);
+          goto abort;
+       }
++      free(recordBuf);
++      recordBuf = NULL;
+    }
+ 
+    *result = resultBuffer;
+@@ -9817,7 +9820,6 @@ VixToolsListMappedAliases(VixCommandRequestHeader *requestMsg, // IN
+    char *endDestPtr;
+    char *tmpBuf = NULL;
+    char *tmpBuf2 = NULL;
+-   char *recordBuf;
+    char *escapedStr = NULL;
+    char *escapedStr2 = NULL;
+    size_t recordSize;
+@@ -9870,6 +9872,8 @@ VixToolsListMappedAliases(VixCommandRequestHeader *requestMsg, // IN
+    destPtr += Str_Sprintf(destPtr, endDestPtr - destPtr, "%s",
+                           VIX_XML_ESCAPED_TAG);
+    for (i = 0; i < num; i++) {
++      char *recordBuf = NULL;
++
+       escapedStr = VixToolsEscapeXMLString(maList[i].pemCert);
+       if (escapedStr == NULL) {
+          err = VIX_E_OUT_OF_MEMORY;
+@@ -9938,6 +9942,8 @@ VixToolsListMappedAliases(VixCommandRequestHeader *requestMsg, // IN
+          Log("%s: ListMapped results too large, truncating", __FUNCTION__);
+          goto abort;
+       }
++      free(recordBuf);
++      recordBuf = NULL;
+    }
+ 
+    *result = resultBuffer;
+-- 
+2.23.0
+
diff --git a/debian/patches/lp-1847157-Fix-memory-leaks-in-vix-tools-plugin.patch b/debian/patches/lp-1847157-Fix-memory-leaks-in-vix-tools-plugin.patch
new file mode 100644
index 0000000..e2c87f1
--- /dev/null
+++ b/debian/patches/lp-1847157-Fix-memory-leaks-in-vix-tools-plugin.patch
@@ -0,0 +1,90 @@
+From 015db4c06a8be65eb96cf62421e8b5366993452f Mon Sep 17 00:00:00 2001
+From: Oliver Kurth <[e-mail]>
+Date: Wed, 29 Aug 2018 13:29:45 -0700
+Subject: [PATCH] Fix memory leaks in 'vix' tools plugin.
+
+* vix plugin retrieves the power script file paths from the
+config file but doesn't free them and this causes a memory leak.
+Fixed the code to free the filepaths.
+
+* In GuestAuthPasswordAuthenticateImpersonate function, the VGAuth
+handle is not freed when the impersonation fails. Fixed the
+code to call VGAuth_UserHandleFree in the error path.
+
+Note: I executed one guest operation with wrong credentials.
+Every failure leaks 75 bytes of memory. (in Centos 64-bit VM)
+
+* Fixed another minor issue in the code. At couple of places in
+the code, replaced 'err' with 'vgErr' for storing the return value
+of VGAuth_UserHandleAccessToken.
+
+Origin: upstream, https://github.com/vmware/open-vm-tools/commit/015db4c06a8be65eb96cf62421e8b5366993452f
+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1847157
+Last-Update: 2019-10-09
+
+---
+ open-vm-tools/services/plugins/vix/vixTools.c | 20 +++++++++++++------
+ 1 file changed, 14 insertions(+), 6 deletions(-)
+
+diff --git a/open-vm-tools/services/plugins/vix/vixTools.c b/open-vm-tools/services/plugins/vix/vixTools.c
+index 55b1f0a51..44369444c 100644
+--- a/open-vm-tools/services/plugins/vix/vixTools.c
++++ b/open-vm-tools/services/plugins/vix/vixTools.c
+@@ -2522,10 +2522,10 @@ VixTools_GetToolsPropertiesImpl(GKeyFile *confDictRef,            // IN
+    char *guestName;
+    int osFamily;
+    char *packageList = NULL;
+-   const char *powerOffScript = NULL;
+-   const char *powerOnScript = NULL;
+-   const char *resumeScript = NULL;
+-   const char *suspendScript = NULL;
++   char *powerOffScript = NULL;
++   char *powerOnScript = NULL;
++   char *resumeScript = NULL;
++   char *suspendScript = NULL;
+    char *osName = NULL;
+    char *osNameFull = NULL;
+    Bool foundHostName;
+@@ -2726,6 +2726,10 @@ abort:
+    free(tempDir);
+    free(osName);
+    free(osNameFull);
++   free(suspendScript);
++   free(resumeScript);
++   free(powerOnScript);
++   free(powerOffScript);
+ #else
+    /*
+     * FreeBSD. We do not require all the properties above.
+@@ -11583,7 +11587,7 @@ GuestAuthPasswordAuthenticateImpersonate(
+ 
+ #ifdef _WIN32
+    // this is making a copy of the token, be sure to close it
+-   err = VGAuth_UserHandleAccessToken(ctx, newHandle, userToken);
++   vgErr = VGAuth_UserHandleAccessToken(ctx, newHandle, userToken);
+    if (VGAUTH_FAILED(vgErr)) {
+       err = VixToolsTranslateVGAuthError(vgErr);
+       goto done;
+@@ -11599,6 +11603,10 @@ done:
+    free(username);
+    Util_ZeroFreeString(password);
+ 
++   if (VIX_OK != err) {
++      VGAuth_UserHandleFree(newHandle);
++      newHandle = NULL;
++   }
+    return err;
+ #else
+    return VIX_E_NOT_SUPPORTED;
+@@ -11729,7 +11737,7 @@ impersonate:
+ 
+ #ifdef _WIN32
+    // this is making a copy of the token, be sure to close it
+-   err = VGAuth_UserHandleAccessToken(ctx, newHandle, userToken);
++   vgErr = VGAuth_UserHandleAccessToken(ctx, newHandle, userToken);
+    if (VGAUTH_FAILED(vgErr)) {
+       err = VixToolsTranslateVGAuthError(vgErr);
+       goto done;
+-- 
+2.23.0
+
diff --git a/debian/patches/series b/debian/patches/series
index 8140b20..0950b30 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -1,3 +1,6 @@
 debian/pam-use-common-auth-account
 debian/max_nic_count
 debian/scsi-udev-rule
+lp-1847157-Fix-memory-leaks-in-vix-tools-plugin.patch
+lp-1847157-End-VGAuth-impersonation-in-the-case-of-error.patch
+lp-1847157-Fix-leaks-in-ListAliases-and-ListMappedAliases-9bc72.patch