From 97ea9e3f2e77fbfa1b9ff9fc51a2e96f16d85191 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 29 Sep 2026 02:27:13 +0000 Subject: [PATCH 1/2] Read GError.message at its real offset on 64-bit Linux [patch] ThrowIfError read the message pointer at IntPtr.Size * 2, which is offset 16 on x64: past the end of glib's GError { guint32 domain; gint code; gchar *message; }, whose message sits at offset 8. Every libsecret failure produced a garbage message or faulted in PtrToStringUTF8. GError is now declared as a sequential struct and the message read through it, so the layout is written down rather than hand-computed. Fixes ktsu-dev/CredentialCache#163 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01UnqwfbU2boDDiUoqRZSY2D --- CredentialCache.Test/GErrorTests.cs | 60 +++++++++++++++++++ CredentialCache/Storage/GError.cs | 29 +++++++++ .../LinuxSecretServiceCredentialStore.cs | 3 +- 3 files changed, 90 insertions(+), 2 deletions(-) create mode 100644 CredentialCache.Test/GErrorTests.cs create mode 100644 CredentialCache/Storage/GError.cs diff --git a/CredentialCache.Test/GErrorTests.cs b/CredentialCache.Test/GErrorTests.cs new file mode 100644 index 0000000..21a1384 --- /dev/null +++ b/CredentialCache.Test/GErrorTests.cs @@ -0,0 +1,60 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.CredentialCache.Test; + +using System.Runtime.InteropServices; +using ktsu.CredentialCache.Storage; + +/// +/// Covers reading the message out of a glib GError, which the Linux store does on +/// every libsecret failure. The GError here is hand-built, so this runs on every platform. +/// +[TestClass] +public class GErrorTests +{ + // GError is { guint32 domain; gint code; gchar *message; }, so the message pointer + // sits at offset 8 on both 32-bit and 64-bit, with no padding before it. + private const int MessageOffset = sizeof(uint) + sizeof(int); + + [TestMethod] + public void ReadMessageReturnsTheMessageField() + { + IntPtr message = Marshal.StringToCoTaskMemUTF8("Cannot autolaunch D-Bus without X11 $DISPLAY"); + + // One pointer longer than the struct and zeroed, so a read past the end of the struct + // finds a null pointer rather than whatever happens to follow the allocation. + int size = MessageOffset + (IntPtr.Size * 2); + IntPtr error = Marshal.AllocHGlobal(size); + try + { + Marshal.Copy(new byte[size], 0, error, size); + Marshal.WriteInt32(error, 0, 42); + Marshal.WriteInt32(error, sizeof(uint), 7); + Marshal.WriteIntPtr(error, MessageOffset, message); + + Assert.AreEqual("Cannot autolaunch D-Bus without X11 $DISPLAY", GError.ReadMessage(error)); + } + finally + { + Marshal.FreeHGlobal(error); + Marshal.FreeCoTaskMem(message); + } + } + + [TestMethod] + public void ReadMessageReturnsNullForANullMessage() + { + int size = MessageOffset + (IntPtr.Size * 2); + IntPtr error = Marshal.AllocHGlobal(size); + try + { + Marshal.Copy(new byte[size], 0, error, size); + + Assert.IsNull(GError.ReadMessage(error)); + } + finally + { + Marshal.FreeHGlobal(error); + } + } +} diff --git a/CredentialCache/Storage/GError.cs b/CredentialCache/Storage/GError.cs new file mode 100644 index 0000000..d5efe1a --- /dev/null +++ b/CredentialCache/Storage/GError.cs @@ -0,0 +1,29 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.CredentialCache.Storage; + +using System.Runtime.InteropServices; + +/// +/// glib's GError: { guint32 domain; gint code; gchar *message; }. +/// +/// +/// Declared rather than read at a hand-computed offset. The message pointer follows two +/// 4-byte fields, so it sits at offset 8 on both 32-bit and 64-bit; an offset derived from +/// reads past the end of the struct on 64-bit. +/// +[StructLayout(LayoutKind.Sequential)] +internal readonly struct GError +{ + internal readonly uint Domain; + internal readonly int Code; + internal readonly IntPtr Message; + + /// + /// Reads the message of the GError at . + /// + /// A non-null pointer to a GError. + /// The message, or if the error has none. + internal static string? ReadMessage(IntPtr error) => + Marshal.PtrToStringUTF8(Marshal.PtrToStructure(error).Message); +} diff --git a/CredentialCache/Storage/LinuxSecretServiceCredentialStore.cs b/CredentialCache/Storage/LinuxSecretServiceCredentialStore.cs index 6e1b930..e6d225f 100644 --- a/CredentialCache/Storage/LinuxSecretServiceCredentialStore.cs +++ b/CredentialCache/Storage/LinuxSecretServiceCredentialStore.cs @@ -139,8 +139,7 @@ private static void ThrowIfError(IntPtr error, string operation) string? message = null; try { - IntPtr messagePtr = Marshal.ReadIntPtr(error, IntPtr.Size * 2); - message = Marshal.PtrToStringUTF8(messagePtr); + message = GError.ReadMessage(error); } catch { From 39c66956778977c32311e4c8506bd3509e81aefb Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 29 Sep 2026 02:37:19 +0000 Subject: [PATCH 2/2] Cover ThrowIfError with a GError built by glib SonarCloud flagged the ThrowIfError call site as uncovered new code. The new test has glib build a real GError and checks that ThrowIfError reports its message. It runs on Linux, where glib is present, and reports inconclusive elsewhere. With the old IntPtr.Size * 2 offset it kills the test host with an AccessViolationException. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01UnqwfbU2boDDiUoqRZSY2D --- CredentialCache.Test/GErrorTests.cs | 31 +++++++++++++++++++ .../LinuxSecretServiceCredentialStore.cs | 2 +- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/CredentialCache.Test/GErrorTests.cs b/CredentialCache.Test/GErrorTests.cs index 21a1384..aa3edf4 100644 --- a/CredentialCache.Test/GErrorTests.cs +++ b/CredentialCache.Test/GErrorTests.cs @@ -3,6 +3,7 @@ namespace ktsu.CredentialCache.Test; using System.Runtime.InteropServices; +using System.Runtime.Versioning; using ktsu.CredentialCache.Storage; /// @@ -57,4 +58,34 @@ public void ReadMessageReturnsNullForANullMessage() Marshal.FreeHGlobal(error); } } + + [TestMethod] + public void ThrowIfErrorReportsTheMessageOfARealGError() + { + if (!OperatingSystem.IsLinux()) + { + Assert.Inconclusive("GError comes from glib, which is only loaded on Linux."); + return; + } + + AssertThrowIfErrorReportsTheMessage(); + } + + // Built by glib itself rather than by hand, so this checks the real layout. glib frees + // the error inside ThrowIfError. + [SupportedOSPlatform("linux")] + private static void AssertThrowIfErrorReportsTheMessage() + { + IntPtr glib = NativeLibrary.Load("libglib-2.0.so.0"); + GErrorNewLiteral newLiteral = Marshal.GetDelegateForFunctionPointer( + NativeLibrary.GetExport(glib, "g_error_new_literal")); + IntPtr error = newLiteral(1, 2, "Cannot autolaunch D-Bus without X11 $DISPLAY"); + + CredentialStoreException exception = Assert.ThrowsExactly( + () => LinuxSecretServiceCredentialStore.ThrowIfError(error, "secret_password_lookup_sync")); + + Assert.AreEqual("secret_password_lookup_sync failed: Cannot autolaunch D-Bus without X11 $DISPLAY", exception.Message); + } + + private delegate IntPtr GErrorNewLiteral(uint domain, int code, [MarshalAs(UnmanagedType.LPUTF8Str)] string message); } diff --git a/CredentialCache/Storage/LinuxSecretServiceCredentialStore.cs b/CredentialCache/Storage/LinuxSecretServiceCredentialStore.cs index e6d225f..e4e7079 100644 --- a/CredentialCache/Storage/LinuxSecretServiceCredentialStore.cs +++ b/CredentialCache/Storage/LinuxSecretServiceCredentialStore.cs @@ -130,7 +130,7 @@ public bool Remove(PersonaGUID persona) return removed; } - private static void ThrowIfError(IntPtr error, string operation) + internal static void ThrowIfError(IntPtr error, string operation) { if (error == IntPtr.Zero) {