feat(network): Add safe packet handling and pointer validation

- Add NetworkPointerValidator for validating packet pointers with bounds
  checking and user-mode address space limits
- Add SafePacket wrapper providing lifetime safety, bounds checking, and
  sensitive data clearing on dispose
- Fix integer overflow vulnerabilities in bounds checking (use safe
  subtraction pattern instead of addition)
- Fix pointer validation timing in GameNetwork to validate after
  adjustment
- Add deprecation documentation to IGameNetwork interface
- Add runtime check for scoped service access outside plugin context

Dalamud/Game/Network/GameNetwork.cs

@@ -16,6 +16,26 @@ namespace Dalamud.Game.Network;
 [ServiceManager.EarlyLoadedService]
 internal sealed unsafe class GameNetwork : IInternalDisposableService
 {
+    /// <summary>
+    /// Offset from the data pointer to the start of the packet header.
+    /// </summary>
+    private const int PacketHeaderOffset = 0x10;
+
+    /// <summary>
+    /// Offset from the packet header to where the opcode is located.
+    /// </summary>
+    private const int OpCodeOffset = 0x12;
+
+    /// <summary>
+    /// Offset from the packet header to where the packet data begins.
+    /// </summary>
+    private const int PacketDataOffset = 0x20;
+
+    /// <summary>
+    /// Size of the packet header for validation and logging purposes.
+    /// </summary>
+    private const int PacketHeaderSize = 0x20;
+
     private readonly GameNetworkAddressResolver address;
     private readonly Hook<PacketDispatcher.Delegates.OnReceivePacket> processZonePacketDownHook;
     private readonly Hook<ProcessZonePacketUpDelegate> processZonePacketUpHook;
@@ -77,16 +97,33 @@ internal sealed unsafe class GameNetwork : IInternalDisposableService
     {
         this.hitchDetectorDown.Start();
 
-        // Go back 0x10 to get back to the start of the packet header
+        // Go back to the start of the packet header
-        dataPtr -= 0x10;
+        dataPtr -= PacketHeaderOffset;
+
+        // Validate the adjusted pointer (after moving to packet header start)
+        // We need PacketHeaderSize bytes for the full header + data offset
+        if (!NetworkPointerValidator.IsValidPacketPointer(dataPtr, PacketHeaderSize))
+        {
+            Log.Warning("ProcessZonePacketDown received invalid pointer: {Ptr:X}", dataPtr + PacketHeaderOffset);
+            this.processZonePacketDownHook.Original(dispatcher, targetId, dataPtr + PacketHeaderOffset);
+            this.hitchDetectorDown.Stop();
+            return;
+        }
 
         foreach (var d in Delegate.EnumerateInvocationList(this.NetworkMessage))
         {
             try
             {
+                // Extract opcode with bounds awareness
+                if (!NetworkPointerValidator.TrySafeRead<ushort>(dataPtr, OpCodeOffset, PacketHeaderSize, out var opCode))
+                {
+                    Log.Warning("Failed to read packet opcode at offset {Offset}", OpCodeOffset);
+                    continue;
+                }
+
                 d.Invoke(
-                    dataPtr + 0x20,
+                    dataPtr + PacketDataOffset,
-                    (ushort)Marshal.ReadInt16(dataPtr, 0x12),
+                    opCode,
                     0,
                     targetId,
                     NetworkMessageDirection.ZoneDown);
@@ -96,8 +133,8 @@ internal sealed unsafe class GameNetwork : IInternalDisposableService
                 string header;
                 try
                 {
-                    var data = new byte[32];
+                    var data = new byte[PacketHeaderSize];
-                    Marshal.Copy(dataPtr, data, 0, 32);
+                    Marshal.Copy(dataPtr, data, 0, PacketHeaderSize);
                     header = BitConverter.ToString(data);
                 }
                 catch (Exception)
@@ -109,7 +146,7 @@ internal sealed unsafe class GameNetwork : IInternalDisposableService
             }
         }
 
-        this.processZonePacketDownHook.Original(dispatcher, targetId, dataPtr + 0x10);
+        this.processZonePacketDownHook.Original(dispatcher, targetId, dataPtr + PacketHeaderOffset);
         this.hitchDetectorDown.Stop();
     }
 
@@ -117,19 +154,37 @@ internal sealed unsafe class GameNetwork : IInternalDisposableService
     {
         this.hitchDetectorUp.Start();
 
+        // Validate the incoming pointer before use
+        if (!NetworkPointerValidator.IsValidPacketPointer(dataPtr, PacketHeaderSize))
+        {
+            Log.Warning("ProcessZonePacketUp received invalid pointer: {Ptr:X}", dataPtr);
+            this.hitchDetectorUp.Stop();
+            return this.processZonePacketUpHook.Original(a1, dataPtr, a3, a4);
+        }
+
         try
         {
-            // Call events
+            // Extract opcode with bounds awareness
-            // TODO: Implement actor IDs
+            // Note: Upstream packets have a different structure - opcode is at offset 0, not OpCodeOffset
-            this.NetworkMessage?.Invoke(dataPtr + 0x20, (ushort)Marshal.ReadInt16(dataPtr), 0x0, 0x0, NetworkMessageDirection.ZoneUp);
+            // This is intentional as upstream packet format differs from downstream
+            if (NetworkPointerValidator.TrySafeRead<ushort>(dataPtr, 0, PacketHeaderSize, out var opCode))
+            {
+                // Call events
+                // TODO: Implement actor IDs
+                this.NetworkMessage?.Invoke(dataPtr + PacketDataOffset, opCode, 0x0, 0x0, NetworkMessageDirection.ZoneUp);
+            }
+            else
+            {
+                Log.Warning("Failed to read packet opcode for upstream packet");
+            }
         }
         catch (Exception ex)
         {
             string header;
             try
             {
-                var data = new byte[32];
+                var data = new byte[PacketHeaderSize];
-                Marshal.Copy(dataPtr, data, 0, 32);
+                Marshal.Copy(dataPtr, data, 0, PacketHeaderSize);
                 header = BitConverter.ToString(data);
             }
             catch (Exception)

Dalamud/Game/Network/NetworkPointerValidator.cs

@@ -0,0 +1,101 @@
+using System.Runtime.InteropServices;
+
+namespace Dalamud.Game.Network;
+
+/// <summary>
+/// Provides validation utilities for network packet pointers.
+/// </summary>
+internal static class NetworkPointerValidator
+{
+    /// <summary>
+    /// Minimum address threshold below which pointers are considered invalid.
+    /// Addresses below this are typically reserved by the OS.
+    /// </summary>
+    private const long MinValidAddress = 0x10000;
+
+    /// <summary>
+    /// Maximum valid user-mode address for 64-bit Windows.
+    /// Addresses above this are kernel-mode and inaccessible from user-mode.
+    /// </summary>
+    private const long MaxValidAddress = 0x7FFFFFFFFFFF;
+
+    /// <summary>
+    /// Validates a network packet pointer before use.
+    /// </summary>
+    /// <param name="ptr">The pointer to validate.</param>
+    /// <param name="minSize">The minimum expected size of the data.</param>
+    /// <returns>True if the pointer appears valid; false otherwise.</returns>
+    public static bool IsValidPacketPointer(nint ptr, int minSize)
+    {
+        if (ptr == nint.Zero)
+            return false;
+
+        // Ensure pointer is within reasonable memory range
+        if (ptr < MinValidAddress)
+            return false;
+
+        // Ensure pointer is within user-mode address space
+        if (ptr > MaxValidAddress)
+            return false;
+
+        // Minimum size must be positive
+        if (minSize <= 0)
+            return false;
+
+        return true;
+    }
+
+    /// <summary>
+    /// Safely reads a value from a packet pointer with bounds checking.
+    /// </summary>
+    /// <typeparam name="T">The unmanaged type to read.</typeparam>
+    /// <param name="ptr">The base pointer to read from.</param>
+    /// <param name="offset">The byte offset from the base pointer.</param>
+    /// <param name="packetSize">The total size of the packet for bounds checking.</param>
+    /// <returns>The value read from memory.</returns>
+    /// <exception cref="ArgumentOutOfRangeException">
+    /// Thrown when the read would exceed packet boundaries.
+    /// </exception>
+    /// <exception cref="ArgumentException">
+    /// Thrown when the pointer is invalid.
+    /// </exception>
+    public static unsafe T SafeRead<T>(nint ptr, int offset, int packetSize) where T : unmanaged
+    {
+        if (!IsValidPacketPointer(ptr, packetSize))
+            throw new ArgumentException("Invalid packet pointer.", nameof(ptr));
+
+        var size = sizeof(T);
+        if (offset < 0 || offset > packetSize || size > packetSize - offset)
+        {
+            throw new ArgumentOutOfRangeException(
+                nameof(offset),
+                $"Cannot read {size} bytes at offset {offset} from packet of size {packetSize}.");
+        }
+
+        return *(T*)(ptr + offset);
+    }
+
+    /// <summary>
+    /// Attempts to safely read a value from a packet pointer with bounds checking.
+    /// </summary>
+    /// <typeparam name="T">The unmanaged type to read.</typeparam>
+    /// <param name="ptr">The base pointer to read from.</param>
+    /// <param name="offset">The byte offset from the base pointer.</param>
+    /// <param name="packetSize">The total size of the packet for bounds checking.</param>
+    /// <param name="value">The value read from memory, or default if the read failed.</param>
+    /// <returns>True if the read succeeded; false otherwise.</returns>
+    public static unsafe bool TrySafeRead<T>(nint ptr, int offset, int packetSize, out T value) where T : unmanaged
+    {
+        value = default;
+
+        if (!IsValidPacketPointer(ptr, packetSize))
+            return false;
+
+        var size = sizeof(T);
+        if (offset < 0 || offset > packetSize || size > packetSize - offset)
+            return false;
+
+        value = *(T*)(ptr + offset);
+        return true;
+    }
+}

Dalamud/Game/Network/SafePacket.cs

@@ -0,0 +1,189 @@
+using System.Runtime.InteropServices;
+
+namespace Dalamud.Game.Network;
+
+/// <summary>
+/// A safe wrapper around network packet data with lifetime and bounds guarantees.
+/// </summary>
+/// <remarks>
+/// <para>
+/// This class copies packet data to managed memory, ensuring:
+/// </para>
+/// <list type="bullet">
+/// <item><description>Lifetime safety - data persists as long as this object</description></item>
+/// <item><description>Bounds checking - all reads are validated against packet size</description></item>
+/// <item><description>Thread safety - the copied data cannot be modified externally</description></item>
+/// </list>
+/// <para>
+/// This class is intended to replace raw pointer access in future API versions.
+/// </para>
+/// </remarks>
+internal sealed class SafePacket : IDisposable
+{
+    private readonly byte[] data;
+    private bool disposed;
+
+    /// <summary>
+    /// Initializes a new instance of the <see cref="SafePacket"/> class by copying data from an unmanaged pointer.
+    /// </summary>
+    /// <param name="ptr">The source pointer to copy from.</param>
+    /// <param name="size">The number of bytes to copy.</param>
+    /// <exception cref="ArgumentException">Thrown when the pointer is invalid.</exception>
+    /// <exception cref="ArgumentOutOfRangeException">Thrown when size is not positive.</exception>
+    internal SafePacket(nint ptr, int size)
+    {
+        if (!NetworkPointerValidator.IsValidPacketPointer(ptr, size))
+            throw new ArgumentException("Invalid packet pointer.", nameof(ptr));
+
+        if (size <= 0)
+            throw new ArgumentOutOfRangeException(nameof(size), "Size must be positive.");
+
+        this.data = new byte[size];
+        Marshal.Copy(ptr, this.data, 0, size);
+        this.Size = size;
+    }
+
+    /// <summary>
+    /// Initializes a new instance of the <see cref="SafePacket"/> class from existing data.
+    /// </summary>
+    /// <param name="data">The source data to copy.</param>
+    /// <exception cref="ArgumentNullException">Thrown when data is null.</exception>
+    /// <exception cref="ArgumentException">Thrown when data is empty.</exception>
+    internal SafePacket(byte[] data)
+    {
+        ArgumentNullException.ThrowIfNull(data);
+
+        if (data.Length == 0)
+            throw new ArgumentException("Data cannot be empty.", nameof(data));
+
+        this.data = new byte[data.Length];
+        data.CopyTo(this.data, 0);
+        this.Size = data.Length;
+    }
+
+    /// <summary>
+    /// Gets the total size of the packet data in bytes.
+    /// </summary>
+    public int Size { get; }
+
+    /// <summary>
+    /// Gets the packet opcode (first two bytes interpreted as ushort).
+    /// </summary>
+    /// <exception cref="ObjectDisposedException">Thrown when the packet has been disposed.</exception>
+    /// <exception cref="InvalidOperationException">Thrown when packet is too small to contain an opcode.</exception>
+    public ushort OpCode
+    {
+        get
+        {
+            ObjectDisposedException.ThrowIf(this.disposed, this);
+
+            if (this.Size < sizeof(ushort))
+                throw new InvalidOperationException("Packet too small to contain an opcode.");
+
+            return BitConverter.ToUInt16(this.data, 0);
+        }
+    }
+
+    /// <summary>
+    /// Safely reads a value of type T at the specified byte offset.
+    /// </summary>
+    /// <typeparam name="T">The unmanaged type to read.</typeparam>
+    /// <param name="offset">The byte offset from the start of the packet.</param>
+    /// <returns>The value read from the packet data.</returns>
+    /// <exception cref="ObjectDisposedException">Thrown when the packet has been disposed.</exception>
+    /// <exception cref="ArgumentOutOfRangeException">Thrown when the read would exceed packet boundaries.</exception>
+    public unsafe T Read<T>(int offset) where T : unmanaged
+    {
+        ObjectDisposedException.ThrowIf(this.disposed, this);
+
+        var size = sizeof(T);
+        if (offset < 0 || offset > this.Size || size > this.Size - offset)
+        {
+            throw new ArgumentOutOfRangeException(
+                nameof(offset),
+                $"Cannot read {size} bytes at offset {offset} from packet of size {this.Size}.");
+        }
+
+        return MemoryMarshal.Read<T>(this.data.AsSpan(offset));
+    }
+
+    /// <summary>
+    /// Attempts to safely read a value of type T at the specified byte offset.
+    /// </summary>
+    /// <typeparam name="T">The unmanaged type to read.</typeparam>
+    /// <param name="offset">The byte offset from the start of the packet.</param>
+    /// <param name="value">When this method returns, contains the value read, or default if the read failed.</param>
+    /// <returns>True if the read succeeded; false otherwise.</returns>
+    public unsafe bool TryRead<T>(int offset, out T value) where T : unmanaged
+    {
+        value = default;
+
+        if (this.disposed)
+            return false;
+
+        var size = sizeof(T);
+        if (offset < 0 || offset > this.Size || size > this.Size - offset)
+            return false;
+
+        value = MemoryMarshal.Read<T>(this.data.AsSpan(offset));
+        return true;
+    }
+
+    /// <summary>
+    /// Gets a read-only span of the entire packet data.
+    /// </summary>
+    /// <returns>A read-only span covering all packet data.</returns>
+    /// <exception cref="ObjectDisposedException">Thrown when the packet has been disposed.</exception>
+    public ReadOnlySpan<byte> AsSpan()
+    {
+        ObjectDisposedException.ThrowIf(this.disposed, this);
+        return this.data;
+    }
+
+    /// <summary>
+    /// Gets a read-only span of a portion of the packet data.
+    /// </summary>
+    /// <param name="offset">The starting offset.</param>
+    /// <param name="length">The number of bytes to include.</param>
+    /// <returns>A read-only span covering the specified portion of packet data.</returns>
+    /// <exception cref="ObjectDisposedException">Thrown when the packet has been disposed.</exception>
+    /// <exception cref="ArgumentOutOfRangeException">Thrown when the range exceeds packet boundaries.</exception>
+    public ReadOnlySpan<byte> AsSpan(int offset, int length)
+    {
+        ObjectDisposedException.ThrowIf(this.disposed, this);
+
+        if (offset < 0 || length < 0 || offset > this.Size || length > this.Size - offset)
+        {
+            throw new ArgumentOutOfRangeException(
+                nameof(offset),
+                $"Range [{offset}, {offset + length}) exceeds packet size {this.Size}.");
+        }
+
+        return this.data.AsSpan(offset, length);
+    }
+
+    /// <summary>
+    /// Creates a copy of the packet data as a new byte array.
+    /// </summary>
+    /// <returns>A new byte array containing a copy of the packet data.</returns>
+    /// <exception cref="ObjectDisposedException">Thrown when the packet has been disposed.</exception>
+    public byte[] ToArray()
+    {
+        ObjectDisposedException.ThrowIf(this.disposed, this);
+
+        var copy = new byte[this.Size];
+        this.data.CopyTo(copy, 0);
+        return copy;
+    }
+
+    /// <inheritdoc/>
+    public void Dispose()
+    {
+        if (!this.disposed)
+        {
+            // Clear the data array to prevent sensitive network data from persisting in memory
+            Array.Clear(this.data);
+            this.disposed = true;
+        }
+    }
+}

Dalamud/Plugin/Services/IGameNetwork.cs

@@ -5,23 +5,50 @@ namespace Dalamud.Plugin.Services;
 /// <summary>
 /// This class handles interacting with game network events.
 /// </summary>
-[Obsolete("Will be removed in a future release. Use packet handler hooks instead.", true)]
+/// <remarks>
+/// <para>
+/// <b>DEPRECATED:</b> This interface passes raw unmanaged pointers which are unsafe for the following reasons:
+/// </para>
+/// <list type="bullet">
+/// <item><description>No bounds checking on pointer arithmetic - can cause access violations</description></item>
+/// <item><description>No lifetime management - pointer may be freed while plugin holds it</description></item>
+/// <item><description>No size information - callers must guess packet boundaries</description></item>
+/// <item><description>Async usage can cause use-after-free vulnerabilities</description></item>
+/// </list>
+/// <para>
+/// <b>Migration Guide:</b>
+/// </para>
+/// <list type="bullet">
+/// <item><description>For market board data: Use the MarketBoard observable services</description></item>
+/// <item><description>For duty finder: Use DutyFinder observable services</description></item>
+/// <item><description>For custom packets: Create typed hooks using <c>Hook&lt;T&gt;</c> with proper packet structures</description></item>
+/// </list>
+/// <para>
+/// See the Dalamud developer documentation for detailed migration instructions.
+/// </para>
+/// </remarks>
+[Obsolete("Will be removed in a future release. Use packet handler hooks instead. See XML documentation for migration guide.", true)]
 public interface IGameNetwork : IDalamudService
 {
-    // TODO(v9): we shouldn't be passing pointers to the actual data here
-
     /// <summary>
     /// The delegate type of a network message event.
     /// </summary>
-    /// <param name="dataPtr">The pointer to the raw data.</param>
+    /// <param name="dataPtr">
+    /// The pointer to the raw data. WARNING: This pointer has no lifetime guarantees
+    /// and must not be stored or used asynchronously.
+    /// </param>
     /// <param name="opCode">The operation ID code.</param>
     /// <param name="sourceActorId">The source actor ID.</param>
-    /// <param name="targetActorId">The taret actor ID.</param>
+    /// <param name="targetActorId">The target actor ID.</param>
-    /// <param name="direction">The direction of the packed.</param>
+    /// <param name="direction">The direction of the packet.</param>
     public delegate void OnNetworkMessageDelegate(nint dataPtr, ushort opCode, uint sourceActorId, uint targetActorId, NetworkMessageDirection direction);
 
     /// <summary>
     /// Event that is called when a network message is sent/received.
     /// </summary>
+    /// <remarks>
+    /// WARNING: The dataPtr passed to handlers is only valid during the synchronous
+    /// execution of the handler. Do not store the pointer or use it in async contexts.
+    /// </remarks>
     public event OnNetworkMessageDelegate NetworkMessage;
 }

Dalamud/Service/Service{T}.cs

@@ -23,9 +23,6 @@ namespace Dalamud;
 [SuppressMessage("ReSharper", "StaticMemberInGenericType", Justification = "Service container static type")]
 internal static class Service<T> where T : IServiceType
 {
-    // TODO: Service<T> should only work with singleton services. Trying to call Service<T>.Get() on a scoped service should
-    // be a compile-time error.
-
     private static readonly ServiceManager.ServiceAttribute ServiceAttribute;
     private static TaskCompletionSource<T> instanceTcs = new(TaskCreationOptions.RunContinuationsAsynchronously);
     private static List<Type>? dependencyServices;
@@ -39,6 +36,16 @@ internal static class Service<T> where T : IServiceType
             ?? throw new InvalidOperationException(
                 $"{nameof(T)} is missing {nameof(ServiceManager.ServiceAttribute)} annotations.");
 
+        // Prevent Service<T> from being used with scoped services.
+        // Scoped services must be resolved through constructor injection or IServiceScope.
+        if (ServiceAttribute.Kind.HasFlag(ServiceManager.ServiceKind.ScopedService))
+        {
+            throw new InvalidOperationException(
+                $"Service<{type.Name}> cannot be used with scoped services. " +
+                $"Scoped services must be resolved through constructor injection or IServiceScope. " +
+                $"See: https://dalamud.dev/api/services#scoped-services");
+        }
+
         var exposeToPlugins = type.GetCustomAttribute<PluginInterfaceAttribute>() != null;
         if (exposeToPlugins)
             ServiceManager.Log.Debug("Service<{0}>: Static ctor called; will be exposed to plugins", type.Name);