From 2ecdd147a71c1585979e2421ea26e1901a1d3e6e Mon Sep 17 00:00:00 2001 From: Kevin Bataille Date: Tue, 21 Jul 2026 19:59:48 +0200 Subject: [PATCH] =?UTF-8?q?review=20fixes:=20rename=20CallingConvention?= =?UTF-8?q?=E2=86=92CallConvention,=20seal,=20edge=20cases?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit HIGH: rename CallingConvention to CallConvention to avoid BCL collision with System.Runtime.InteropServices.CallingConvention. FIXES: - checked(uint) casts for x86 pointer truncation (ArgumentOverflow) - checked distance for E8 rel32 range (>2 GiB → throw) - add esp, imm32 (81 /0 id) when cleanup > 127 bytes - pointerSize validation (throw on != 4 and != 8) - switch default: throw on unknown convention - track argIndex instead of args[1..] slicing - EmitMovRegImm32 helper (avoids manual ip tracking bugs) - seal StubAssembler - IAssembler doc: note BuildCallStub is StubAssembler-specific - thiscall 0-args throws test; fastcall 0-args is valid - update remote-execution spec example to CallConvention.Cdecl All passing (total: 93). --- ...CallingConvention.cs => CallConvention.cs} | 4 +- WhiteMagic/Assembly/IAssembler.cs | 5 + WhiteMagic/Assembly/StubAssembler.cs | 145 +++++++++++++----- WhiteMagicTest/StubAssemblerTests.cs | 89 +++++++++-- .../specs/remote-execution/spec.md | 2 +- 5 files changed, 185 insertions(+), 60 deletions(-) rename WhiteMagic/Assembly/{CallingConvention.cs => CallConvention.cs} (76%) diff --git a/WhiteMagic/Assembly/CallingConvention.cs b/WhiteMagic/Assembly/CallConvention.cs similarity index 76% rename from WhiteMagic/Assembly/CallingConvention.cs rename to WhiteMagic/Assembly/CallConvention.cs index 5a16a85..88ae08e 100644 --- a/WhiteMagic/Assembly/CallingConvention.cs +++ b/WhiteMagic/Assembly/CallConvention.cs @@ -2,8 +2,10 @@ namespace WhiteMagic.Assembly; /// /// x86/x86-64 calling conventions for call-stub generation. +/// Named CallConvention (not CallingConvention) to avoid ambiguity with +/// . /// -public enum CallingConvention +public enum CallConvention { /// Caller pushes args right-to-left and cleans the stack (x86). Cdecl, diff --git a/WhiteMagic/Assembly/IAssembler.cs b/WhiteMagic/Assembly/IAssembler.cs index 321c4d8..a924560 100644 --- a/WhiteMagic/Assembly/IAssembler.cs +++ b/WhiteMagic/Assembly/IAssembler.cs @@ -6,6 +6,11 @@ namespace WhiteMagic.Assembly; /// (Phase 8) handles arbitrary mnemonics via the Iced /// library. /// +/// +/// This seam covers text assembly only (). Call-stub building +/// (BuildCallStub, EmitU8/EmitU32/EmitU64) is a +/// capability — not all backends need it. +/// public interface IAssembler { /// diff --git a/WhiteMagic/Assembly/StubAssembler.cs b/WhiteMagic/Assembly/StubAssembler.cs index e22db38..ca35ed0 100644 --- a/WhiteMagic/Assembly/StubAssembler.cs +++ b/WhiteMagic/Assembly/StubAssembler.cs @@ -11,7 +11,7 @@ namespace WhiteMagic.Assembly; /// emitter, not a text assembler). Use (Phase 8) for /// arbitrary mnemonics. /// -public class StubAssembler : IAssembler +public sealed class StubAssembler : IAssembler { /// public byte[] Assemble(string assemblyText, ulong origin = 0) @@ -41,70 +41,113 @@ public class StubAssembler : IAssembler // ── Call-stub builders ───────────────────────────────────────────────── + /// + /// Builds a calling-convention call stub for x86 or x64. + /// + /// Where the stub lands (for E8 rel32 encoding). + /// Function to call. + /// Argument values (uint[] — each 4 or 8 bytes per pointerSize). + /// 4 (x86) or 8 (x64). + /// Calling convention. + /// is not 4 or 8, + /// or is not known, or the distance between stub and target + /// exceeds the E8 rel32 range. public byte[] BuildCallStub(IntPtr stubAddress, IntPtr targetAddress, - uint[] arguments, int pointerSize, CallingConvention convention) + uint[] arguments, int pointerSize, CallConvention convention) { var buffer = new List(64); if (pointerSize == 4) - BuildX86Stub(buffer, (uint)stubAddress, (uint)targetAddress, arguments, convention); + { + BuildX86Stub(buffer, checked((uint)stubAddress), checked((uint)targetAddress), + arguments, convention); + } + else if (pointerSize == 8) + { + // Windows x64 uses a single ABI — the convention parameter is unused. + BuildX64Stub(buffer, (ulong)(nint)stubAddress, (ulong)(nint)targetAddress, arguments); + } else - BuildX64Stub(buffer, (ulong)stubAddress, (ulong)targetAddress, arguments); + { + throw new ArgumentOutOfRangeException(nameof(pointerSize), pointerSize, + $"Expected 4 (x86) or 8 (x64), got {pointerSize}."); + } return buffer.ToArray(); } private void BuildX86Stub(List buffer, uint stubAddr, - uint target, uint[] args, CallingConvention convention) + uint target, uint[] args, CallConvention convention) { uint current = stubAddr; + int argIndex = 0; switch (convention) { - case CallingConvention.Thiscall when args.Length >= 1: - buffer.Add(0xB9); // mov ecx, arg0 - EmitU32(buffer, args[0]); - current += 5; - args = args[1..]; + case CallConvention.Thiscall when args.Length - argIndex >= 1: + EmitMovRegImm32(buffer, 0xB9, args[argIndex], ref current); // mov ecx, arg0 + argIndex++; break; - case CallingConvention.Fastcall: - if (args.Length >= 1) + case CallConvention.Fastcall: + if (args.Length - argIndex >= 1) { - buffer.Add(0xB9); // mov ecx, arg0 - EmitU32(buffer, args[0]); - current += 5; - args = args[1..]; + EmitMovRegImm32(buffer, 0xB9, args[argIndex], ref current); // mov ecx, arg0 + argIndex++; } - if (args.Length >= 1) + if (args.Length - argIndex >= 1) { - buffer.Add(0xBA); // mov edx, arg1 - EmitU32(buffer, args[0]); - current += 5; - args = args[1..]; + EmitMovRegImm32(buffer, 0xBA, args[argIndex], ref current); // mov edx, arg1 + argIndex++; } break; + + case CallConvention.Cdecl: + case CallConvention.Stdcall: + break; + + default: + throw new ArgumentOutOfRangeException(nameof(convention), convention, + $"Unsupported calling convention: {convention}."); } - // Push remaining args in reverse order - for (int i = args.Length - 1; i >= 0; i--) + // Push remaining args in reverse order (right-to-left) + for (int i = args.Length - 1; i >= argIndex; i--) { + current += 5; buffer.Add(0x68); // push imm32 EmitU32(buffer, args[i]); - current += 5; } // call rel32 - uint rel32 = target - (current + 5); + long distance = (long)target - (long)(current + 5); + if (distance < int.MinValue || distance > int.MaxValue) + { + throw new ArgumentOutOfRangeException( + $"target (0x{target:X}) is >2 GiB from stub (0x{stubAddr:X}); " + + "E8 rel32 cannot encode this distance. Place the stub closer to the target."); + } buffer.Add(0xE8); - EmitU32(buffer, rel32); + EmitU32(buffer, (uint)distance); + current += 5; // Caller cleanup (cdecl only) - if (convention == CallingConvention.Cdecl && args.Length > 0) + int stackCount = args.Length - argIndex; + if (convention == CallConvention.Cdecl && stackCount > 0) { - buffer.Add(0x83); // add esp, imm8 - buffer.Add(0xC4); - buffer.Add((byte)(args.Length * 4)); + int cleanup = stackCount * 4; + if (cleanup <= 127) + { + buffer.Add(0x83); // add esp, imm8 + buffer.Add(0xC4); + buffer.Add((byte)cleanup); + } + else + { + buffer.Add(0x81); // add esp, imm32 + buffer.Add(0xC4); + EmitU32(buffer, (uint)cleanup); + } } buffer.Add(0xC3); // ret @@ -113,11 +156,11 @@ public class StubAssembler : IAssembler private void BuildX64Stub(List buffer, ulong stubAddr, ulong target, uint[] args) { + // Windows x64 single ABI: first 4 args in RCX, RDX, R8D, R9D. ulong current = stubAddr; - // First 4 args go in RCX, RDX, R8D, R9D - var regCodes = new byte[] { 0xB9, 0xBA, 0xB8, 0xB9 }; // mov ecx/r8d/edx/r9d, imm32 - var rexBytes = new byte[] { 0x00, 0x00, 0x41, 0x41 }; // 0x00 = no REX, 0x41 = REX.B + var regCodes = new byte[] { 0xB9, 0xBA, 0xB8, 0xB9 }; + var rexBytes = new byte[] { 0x00, 0x00, 0x41, 0x41 }; int regCount = Math.Min(args.Length, 4); for (int i = 0; i < regCount; i++) @@ -129,33 +172,51 @@ public class StubAssembler : IAssembler current += (rexBytes[i] != 0 ? 6u : 5u); } - // Push remaining args in reverse order (right-to-left) + // Push remaining args in reverse order for (int i = args.Length - 1; i >= 4; i--) { - buffer.Add(0x68); // push imm32 - EmitU32(buffer, args[i]); current += 5; + buffer.Add(0x68); + EmitU32(buffer, args[i]); } // call rel32 - uint rel32 = (uint)(target - (current + 5)); + long distance = (long)target - (long)(current + 5); + if (distance < int.MinValue || distance > int.MaxValue) + { + throw new ArgumentOutOfRangeException( + "target and stub are >2 GiB apart; E8 rel32 cannot encode this distance."); + } buffer.Add(0xE8); - EmitU32(buffer, rel32); + EmitU32(buffer, (uint)distance); - // Caller cleanup: pop any args pushed on stack + // Pop any args pushed on stack (x64 is caller-clean) int stackArgs = args.Length > 4 ? args.Length - 4 : 0; if (stackArgs > 0) { int bytes = stackArgs * 8; - buffer.Add(0x48); // REX.W + buffer.Add(0x48); // REX.W buffer.Add(bytes <= 127 ? (byte)0x83 : (byte)0x81); // add r/m64, imm8/imm32 - buffer.Add(0xC4); // rsp + buffer.Add(0xC4); // rsp if (bytes <= 127) buffer.Add((byte)bytes); else EmitU32(buffer, (uint)bytes); } - buffer.Add(0xC3); // ret + buffer.Add(0xC3); + } + + // ── Instruction helpers ──────────────────────────────────────────────── + + /// Emit mov reg32, imm32 and advances by 5. + private static void EmitMovRegImm32(List buffer, byte opcode, uint imm32, ref uint ip) + { + buffer.Add(opcode); + buffer.Add((byte)imm32); + buffer.Add((byte)(imm32 >> 8)); + buffer.Add((byte)(imm32 >> 16)); + buffer.Add((byte)(imm32 >> 24)); + ip += 5; } } diff --git a/WhiteMagicTest/StubAssemblerTests.cs b/WhiteMagicTest/StubAssemblerTests.cs index ec8f01a..89acb4e 100644 --- a/WhiteMagicTest/StubAssemblerTests.cs +++ b/WhiteMagicTest/StubAssemblerTests.cs @@ -23,7 +23,7 @@ public class StubAssemblerTests { uint r = 0x12345678u-(0x10000000u+5); Assert.Equal([0xE8,(byte)r,(byte)(r>>8),(byte)(r>>16),(byte)(r>>24),0xC3], - Create().BuildCallStub((IntPtr)0x10000000,(IntPtr)0x12345678,[],4,CallingConvention.Cdecl)); + Create().BuildCallStub((IntPtr)0x10000000,(IntPtr)0x12345678,[],4,CallConvention.Cdecl)); } [Fact] @@ -34,7 +34,7 @@ public class StubAssemblerTests 0x68,0xDD,0xCC,0xBB,0xAA, 0xE8,(byte)r,(byte)(r>>8),(byte)(r>>16),(byte)(r>>24), 0x83,0xC4,0x04,0xC3], - Create().BuildCallStub((IntPtr)0x10000000,(IntPtr)0x12345678,[0xAABBCCDD],4,CallingConvention.Cdecl)); + Create().BuildCallStub((IntPtr)0x10000000,(IntPtr)0x12345678,[0xAABBCCDD],4,CallConvention.Cdecl)); } [Fact] @@ -46,7 +46,7 @@ public class StubAssemblerTests 0x68,0x11,0x11,0x11,0x11, 0xE8,(byte)r,(byte)(r>>8),(byte)(r>>16),(byte)(r>>24), 0x83,0xC4,0x08,0xC3], - Create().BuildCallStub((IntPtr)0x10000000,(IntPtr)0x12345678,[0x11111111,0x22222222],4,CallingConvention.Cdecl)); + Create().BuildCallStub((IntPtr)0x10000000,(IntPtr)0x12345678,[0x11111111,0x22222222],4,CallConvention.Cdecl)); } // ── x86 stdcall ────────────────────────────────────────────────────── @@ -58,7 +58,7 @@ public class StubAssemblerTests Assert.Equal([ 0x68,0xDD,0xCC,0xBB,0xAA, 0xE8,(byte)r,(byte)(r>>8),(byte)(r>>16),(byte)(r>>24),0xC3], - Create().BuildCallStub((IntPtr)0x10000000,(IntPtr)0x12345678,[0xAABBCCDD],4,CallingConvention.Stdcall)); + Create().BuildCallStub((IntPtr)0x10000000,(IntPtr)0x12345678,[0xAABBCCDD],4,CallConvention.Stdcall)); } [Fact] @@ -69,7 +69,7 @@ public class StubAssemblerTests 0x68,0x22,0x22,0x22,0x22, 0x68,0x11,0x11,0x11,0x11, 0xE8,(byte)r,(byte)(r>>8),(byte)(r>>16),(byte)(r>>24),0xC3], - Create().BuildCallStub((IntPtr)0x10000000,(IntPtr)0x12345678,[0x11111111,0x22222222],4,CallingConvention.Stdcall)); + Create().BuildCallStub((IntPtr)0x10000000,(IntPtr)0x12345678,[0x11111111,0x22222222],4,CallConvention.Stdcall)); } // ── x86 thiscall ───────────────────────────────────────────────────── @@ -82,18 +82,25 @@ public class StubAssemblerTests 0xB9,0x55,0x55,0xAA,0xAA, 0x68,0x66,0x66,0xBB,0xBB, 0xE8,(byte)r,(byte)(r>>8),(byte)(r>>16),(byte)(r>>24),0xC3], - Create().BuildCallStub((IntPtr)0x10000000,(IntPtr)0x12345678,[0xAAAA5555,0xBBBB6666],4,CallingConvention.Thiscall)); + Create().BuildCallStub((IntPtr)0x10000000,(IntPtr)0x12345678,[0xAAAA5555,0xBBBB6666],4,CallConvention.Thiscall)); } [Fact] - public void Thiscall_1arg() + public void Thiscall_1arg_ecx_only() { uint ca=0x10000000u+5,r=0x12345678u-(ca+5); - byte[] s=Create().BuildCallStub((IntPtr)0x10000000,(IntPtr)0x12345678,[0xCAFEBABE],4,CallingConvention.Thiscall); + byte[] s=Create().BuildCallStub((IntPtr)0x10000000,(IntPtr)0x12345678,[0xCAFEBABE],4,CallConvention.Thiscall); Assert.Equal(11,s.Length); Assert.Equal(0xB9,s[0]); Assert.Equal(0xCAFEBABE,BitConverter.ToUInt32(s,1)); Assert.Equal(0xE8,s[5]); Assert.Equal(r,BitConverter.ToUInt32(s,6)); Assert.Equal(0xC3,s[10]); } + [Fact] + public void Thiscall_0args_throws() + { + Assert.Throws(() => + Create().BuildCallStub((IntPtr)0x10000000,(IntPtr)0x12345678,[],4,CallConvention.Thiscall)); + } + // ── x86 fastcall ──────────────────────────────────────────────────── [Fact] @@ -105,19 +112,27 @@ public class StubAssemblerTests 0xBA,0x22,0x22,0x22,0x22, 0x68,0x33,0x33,0x33,0x33, 0xE8,(byte)r,(byte)(r>>8),(byte)(r>>16),(byte)(r>>24),0xC3], - Create().BuildCallStub((IntPtr)0x10000000,(IntPtr)0x12345678,[0x11111111,0x22222222,0x33333333],4,CallingConvention.Fastcall)); + Create().BuildCallStub((IntPtr)0x10000000,(IntPtr)0x12345678,[0x11111111,0x22222222,0x33333333],4,CallConvention.Fastcall)); } [Fact] - public void Fastcall_2args() + public void Fastcall_2args_registers_only() { uint ca=0x10000000u+10,r=0x12345678u-(ca+5); - byte[] s=Create().BuildCallStub((IntPtr)0x10000000,(IntPtr)0x12345678,[0xAAAAAAAA,0xBBBBBBBB],4,CallingConvention.Fastcall); + byte[] s=Create().BuildCallStub((IntPtr)0x10000000,(IntPtr)0x12345678,[0xAAAAAAAA,0xBBBBBBBB],4,CallConvention.Fastcall); Assert.Equal(16,s.Length); Assert.Equal(0xB9,s[0]); Assert.Equal(0xAAAAAAAA,BitConverter.ToUInt32(s,1)); Assert.Equal(0xBA,s[5]); Assert.Equal(0xBBBBBBBB,BitConverter.ToUInt32(s,6)); Assert.Equal(0xE8,s[10]); Assert.Equal(r,BitConverter.ToUInt32(s,11)); Assert.Equal(0xC3,s[15]); } + [Fact] + public void Fastcall_0args_is_valid() + { + uint r=0x12345678u-(0x10000000u+5); + Assert.Equal([0xE8,(byte)r,(byte)(r>>8),(byte)(r>>16),(byte)(r>>24),0xC3], + Create().BuildCallStub((IntPtr)0x10000000,(IntPtr)0x12345678,[],4,CallConvention.Fastcall)); + } + // ── x64 ───────────────────────────────────────────────────────────── [Fact] @@ -125,7 +140,7 @@ public class StubAssemblerTests { var s=Create(); ulong a=0x100000000,t=0x123456788; uint r=(uint)(t-(a+5)); - byte[] stub=s.BuildCallStub((IntPtr)(nint)a,(IntPtr)(nint)t,[],8,CallingConvention.Cdecl); + byte[] stub=s.BuildCallStub((IntPtr)(nint)a,(IntPtr)(nint)t,[],8,CallConvention.Cdecl); Assert.Equal(6,stub.Length); Assert.Equal(0xE8,stub[0]); Assert.Equal(r,BitConverter.ToUInt32(stub,1)); Assert.Equal(0xC3,stub[5]); } @@ -137,7 +152,7 @@ public class StubAssemblerTests Assert.Equal([ 0xB9,0xDD,0xCC,0xBB,0xAA, 0xE8,(byte)r,(byte)(r>>8),(byte)(r>>16),(byte)(r>>24),0xC3], - s.BuildCallStub((IntPtr)(nint)a,(IntPtr)(nint)t,[0xAABBCCDD],8,CallingConvention.Cdecl)); + s.BuildCallStub((IntPtr)(nint)a,(IntPtr)(nint)t,[0xAABBCCDD],8,CallConvention.Cdecl)); } [Fact] @@ -149,7 +164,7 @@ public class StubAssemblerTests 0xB9,0x11,0x11,0x11,0x11, 0xBA,0x22,0x22,0x22,0x22, 0x41,0xB8,0x33,0x33,0x33,0x33, 0x41,0xB9,0x44,0x44,0x44,0x44, 0xE8,(byte)r,(byte)(r>>8),(byte)(r>>16),(byte)(r>>24),0xC3], - s.BuildCallStub((IntPtr)(nint)a,(IntPtr)(nint)t,[0x11111111,0x22222222,0x33333333,0x44444444],8,CallingConvention.Cdecl)); + s.BuildCallStub((IntPtr)(nint)a,(IntPtr)(nint)t,[0x11111111,0x22222222,0x33333333,0x44444444],8,CallConvention.Cdecl)); } [Fact] @@ -163,10 +178,52 @@ public class StubAssemblerTests 0x68,5,0,0,0, 0xE8,(byte)r,(byte)(r>>8),(byte)(r>>16),(byte)(r>>24), 0x48,0x83,0xC4,8, 0xC3], - s.BuildCallStub((IntPtr)(nint)a,(IntPtr)(nint)t,[1,2,3,4,5],8,CallingConvention.Cdecl)); + s.BuildCallStub((IntPtr)(nint)a,(IntPtr)(nint)t,[1,2,3,4,5],8,CallConvention.Cdecl)); } - // ── No-FASM assertion ─────────────────────────────────────────────── + // ── Edge cases ──────────────────────────────────────────────────────── + + [Fact] + public void Far_target_throws() + { + Assert.Throws(() => + Create().BuildCallStub(IntPtr.Zero, (IntPtr)0xC0000000, [], 4, CallConvention.Cdecl)); + } + + [Fact] + public void Many_args_cleanup_uses_imm32_form() + { + var args = new uint[33]; + for (int i = 0; i < 33; i++) args[i] = (uint)(i * 0x10000 + i); + byte[] stub = Create().BuildCallStub( + (IntPtr)0x10000000, (IntPtr)0x12345678, args, 4, CallConvention.Cdecl); + + for (int i = 0; i < stub.Length - 5; i++) + { + if (stub[i] == 0x81 && stub[i + 1] == 0xC4) + { + Assert.Equal(132, BitConverter.ToInt32(stub, i + 2)); + return; + } + } + Assert.Fail("Expected 0x81 0xC4 (add esp, imm32) not found"); + } + + [Fact] + public void Invalid_pointerSize_throws() + { + Assert.Throws(() => + Create().BuildCallStub((IntPtr)0x10000000, (IntPtr)0x12345678, [], 2, CallConvention.Cdecl)); + } + + [Fact] + public void Invalid_calling_convention_throws() + { + Assert.Throws(() => + Create().BuildCallStub((IntPtr)0x10000000, (IntPtr)0x12345678, [], 4, (CallConvention)99)); + } + + // ── No-FASM ───────────────────────────────────────────────────────── [Fact] public void No_fasm_reference_in_output() diff --git a/openspec/changes/whitemagic-foundation/specs/remote-execution/spec.md b/openspec/changes/whitemagic-foundation/specs/remote-execution/spec.md index 90907e9..0d5d70e 100644 --- a/openspec/changes/whitemagic-foundation/specs/remote-execution/spec.md +++ b/openspec/changes/whitemagic-foundation/specs/remote-execution/spec.md @@ -17,7 +17,7 @@ WhiteMagic SHALL provide three execution strategies selected by payload safety: `RemoteThreadExecutor` SHALL create a remote thread at a target address using a calling-convention-aware stub, wait for completion, and return the typed exit value. Its documentation MUST state that it is safe only for thread-agnostic payloads. #### Scenario: execute with parameters and convention -- **WHEN** `Execute(addr, CallingConvention.Cdecl, arg1, arg2)` is called on a safe self-contained function +- **WHEN** `Execute(addr, CallConvention.Cdecl, arg1, arg2)` is called on a safe self-contained function - **THEN** the target MUST be called with the arguments laid out per cdecl and the typed return value returned #### Scenario: parameters marshalled and freed