Moving BSTR conv to native code in SecureStringToBSTR.

Previous implementation laid out the BSTR structure
in directly memory in C# code. This layout was not correct
for windows.
Furthermore memory was allocated using AllocCoTaskMem but later freed
using SysFreeString which on windows are different memory areas.
Third issue is that SecureString asumes big-endian when copying the chars
to its internal byte buffer. So on little-endian environments,
a swap of bytes is needed before the buffer is sent to native side
for BSTR conversion. This should probably be handled internally in
SecureString instead.

diff --git a/mcs/class/corlib/System.Runtime.InteropServices/Marshal.cs b/mcs/class/corlib/System.Runtime.InteropServices/Marshal.cs
index a1b028b08c17a975da156186da5cb736e74ed47d..f983beeb634111f050844731b252ead7ccf04db1 100644 (file)
--- a/mcs/class/corlib/System.Runtime.InteropServices/Marshal.cs
+++ b/mcs/class/corlib/System.Runtime.InteropServices/Marshal.cs
@@ -1051,25 +1051,21 @@ namespace System.Runtime.InteropServices
                {
                        if (s == null)
                                throw new ArgumentNullException ("s");
-                       int len = s.Length;
-                       IntPtr ctm = AllocCoTaskMem ((len+1) * 2 + 4);
-                       byte [] buffer = null;
-                       WriteInt32 (ctm, 0, len*2);
-                       try {
-                               buffer = s.GetBuffer ();
 
-                               for (int i = 0; i < len; i++)
-                                       WriteInt16 (ctm, 4 + (i * 2), (short) ((buffer [(i*2)] << 8) | (buffer [i*2+1])));
-                               WriteInt16 (ctm, 4 + buffer.Length, 0);
-                       } finally {
-                               if (buffer != null)
-                                       for (int i = buffer.Length; i > 0; ){
-                                               i--;
-                                               buffer [i] = 0;
-                                       }
+                       byte[] buffer = s.GetBuffer ();
+                       int len = s.Length;
+                       
+                       // SecureString doesn't take endian-ness into account. 
+                       // Therefore swap bytes here before we send it to c-side if little-endian.
+                       if (BitConverter.IsLittleEndian) {
+                               for (int i = 0; i < buffer.Length; i += 2) {
+                                       byte b = buffer[i];
+                                       buffer[i] = buffer[i + 1];
+                                       buffer[i + 1] = b;
+                               }
                        }
-                       return (IntPtr) ((long)ctm + 4);
-               }
+                       return BufferToBSTR (buffer, len);
+        }
 
                public static IntPtr SecureStringToCoTaskMemAnsi (SecureString s)
                {
@@ -1156,6 +1152,10 @@ namespace System.Runtime.InteropServices
                                throw ex;
                }
 
+
+               [MethodImplAttribute(MethodImplOptions.InternalCall)]
+               public extern static IntPtr BufferToBSTR (Array ptr, int slen);
+
                [MethodImplAttribute(MethodImplOptions.InternalCall)]
                public extern static IntPtr UnsafeAddrOfPinnedArrayElement (Array arr, int index);
 

diff --git a/mono/metadata/cominterop.c b/mono/metadata/cominterop.c
index 889c19ece4f786221a020fc365cebe41983b7734..bedd740079bfdc160e07c76ae65b67134f7af777 100644 (file)
--- a/mono/metadata/cominterop.c
+++ b/mono/metadata/cominterop.c
@@ -51,6 +51,15 @@ register_icall (gpointer func, const char *name, const char *sigstr, gboolean sa
        mono_register_jit_icall (func, name, sig, save);
 }
 
+gpointer
+mono_string_to_bstr(MonoString* ptr)
+{
+       if (!ptr)
+               return NULL;
+
+       return mono_ptr_to_bstr(mono_string_chars(ptr), mono_string_length(ptr));
+}
+
 #ifndef DISABLE_COM
 
 #define OPDEF(a,b,c,d,e,f,g,h,i,j) \
@@ -2742,37 +2751,37 @@ init_com_provider_ms (void)
 }
 
 gpointer
-mono_string_to_bstr (MonoString *string_obj)
+mono_ptr_to_bstr(gpointer ptr, int slen)
 {
-       if (!string_obj)
+       if (!ptr)
                return NULL;
 #ifdef HOST_WIN32
-       return SysAllocStringLen (mono_string_chars (string_obj), mono_string_length (string_obj));
+       return SysAllocStringLen (ptr, slen);
 #else
        if (com_provider == MONO_COM_DEFAULT) {
-               int slen = mono_string_length (string_obj);
                /* allocate len + 1 utf16 characters plus 4 byte integer for length*/
-               char *ret = (char *)g_malloc ((slen + 1) * sizeof(gunichar2) + sizeof(guint32));
+               char *ret = (char *)g_malloc((slen + 1) * sizeof(gunichar2) + sizeof(guint32));
                if (ret == NULL)
                        return NULL;
-               memcpy (ret + sizeof(guint32), mono_string_chars (string_obj), slen * sizeof(gunichar2));
-               * ((guint32 *) ret) = slen * sizeof(gunichar2);
-               ret [4 + slen * sizeof(gunichar2)] = 0;
-               ret [5 + slen * sizeof(gunichar2)] = 0;
+               memcpy(ret + sizeof(guint32), ptr, slen * sizeof(gunichar2));
+               *((guint32 *)ret) = slen * sizeof(gunichar2);
+               ret[4 + slen * sizeof(gunichar2)] = 0;
+               ret[5 + slen * sizeof(gunichar2)] = 0;
 
                return ret + 4;
-       } else if (com_provider == MONO_COM_MS && init_com_provider_ms ()) {
+       }
+       else if (com_provider == MONO_COM_MS && init_com_provider_ms()) {
                gpointer ret = NULL;
                gunichar* str = NULL;
-               guint32 len;
-               len = mono_string_length (string_obj);
-               str = g_utf16_to_ucs4 (mono_string_chars (string_obj), len,
+               guint32 len = slen;
+               str = g_utf16_to_ucs4(ptr, len,
                        NULL, NULL, NULL);
-               ret = sys_alloc_string_len_ms (str, len);
+               ret = sys_alloc_string_len_ms(str, len);
                g_free(str);
                return ret;
-       } else {
-               g_assert_not_reached ();
+       }
+       else {
+               g_assert_not_reached();
        }
 #endif
 }
@@ -3411,20 +3420,19 @@ cominterop_release_all_rcws (void)
 }
 
 gpointer
-mono_string_to_bstr (MonoString *string_obj)
+mono_ptr_to_bstr (gpointer ptr, int slen)
 {
-       if (!string_obj)
+       if (!ptr)
                return NULL;
 #ifdef HOST_WIN32
-       return SysAllocStringLen (mono_string_chars (string_obj), mono_string_length (string_obj));
+       return SysAllocStringLen (ptr, slen);
 #else
        {
-               int slen = mono_string_length (string_obj);
                /* allocate len + 1 utf16 characters plus 4 byte integer for length*/
                char *ret = g_malloc ((slen + 1) * sizeof(gunichar2) + sizeof(guint32));
                if (ret == NULL)
                        return NULL;
-               memcpy (ret + sizeof(guint32), mono_string_chars (string_obj), slen * sizeof(gunichar2));
+               memcpy (ret + sizeof(guint32), ptr, slen * sizeof(gunichar2));
                * ((guint32 *) ret) = slen * sizeof(gunichar2);
                ret [4 + slen * sizeof(gunichar2)] = 0;
                ret [5 + slen * sizeof(gunichar2)] = 0;
@@ -3524,6 +3532,12 @@ ves_icall_System_Runtime_InteropServices_Marshal_StringToBSTR (MonoString* ptr)
        return mono_string_to_bstr(ptr);
 }
 
+gpointer
+ves_icall_System_Runtime_InteropServices_Marshal_BufferToBSTR (MonoArray* ptr, int len)
+{
+       return mono_ptr_to_bstr (ptr->vector, len);
+}
+
 void
 ves_icall_System_Runtime_InteropServices_Marshal_FreeBSTR (gpointer ptr)
 {

diff --git a/mono/metadata/icall-def.h b/mono/metadata/icall-def.h
index 432ddd6bea5c9653957967876a3728bbafdfb1cc..68903954c10e09e4aea8045bb72c100a6e024370 100644 (file)
--- a/mono/metadata/icall-def.h
+++ b/mono/metadata/icall-def.h
@@ -646,6 +646,7 @@ ICALL_TYPE(MARSHAL, "System.Runtime.InteropServices.Marshal", MARSHAL_2)
 #endif
 ICALL(MARSHAL_2, "AllocCoTaskMem", ves_icall_System_Runtime_InteropServices_Marshal_AllocCoTaskMem)
 ICALL(MARSHAL_3, "AllocHGlobal", ves_icall_System_Runtime_InteropServices_Marshal_AllocHGlobal)
+ICALL(MARSHAL_50, "BufferToBSTR", ves_icall_System_Runtime_InteropServices_Marshal_BufferToBSTR)
 ICALL(MARSHAL_4, "DestroyStructure", ves_icall_System_Runtime_InteropServices_Marshal_DestroyStructure)
 ICALL(MARSHAL_5, "FreeBSTR", ves_icall_System_Runtime_InteropServices_Marshal_FreeBSTR)
 ICALL(MARSHAL_6, "FreeCoTaskMem", ves_icall_System_Runtime_InteropServices_Marshal_FreeCoTaskMem)
@@ -692,6 +693,7 @@ ICALL(MARSHAL_32, "StringToHGlobalAnsi", ves_icall_System_Runtime_InteropService
 ICALL(MARSHAL_33, "StringToHGlobalUni", ves_icall_System_Runtime_InteropServices_Marshal_StringToHGlobalUni)
 ICALL(MARSHAL_34, "StructureToPtr", ves_icall_System_Runtime_InteropServices_Marshal_StructureToPtr)
 ICALL(MARSHAL_35, "UnsafeAddrOfPinnedArrayElement", ves_icall_System_Runtime_InteropServices_Marshal_UnsafeAddrOfPinnedArrayElement)
+
 ICALL(MARSHAL_41, "copy_from_unmanaged", ves_icall_System_Runtime_InteropServices_Marshal_copy_from_unmanaged)
 ICALL(MARSHAL_42, "copy_to_unmanaged", ves_icall_System_Runtime_InteropServices_Marshal_copy_to_unmanaged)
 

diff --git a/mono/metadata/marshal.h b/mono/metadata/marshal.h
index d3c0a2b051762f2ec82b4f7b8095a053f78c9e9b..58caa69d814f5d57e74a6fb176aba6cff0248004 100644 (file)
--- a/mono/metadata/marshal.h
+++ b/mono/metadata/marshal.h
@@ -269,7 +269,10 @@ gpointer
 mono_string_to_ansibstr (MonoString *string_obj);
 
 gpointer
-mono_string_to_bstr (MonoString *string_obj);
+mono_ptr_to_bstr (gpointer ptr, int slen);
+
+gpointer
+mono_string_to_bstr(MonoString* str);
 
 void mono_delegate_free_ftnptr (MonoDelegate *delegate);
 
@@ -466,6 +469,9 @@ ves_icall_System_Runtime_InteropServices_Marshal_OffsetOf (MonoReflectionType *t
 gpointer
 ves_icall_System_Runtime_InteropServices_Marshal_StringToBSTR (MonoString *string);
 
+gpointer
+ves_icall_System_Runtime_InteropServices_Marshal_BufferToBSTR (MonoArray *ptr, int len);
+
 gpointer
 ves_icall_System_Runtime_InteropServices_Marshal_StringToHGlobalAnsi (MonoString *string);