Fixes two heap corruptions in the corlib tests on Windows
authorNiklas Therning <niklas@therning.org>
Wed, 31 Aug 2016 16:40:41 +0000 (18:40 +0200)
committerNiklas Therning <niklas@therning.org>
Wed, 31 Aug 2016 17:11:24 +0000 (19:11 +0200)
The patch of mono/metadata/marshal.c makes sure that
Marshal.StringToHGlobalAnsi() allocates a memory area for the returned C
string which is at least as large as the length of the string object. This is
the behavior on non-Windows platforms. Without this patch the
MarshalTest.StringToHGlobalAnsiWithNullValues() test wrote outside of a memory
area and corrupted the heap which later on caused a heap corruption crash when
running the corlib test suite on Windows.

The patch of mono/metadata/mono-security.c fixes the freeing of the memory
returned by GetNamedSecurityInfoW(). When GetNamedSecurityInfoW() is used to
request a DACL one must also pass an argument for the ppSecurityDescriptor
parameter and this is the one that should be freed later on, not the pointer
to the DACL as the original code used to free. Before this patch
DSACryptoServiceProviderTest.UseMachineKeyStore() crashed because of a heap
corruption.

mono/metadata/marshal.c
mono/metadata/mono-security.c

index b27996839e9356dd40c0496abbc451ef3723a6d6..d56a3382048211fe5d44156f3ee50a6e4b2d3abc 100644 (file)
@@ -10949,7 +10949,12 @@ ves_icall_System_Runtime_InteropServices_Marshal_StringToHGlobalAnsi (MonoString
        if (!tres)
                return tres;
 
-       len = strlen (tres) + 1;
+       /*
+        * mono_string_to_utf8_checked() returns a memory area at least as large as the size of the
+        * MonoString, even if it contains NULL characters. The copy we allocate here has to be equally
+        * large.
+        */
+       len = MAX (strlen (tres) + 1, string->length);
        ret = ves_icall_System_Runtime_InteropServices_Marshal_AllocHGlobal (len);
        memcpy (ret, tres, len);
        g_free (tres);
index b27da358575742b762965b76338ececeb5248327..0b2d4a8c039fecae0ff326da57ced48c9ee52b9d 100644 (file)
@@ -660,9 +660,10 @@ IsMachineProtected (gunichar2 *path)
 {
        gboolean success = FALSE;
        PACL pDACL = NULL;
+       PSECURITY_DESCRIPTOR pSD = NULL;
        PSID pEveryoneSid = NULL;
 
-       DWORD dwRes = GetNamedSecurityInfoW (path, SE_FILE_OBJECT, DACL_SECURITY_INFORMATION, NULL, NULL, &pDACL, NULL, NULL);
+       DWORD dwRes = GetNamedSecurityInfoW (path, SE_FILE_OBJECT, DACL_SECURITY_INFORMATION, NULL, NULL, &pDACL, NULL, &pSD);
        if (dwRes != ERROR_SUCCESS)
                return FALSE;
 
@@ -679,8 +680,8 @@ IsMachineProtected (gunichar2 *path)
        /* Note: we don't need to check our own access - 
        we'll know soon enough when reading the file */
 
-       if (pDACL)
-               LocalFree (pDACL);
+       if (pSD)
+               LocalFree (pSD);
 
        return success;
 }