Changeset 93393 in vbox for trunk/src/VBox/VMM/VMMR3

Timestamp:

Jan 21, 2022 11:54:15 AM (3 years ago)

Author:

vboxsync

svn:sync-xref-src-repo-rev:

149489

Message:

VMM/CFGM: Use the RTMemSafer allocator for the password strings and scramble them while not being accessed. bugref:9469

File:

• trunk/src/VBox/VMM/VMMR3/CFGM.cpp (modified) (15 diffs)

trunk/src/VBox/VMM/VMMR3/CFGM.cpp

@@ -68 +68 @@
 #include <iprt/assert.h>
 #include <iprt/mem.h>
+#include <iprt/memsafer.h>
 #include <iprt/param.h>
 #include <iprt/string.h>
@@ -876 +877 @@
  * @param   pszString       Where to store the string.
  * @param   cchString       Size of the string buffer. (Includes terminator.)
+ *
+ * @note    Concurrent calls to this function and CFGMR3QueryPasswordDef are not
+ *          supported.
  */
 VMMR3DECL(int) CFGMR3QueryPassword(PCFGMNODE pNode, const char *pszName, char *pszString, size_t cchString)
@@ -888 +892 @@
             if (cchString >= cbSrc)
             {
+                RTMemSaferUnscramble(pLeaf->Value.String.psz, cbSrc);
                 memcpy(pszString, pLeaf->Value.String.psz, cbSrc);
                 memset(pszString + cbSrc, 0, cchString - cbSrc);
+                RTMemSaferScramble(pLeaf->Value.String.psz, cbSrc);
+
+                Assert(pszString[cbSrc - 1] == '\0');
             }
             else
@@ -910 +918 @@
  * @param   cchString       Size of the string buffer. (Includes terminator.)
  * @param   pszDef          The default value.
+ *
+ * @note    Concurrent calls to this function and CFGMR3QueryPassword are not
+ *          supported.
  */
 VMMR3DECL(int) CFGMR3QueryPasswordDef(PCFGMNODE pNode, const char *pszName, char *pszString, size_t cchString, const char *pszDef)
@@ -922 +933 @@
             if (cchString >= cbSrc)
             {
+                RTMemSaferUnscramble(pLeaf->Value.String.psz, cbSrc);
                 memcpy(pszString, pLeaf->Value.String.psz, cbSrc);
                 memset(pszString + cbSrc, 0, cchString - cbSrc);
+                RTMemSaferScramble(pLeaf->Value.String.psz, cbSrc);
+
+                Assert(pszString[cbSrc - 1] == '\0');
             }
             else
@@ -2115 +2130 @@
                 break;
 
-            case CFGMVALUETYPE_PASSWORD:
-                RTMemWipeThoroughly(pLeaf->Value.String.psz, pLeaf->Value.String.cb, 10);
-                RT_FALL_THROUGH();
             case CFGMVALUETYPE_STRING:
                 cfgmR3StrFree(pVM, pLeaf->Value.String.psz);
@@ -2124 +2136 @@
                 break;
 
+            case CFGMVALUETYPE_PASSWORD:
+                RTMemSaferFree(pLeaf->Value.String.psz, pLeaf->Value.String.cb);
+                pLeaf->Value.String.psz = NULL;
+                pLeaf->Value.String.cb = 0;
+                break;
+
             case CFGMVALUETYPE_INTEGER:
                 break;
@@ -2171 +2189 @@
 
 /**
- * Inserts a new string value. This variant expects that the caller know the length
- * of the string already so we can avoid calling strlen() here.
+ * Inserts a new string value.
+ *
+ * This variant expects that the caller know the length of the string already so
+ * we can avoid calling strlen() here.
  *
  * @returns VBox status code.
@@ -2222 +2242 @@
 
 /**
- * Inserts a new string value. Calls strlen(pszString) internally; if you know the
- * length of the string, CFGMR3InsertStringLengthKnown() is faster.
+ * Inserts a new string value.
+ *
+ * Calls strlen(pszString) internally; if you know the length of the string,
+ * CFGMR3InsertStringLengthKnown() is faster.
  *
  * @returns VBox status code.
@@ -2327 +2349 @@
 
 /**
- * Inserts a new integer value.
+ * Inserts a new bytes value.
  *
  * @returns VBox status code.
@@ -2378 +2400 @@
 
 /**
- * Inserts a new password value. This variant expects that the caller know the length
- * of the password string already so we can avoid calling strlen() here.
+ * Inserts a new password value.
+ *
+ * This variant expects that the caller know the length of the password string
+ * already so we can avoid calling strlen() here.
  *
  * @returns VBox status code.
@@ -2385 +2409 @@
  * @param   pszName         Value name.
  * @param   pszString       The value. Must not be NULL.
- * @param   cchString       The length of the string excluding the
- *                          terminator.
+ * @param   cchString       The length of the string excluding the terminator.
  */
 VMMR3DECL(int) CFGMR3InsertPasswordN(PCFGMNODE pNode, const char *pszName, const char *pszString, size_t cchString)
@@ -2396 +2419 @@
     {
         /*
-         * Allocate string object first.
+         * Allocate string object first using the safer memory API since this
+         * is considered sensitive information.
          */
-        char *pszStringCopy = (char *)cfgmR3StrAlloc(pNode->pVM, MM_TAG_CFGM_STRING, cchString + 1);
+        char *pszStringCopy = (char *)RTMemSaferAllocZ(cchString + 1);
         if (pszStringCopy)
         {
             memcpy(pszStringCopy, pszString, cchString);
             pszStringCopy[cchString] = '\0';
+            RTMemSaferScramble(pszStringCopy, cchString + 1);
 
             /*
@@ -2416 +2441 @@
             }
             else
-            {
-                RTMemWipeThoroughly(pszStringCopy, cchString, 10);
-                cfgmR3StrFree(pNode->pVM, pszStringCopy);
-            }
+                RTMemSaferFree(pszStringCopy, cchString + 1);
         }
         else
@@ -2432 +2454 @@
 
 /**
- * Inserts a new password value. Calls strlen(pszString) internally; if you know the
- * length of the string, CFGMR3InsertStringLengthKnown() is faster.
+ * Inserts a new password value.
+ *
+ * Calls strlen(pszString) internally; if you know the length of the string,
+ * CFGMR3InsertStringLengthKnown() is faster.
  *
  * @returns VBox status code.