From 5bf70cedf4ca26d7cdd5c9ce62e3a1cdb6187cff Mon Sep 17 00:00:00 2001 From: Ayoub Kaanich Date: Tue, 8 Oct 2024 08:26:05 +0200 Subject: [PATCH] Fix marshalling of StringBuilder errors (#533) * Fix marshalling of StringBuilder errors * Move StringBuilder logic to PcapStringBuilderMarshaler * Drop ICustomMarshaler for error string, use ErrorBuffer class instead.ss * Revert csproj changes * Fix build * Cleanup --- SharpPcap/LibPcap/CaptureFileReaderDevice.cs | 7 ++- .../LibPcap/CaptureHandleReaderDevice.cs | 4 +- SharpPcap/LibPcap/LibPcapLiveDevice.cs | 18 +++---- .../LibPcapSafeNativeMethods.Encoding.cs | 48 ++++++++++--------- .../LibPcapSafeNativeMethods.Interop.cs | 25 +++++----- SharpPcap/LibPcap/LibPcapSafeNativeMethods.cs | 6 +-- SharpPcap/LibPcap/PcapInterface.cs | 13 ++--- SharpPcap/Pcap.cs | 1 - 8 files changed, 56 insertions(+), 66 deletions(-) diff --git a/SharpPcap/LibPcap/CaptureFileReaderDevice.cs b/SharpPcap/LibPcap/CaptureFileReaderDevice.cs index 1f6bfe3e..ab8bb0e2 100644 --- a/SharpPcap/LibPcap/CaptureFileReaderDevice.cs +++ b/SharpPcap/LibPcap/CaptureFileReaderDevice.cs @@ -72,8 +72,7 @@ public CaptureFileReaderDevice(string captureFilename) /// public override void Open(DeviceConfiguration configuration) { - // holds errors - StringBuilder errbuf = new StringBuilder(Pcap.PCAP_ERRBUF_SIZE); //will hold errors + ErrorBuffer errbuf; //will hold errors PcapHandle adapterHandle; @@ -82,7 +81,7 @@ public override void Open(DeviceConfiguration configuration) var resolution = configuration.TimestampResolution ?? TimestampResolution.Microsecond; if (has_offline_with_tstamp_precision_support) { - adapterHandle = LibPcapSafeNativeMethods.pcap_open_offline_with_tstamp_precision(m_pcapFile, (uint)resolution, errbuf); + adapterHandle = LibPcapSafeNativeMethods.pcap_open_offline_with_tstamp_precision(m_pcapFile, (uint)resolution, out errbuf); } else { @@ -97,7 +96,7 @@ public override void Open(DeviceConfiguration configuration) ); } - adapterHandle = LibPcapSafeNativeMethods.pcap_open_offline(m_pcapFile, errbuf); + adapterHandle = LibPcapSafeNativeMethods.pcap_open_offline(m_pcapFile, out errbuf); } // handle error diff --git a/SharpPcap/LibPcap/CaptureHandleReaderDevice.cs b/SharpPcap/LibPcap/CaptureHandleReaderDevice.cs index 34993e72..115e07fe 100644 --- a/SharpPcap/LibPcap/CaptureHandleReaderDevice.cs +++ b/SharpPcap/LibPcap/CaptureHandleReaderDevice.cs @@ -48,14 +48,14 @@ public CaptureHandleReaderDevice(SafeHandle handle) public override void Open(DeviceConfiguration configuration) { // holds errors - StringBuilder errbuf = new StringBuilder(Pcap.PCAP_ERRBUF_SIZE); + ErrorBuffer errbuf; var resolution = configuration.TimestampResolution ?? TimestampResolution.Microsecond; PcapHandle adapterHandle; try { adapterHandle = LibPcapSafeNativeMethods.pcap_open_handle_offline_with_tstamp_precision( - FileHandle, (uint)resolution, errbuf); + FileHandle, (uint)resolution, out errbuf); } catch (TypeLoadException ex) { diff --git a/SharpPcap/LibPcap/LibPcapLiveDevice.cs b/SharpPcap/LibPcap/LibPcapLiveDevice.cs index 2827dcd9..6e9dc65a 100644 --- a/SharpPcap/LibPcap/LibPcapLiveDevice.cs +++ b/SharpPcap/LibPcap/LibPcapLiveDevice.cs @@ -102,7 +102,7 @@ public override void Open(DeviceConfiguration configuration) // See https://www.tcpdump.org/manpages/pcap_set_immediate_mode.3pcap.html var mintocopy_supported = RuntimeInformation.IsOSPlatform(OSPlatform.Windows); - var errbuf = new StringBuilder(Pcap.PCAP_ERRBUF_SIZE); //will hold errors + ErrorBuffer errbuf; //will hold errors // set the StopCaptureTimeout value to twice the read timeout to ensure that // we wait long enough before considering the capture thread to be stuck when stopping @@ -131,7 +131,7 @@ public override void Open(DeviceConfiguration configuration) { Handle = LibPcapSafeNativeMethods.pcap_create( Name, // name of the device - errbuf); // error buffer + out errbuf); // error buffer if (Handle.IsInvalid) { @@ -171,7 +171,7 @@ public override void Open(DeviceConfiguration configuration) (short)mode, // flags (short)configuration.ReadTimeout, // read timeout ref auth, // authentication - errbuf); // error buffer + out errbuf); // error buffer } catch (TypeLoadException) { @@ -275,14 +275,12 @@ public bool NonBlockingMode get { ThrowIfNotOpen("Can't get blocking mode, the device is closed"); - - var errbuf = new StringBuilder(Pcap.PCAP_ERRBUF_SIZE); //will hold errors - int ret = LibPcapSafeNativeMethods.pcap_getnonblock(Handle, errbuf); + int ret = LibPcapSafeNativeMethods.pcap_getnonblock(Handle, out var errbuf); // Errorbuf is only filled when ret = -1 if (ret == -1) { - string err = "Unable to set get blocking" + errbuf.ToString(); + string err = "Unable to get blocking mode. " + errbuf.ToString(); throw new PcapException(err); } @@ -294,18 +292,16 @@ public bool NonBlockingMode { ThrowIfNotOpen("Can't set blocking mode, the device is closed"); - var errbuf = new StringBuilder(Pcap.PCAP_ERRBUF_SIZE); //will hold errors - int block = disableBlocking; if (value) block = enableBlocking; - int ret = LibPcapSafeNativeMethods.pcap_setnonblock(Handle, block, errbuf); + int ret = LibPcapSafeNativeMethods.pcap_setnonblock(Handle, block, out var errbuf); // Errorbuf is only filled when ret = -1 if (ret == -1) { - string err = "Unable to set non blocking" + errbuf.ToString(); + string err = "Unable to set blocking mode. " + errbuf.ToString(); throw new PcapException(err); } } diff --git a/SharpPcap/LibPcap/LibPcapSafeNativeMethods.Encoding.cs b/SharpPcap/LibPcap/LibPcapSafeNativeMethods.Encoding.cs index 693a396d..18fa7805 100644 --- a/SharpPcap/LibPcap/LibPcapSafeNativeMethods.Encoding.cs +++ b/SharpPcap/LibPcap/LibPcapSafeNativeMethods.Encoding.cs @@ -15,9 +15,9 @@ internal static partial class LibPcapSafeNativeMethods { /// - /// This defaul is good enough for .NET Framework and .NET Core on non Windows with Libpcap default config + /// This default is good enough for .NET Framework and .NET Core on non Windows with Libpcap default config /// - private static readonly Encoding StringEncoding = Encoding.Default; + internal static readonly Encoding StringEncoding = Encoding.Default; private static Encoding ConfigureStringEncoding() { @@ -29,9 +29,8 @@ private static Encoding ConfigureStringEncoding() try { // Try to change Libpcap to UTF-8 mode - var errorBuffer = new StringBuilder(Pcap.PCAP_ERRBUF_SIZE); const uint PCAP_CHAR_ENC_UTF_8 = 1; - var res = pcap_init(PCAP_CHAR_ENC_UTF_8, errorBuffer); + var res = pcap_init(PCAP_CHAR_ENC_UTF_8, out _); if (res == 0) { // We made it @@ -90,25 +89,11 @@ public IntPtr MarshalManagedToNative(object managedObj) { return IntPtr.Zero; } - byte[] bytes = null; - var byteCount = 0; - if (managedObj is string str) - { - bytes = StringEncoding.GetBytes(str); - byteCount = bytes.Length + 1; - } - - if (managedObj is StringBuilder builder) - { - bytes = StringEncoding.GetBytes(builder.ToString()); - byteCount = StringEncoding.GetMaxByteCount(builder.Capacity) + 1; - } - - if (bytes is null) - { - throw new ArgumentException("The input argument is not a supported type."); - } - var ptr = Marshal.AllocHGlobal(byteCount); + var str = (string)managedObj; + var bytes = StringEncoding.GetBytes(str); + // The problem is that we need a reference to the StringBuilder in MarshalNativeToManaged + // So we get a pointer to it with GCHandle, and put it as prefix of the pointer we return + var ptr = Marshal.AllocHGlobal(bytes.Length + 1); Marshal.Copy(bytes, 0, ptr, bytes.Length); // Put zero string termination Marshal.WriteByte(ptr + bytes.Length, 0); @@ -131,4 +116,21 @@ public unsafe object MarshalNativeToManaged(IntPtr nativeData) } } } + + [StructLayout(LayoutKind.Sequential)] + internal struct ErrorBuffer + { + [MarshalAs(UnmanagedType.ByValArray, SizeConst = 256)] + internal byte[] Data; + + public override string ToString() + { + var nbBytes = 0; + while (Data[nbBytes] != 0) + { + nbBytes++; + } + return LibPcapSafeNativeMethods.StringEncoding.GetString(Data, 0, nbBytes); + } + } } diff --git a/SharpPcap/LibPcap/LibPcapSafeNativeMethods.Interop.cs b/SharpPcap/LibPcap/LibPcapSafeNativeMethods.Interop.cs index bb77b61d..3c6fe3ff 100644 --- a/SharpPcap/LibPcap/LibPcapSafeNativeMethods.Interop.cs +++ b/SharpPcap/LibPcap/LibPcapSafeNativeMethods.Interop.cs @@ -5,11 +5,8 @@ // SPDX-License-Identifier: MIT using System; -using System.IO; -using System.Reflection; using System.Runtime.InteropServices; using System.Security; -using System.Text; using static SharpPcap.LibPcap.PcapUnmanagedStructures; namespace SharpPcap.LibPcap @@ -30,13 +27,13 @@ internal static partial class LibPcapSafeNativeMethods [DllImport(PCAP_DLL, CallingConvention = CallingConvention.Cdecl)] internal extern static int pcap_init( uint opts, - [MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(PcapStringMarshaler))] StringBuilder /* char* */ errbuf + out ErrorBuffer /* char* */ errbuf ); [DllImport(PCAP_DLL, CallingConvention = CallingConvention.Cdecl)] internal extern static int pcap_findalldevs( ref IntPtr /* pcap_if_t** */ alldevs, - [MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(PcapStringMarshaler))] StringBuilder /* char* */ errbuf + out ErrorBuffer /* char* */ errbuf ); [DllImport(PCAP_DLL, CallingConvention = CallingConvention.Cdecl)] @@ -44,7 +41,7 @@ internal extern static int pcap_findalldevs_ex( [MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(PcapStringMarshaler))] string /*char **/source, ref pcap_rmtauth /*pcap_rmtauth **/auth, ref IntPtr /*pcap_if_t ** */alldevs, - [MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(PcapStringMarshaler))] StringBuilder /*char * */errbuf + out ErrorBuffer /* char* */ errbuf ); [DllImport(PCAP_DLL, CallingConvention = CallingConvention.Cdecl)] @@ -57,19 +54,19 @@ internal extern static int pcap_findalldevs_ex( int flags, int read_timeout, ref pcap_rmtauth rmtauth, - [MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(PcapStringMarshaler))] StringBuilder errbuf + out ErrorBuffer /* char* */ errbuf ); [DllImport(PCAP_DLL, CallingConvention = CallingConvention.Cdecl)] internal extern static PcapHandle /* pcap_t* */ pcap_create( [MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(PcapStringMarshaler))] string dev, - [MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(PcapStringMarshaler))] StringBuilder errbuf + out ErrorBuffer /* char* */ errbuf ); [DllImport(PCAP_DLL, CallingConvention = CallingConvention.Cdecl)] internal extern static PcapHandle /* pcap_t* */ pcap_open_offline( [MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(PcapStringMarshaler))] string/*const char* */ fname, - [MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(PcapStringMarshaler))] StringBuilder/* char* */ errbuf + out ErrorBuffer /* char* */ errbuf ); [DllImport(PCAP_DLL, CallingConvention = CallingConvention.Cdecl)] @@ -187,7 +184,7 @@ internal extern static int pcap_compile( internal extern static int pcap_setnonblock( PcapHandle /* pcap_if_t** */ adaptHandle, int nonblock, - [MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(PcapStringMarshaler))] StringBuilder /* char* */ errbuf + out ErrorBuffer /* char* */ errbuf ); /// @@ -196,7 +193,7 @@ internal extern static int pcap_setnonblock( [DllImport(PCAP_DLL, CallingConvention = CallingConvention.Cdecl)] internal extern static int pcap_getnonblock( PcapHandle /* pcap_if_t** */ adaptHandle, - [MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(PcapStringMarshaler))] StringBuilder /* char* */ errbuf + out ErrorBuffer /* char* */ errbuf ); /// @@ -422,7 +419,7 @@ internal extern static int pcap_tstamp_type_name_to_val( internal extern static PcapHandle /* pcap_t* */ pcap_open_offline_with_tstamp_precision( [MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(PcapStringMarshaler))] string /* const char* */ fname, uint precision, - [MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(PcapStringMarshaler))] StringBuilder /* char* */ errbuf + out ErrorBuffer /* char* */ errbuf ); /// @@ -488,7 +485,7 @@ internal extern static int pcap_tstamp_type_name_to_val( internal extern static IntPtr /* pcap_t* */ _pcap_hopen_offline_with_tstamp_precision( SafeHandle handle, uint precision, - [MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(PcapStringMarshaler))] StringBuilder /* char* */ errbuf + out ErrorBuffer /* char* */ errbuf ); /// @@ -503,7 +500,7 @@ internal extern static int pcap_tstamp_type_name_to_val( internal extern static IntPtr /* pcap_t* */ _pcap_fopen_offline_with_tstamp_precision( SafeHandle fileObject, uint precision, - [MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(PcapStringMarshaler))] StringBuilder /* char* */ errbuf + out ErrorBuffer /* char* */ errbuf ); } } diff --git a/SharpPcap/LibPcap/LibPcapSafeNativeMethods.cs b/SharpPcap/LibPcap/LibPcapSafeNativeMethods.cs index c31fd2eb..177935be 100644 --- a/SharpPcap/LibPcap/LibPcapSafeNativeMethods.cs +++ b/SharpPcap/LibPcap/LibPcapSafeNativeMethods.cs @@ -93,11 +93,11 @@ internal static int pcap_get_tstamp_precision(PcapHandle /* pcap_t* p */ adapter /// Buffer that will receive an error description if an error occurs. /// internal static PcapHandle pcap_open_handle_offline_with_tstamp_precision( - SafeHandle handle, uint precision, StringBuilder errbuf) + SafeHandle handle, uint precision, out ErrorBuffer errbuf) { var pointer = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) - ? _pcap_hopen_offline_with_tstamp_precision(handle, precision, errbuf) - : _pcap_fopen_offline_with_tstamp_precision(handle, precision, errbuf); + ? _pcap_hopen_offline_with_tstamp_precision(handle, precision, out errbuf) + : _pcap_fopen_offline_with_tstamp_precision(handle, precision, out errbuf); if (pointer == IntPtr.Zero) { return PcapHandle.Invalid; diff --git a/SharpPcap/LibPcap/PcapInterface.cs b/SharpPcap/LibPcap/PcapInterface.cs index 6f289835..caa5dceb 100644 --- a/SharpPcap/LibPcap/PcapInterface.cs +++ b/SharpPcap/LibPcap/PcapInterface.cs @@ -176,15 +176,14 @@ static public IReadOnlyList GetAllPcapInterfaces(IPEndPoint sourc static public IReadOnlyList GetAllPcapInterfaces(string source, RemoteAuthentication credentials) { var devicePtr = IntPtr.Zero; - var errorBuffer = new StringBuilder(Pcap.PCAP_ERRBUF_SIZE); var auth = RemoteAuthentication.CreateAuth(credentials); try { - var result = LibPcapSafeNativeMethods.pcap_findalldevs_ex(source, ref auth, ref devicePtr, errorBuffer); + var result = LibPcapSafeNativeMethods.pcap_findalldevs_ex(source, ref auth, ref devicePtr, out var errbuf); if (result < 0) { - throw new PcapException(errorBuffer.ToString()); + throw new PcapException(errbuf.ToString()); } } catch (TypeLoadException ex) @@ -206,12 +205,11 @@ static public IReadOnlyList GetAllPcapInterfaces(string source, R static public IReadOnlyList GetAllPcapInterfaces() { var devicePtr = IntPtr.Zero; - var errorBuffer = new StringBuilder(Pcap.PCAP_ERRBUF_SIZE); - int result = LibPcapSafeNativeMethods.pcap_findalldevs(ref devicePtr, errorBuffer); + int result = LibPcapSafeNativeMethods.pcap_findalldevs(ref devicePtr, out var errbuf); if (result < 0) { - throw new PcapException(errorBuffer.ToString()); + throw new PcapException(errbuf.ToString()); } var pcapInterfaces = GetAllPcapInterfaces(devicePtr, null); @@ -260,8 +258,7 @@ public System.Collections.Generic.List TimestampsSupported { get { - StringBuilder errbuf = new StringBuilder(Pcap.PCAP_ERRBUF_SIZE); //will hold errors - using (var handle = LibPcapSafeNativeMethods.pcap_create(Name, errbuf)) + using (var handle = LibPcapSafeNativeMethods.pcap_create(Name, out var errbuf)) { IntPtr typePtr = IntPtr.Zero; diff --git a/SharpPcap/Pcap.cs b/SharpPcap/Pcap.cs index 3c173bac..dfd7cc55 100644 --- a/SharpPcap/Pcap.cs +++ b/SharpPcap/Pcap.cs @@ -21,7 +21,6 @@ public class Pcap /* interface is loopback */ internal const uint PCAP_IF_LOOPBACK = 0x00000001; internal const int MAX_PACKET_SIZE = 65536; - internal const int PCAP_ERRBUF_SIZE = 256; // Constants for address families // These are set in a Pcap static initializer because the values