diff --git a/Dalamud/Game/Network/GameNetwork.cs b/Dalamud/Game/Network/GameNetwork.cs
index b8c91b235..9610bae67 100644
--- a/Dalamud/Game/Network/GameNetwork.cs
+++ b/Dalamud/Game/Network/GameNetwork.cs
@@ -16,6 +16,26 @@ namespace Dalamud.Game.Network;
[ServiceManager.EarlyLoadedService]
internal sealed unsafe class GameNetwork : IInternalDisposableService
{
+ ///
+ /// Offset from the data pointer to the start of the packet header.
+ ///
+ private const int PacketHeaderOffset = 0x10;
+
+ ///
+ /// Offset from the packet header to where the opcode is located.
+ ///
+ private const int OpCodeOffset = 0x12;
+
+ ///
+ /// Offset from the packet header to where the packet data begins.
+ ///
+ private const int PacketDataOffset = 0x20;
+
+ ///
+ /// Size of the packet header for validation and logging purposes.
+ ///
+ private const int PacketHeaderSize = 0x20;
+
private readonly GameNetworkAddressResolver address;
private readonly Hook processZonePacketDownHook;
private readonly Hook 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
- dataPtr -= 0x10;
+ // Go back to the start of the packet header
+ 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(dataPtr, OpCodeOffset, PacketHeaderSize, out var opCode))
+ {
+ Log.Warning("Failed to read packet opcode at offset {Offset}", OpCodeOffset);
+ continue;
+ }
+
d.Invoke(
- dataPtr + 0x20,
- (ushort)Marshal.ReadInt16(dataPtr, 0x12),
+ dataPtr + PacketDataOffset,
+ opCode,
0,
targetId,
NetworkMessageDirection.ZoneDown);
@@ -96,8 +133,8 @@ internal sealed unsafe class GameNetwork : IInternalDisposableService
string header;
try
{
- var data = new byte[32];
- Marshal.Copy(dataPtr, data, 0, 32);
+ var data = new byte[PacketHeaderSize];
+ 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
- // TODO: Implement actor IDs
- this.NetworkMessage?.Invoke(dataPtr + 0x20, (ushort)Marshal.ReadInt16(dataPtr), 0x0, 0x0, NetworkMessageDirection.ZoneUp);
+ // Extract opcode with bounds awareness
+ // Note: Upstream packets have a different structure - opcode is at offset 0, not OpCodeOffset
+ // This is intentional as upstream packet format differs from downstream
+ if (NetworkPointerValidator.TrySafeRead(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];
- Marshal.Copy(dataPtr, data, 0, 32);
+ var data = new byte[PacketHeaderSize];
+ Marshal.Copy(dataPtr, data, 0, PacketHeaderSize);
header = BitConverter.ToString(data);
}
catch (Exception)
diff --git a/Dalamud/Game/Network/NetworkPointerValidator.cs b/Dalamud/Game/Network/NetworkPointerValidator.cs
new file mode 100644
index 000000000..3c9fa7b2f
--- /dev/null
+++ b/Dalamud/Game/Network/NetworkPointerValidator.cs
@@ -0,0 +1,101 @@
+using System.Runtime.InteropServices;
+
+namespace Dalamud.Game.Network;
+
+///
+/// Provides validation utilities for network packet pointers.
+///
+internal static class NetworkPointerValidator
+{
+ ///
+ /// Minimum address threshold below which pointers are considered invalid.
+ /// Addresses below this are typically reserved by the OS.
+ ///
+ private const long MinValidAddress = 0x10000;
+
+ ///
+ /// Maximum valid user-mode address for 64-bit Windows.
+ /// Addresses above this are kernel-mode and inaccessible from user-mode.
+ ///
+ private const long MaxValidAddress = 0x7FFFFFFFFFFF;
+
+ ///
+ /// Validates a network packet pointer before use.
+ ///
+ /// The pointer to validate.
+ /// The minimum expected size of the data.
+ /// True if the pointer appears valid; false otherwise.
+ 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;
+ }
+
+ ///
+ /// Safely reads a value from a packet pointer with bounds checking.
+ ///
+ /// The unmanaged type to read.
+ /// The base pointer to read from.
+ /// The byte offset from the base pointer.
+ /// The total size of the packet for bounds checking.
+ /// The value read from memory.
+ ///
+ /// Thrown when the read would exceed packet boundaries.
+ ///
+ ///
+ /// Thrown when the pointer is invalid.
+ ///
+ public static unsafe T SafeRead(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);
+ }
+
+ ///
+ /// Attempts to safely read a value from a packet pointer with bounds checking.
+ ///
+ /// The unmanaged type to read.
+ /// The base pointer to read from.
+ /// The byte offset from the base pointer.
+ /// The total size of the packet for bounds checking.
+ /// The value read from memory, or default if the read failed.
+ /// True if the read succeeded; false otherwise.
+ public static unsafe bool TrySafeRead(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;
+ }
+}
diff --git a/Dalamud/Game/Network/SafePacket.cs b/Dalamud/Game/Network/SafePacket.cs
new file mode 100644
index 000000000..edb5f7aac
--- /dev/null
+++ b/Dalamud/Game/Network/SafePacket.cs
@@ -0,0 +1,189 @@
+using System.Runtime.InteropServices;
+
+namespace Dalamud.Game.Network;
+
+///
+/// A safe wrapper around network packet data with lifetime and bounds guarantees.
+///
+///
+///
+/// This class copies packet data to managed memory, ensuring:
+///
+///
+/// - Lifetime safety - data persists as long as this object
+/// - Bounds checking - all reads are validated against packet size
+/// - Thread safety - the copied data cannot be modified externally
+///
+///
+/// This class is intended to replace raw pointer access in future API versions.
+///
+///
+internal sealed class SafePacket : IDisposable
+{
+ private readonly byte[] data;
+ private bool disposed;
+
+ ///
+ /// Initializes a new instance of the class by copying data from an unmanaged pointer.
+ ///
+ /// The source pointer to copy from.
+ /// The number of bytes to copy.
+ /// Thrown when the pointer is invalid.
+ /// Thrown when size is not positive.
+ 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;
+ }
+
+ ///
+ /// Initializes a new instance of the class from existing data.
+ ///
+ /// The source data to copy.
+ /// Thrown when data is null.
+ /// Thrown when data is empty.
+ 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;
+ }
+
+ ///
+ /// Gets the total size of the packet data in bytes.
+ ///
+ public int Size { get; }
+
+ ///
+ /// Gets the packet opcode (first two bytes interpreted as ushort).
+ ///
+ /// Thrown when the packet has been disposed.
+ /// Thrown when packet is too small to contain an opcode.
+ 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);
+ }
+ }
+
+ ///
+ /// Safely reads a value of type T at the specified byte offset.
+ ///
+ /// The unmanaged type to read.
+ /// The byte offset from the start of the packet.
+ /// The value read from the packet data.
+ /// Thrown when the packet has been disposed.
+ /// Thrown when the read would exceed packet boundaries.
+ public unsafe T Read(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(this.data.AsSpan(offset));
+ }
+
+ ///
+ /// Attempts to safely read a value of type T at the specified byte offset.
+ ///
+ /// The unmanaged type to read.
+ /// The byte offset from the start of the packet.
+ /// When this method returns, contains the value read, or default if the read failed.
+ /// True if the read succeeded; false otherwise.
+ public unsafe bool TryRead(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(this.data.AsSpan(offset));
+ return true;
+ }
+
+ ///
+ /// Gets a read-only span of the entire packet data.
+ ///
+ /// A read-only span covering all packet data.
+ /// Thrown when the packet has been disposed.
+ public ReadOnlySpan AsSpan()
+ {
+ ObjectDisposedException.ThrowIf(this.disposed, this);
+ return this.data;
+ }
+
+ ///
+ /// Gets a read-only span of a portion of the packet data.
+ ///
+ /// The starting offset.
+ /// The number of bytes to include.
+ /// A read-only span covering the specified portion of packet data.
+ /// Thrown when the packet has been disposed.
+ /// Thrown when the range exceeds packet boundaries.
+ public ReadOnlySpan 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);
+ }
+
+ ///
+ /// Creates a copy of the packet data as a new byte array.
+ ///
+ /// A new byte array containing a copy of the packet data.
+ /// Thrown when the packet has been disposed.
+ public byte[] ToArray()
+ {
+ ObjectDisposedException.ThrowIf(this.disposed, this);
+
+ var copy = new byte[this.Size];
+ this.data.CopyTo(copy, 0);
+ return copy;
+ }
+
+ ///
+ 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;
+ }
+ }
+}
diff --git a/Dalamud/Plugin/Services/IGameNetwork.cs b/Dalamud/Plugin/Services/IGameNetwork.cs
index 4abf20834..cfcf7bc19 100644
--- a/Dalamud/Plugin/Services/IGameNetwork.cs
+++ b/Dalamud/Plugin/Services/IGameNetwork.cs
@@ -5,23 +5,50 @@ namespace Dalamud.Plugin.Services;
///
/// This class handles interacting with game network events.
///
-[Obsolete("Will be removed in a future release. Use packet handler hooks instead.", true)]
+///
+///
+/// DEPRECATED: This interface passes raw unmanaged pointers which are unsafe for the following reasons:
+///
+///
+/// - No bounds checking on pointer arithmetic - can cause access violations
+/// - No lifetime management - pointer may be freed while plugin holds it
+/// - No size information - callers must guess packet boundaries
+/// - Async usage can cause use-after-free vulnerabilities
+///
+///
+/// Migration Guide:
+///
+///
+/// - For market board data: Use the MarketBoard observable services
+/// - For duty finder: Use DutyFinder observable services
+/// - For custom packets: Create typed hooks using Hook<T> with proper packet structures
+///
+///
+/// See the Dalamud developer documentation for detailed migration instructions.
+///
+///
+[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
-
///
/// The delegate type of a network message event.
///
- /// The pointer to the raw data.
+ ///
+ /// The pointer to the raw data. WARNING: This pointer has no lifetime guarantees
+ /// and must not be stored or used asynchronously.
+ ///
/// The operation ID code.
/// The source actor ID.
- /// The taret actor ID.
- /// The direction of the packed.
+ /// The target actor ID.
+ /// The direction of the packet.
public delegate void OnNetworkMessageDelegate(nint dataPtr, ushort opCode, uint sourceActorId, uint targetActorId, NetworkMessageDirection direction);
///
/// Event that is called when a network message is sent/received.
///
+ ///
+ /// 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.
+ ///
public event OnNetworkMessageDelegate NetworkMessage;
}
diff --git a/Dalamud/Service/Service{T}.cs b/Dalamud/Service/Service{T}.cs
index c965eeb3d..66d2e754e 100644
--- a/Dalamud/Service/Service{T}.cs
+++ b/Dalamud/Service/Service{T}.cs
@@ -23,9 +23,6 @@ namespace Dalamud;
[SuppressMessage("ReSharper", "StaticMemberInGenericType", Justification = "Service container static type")]
internal static class Service where T : IServiceType
{
- // TODO: Service should only work with singleton services. Trying to call Service.Get() on a scoped service should
- // be a compile-time error.
-
private static readonly ServiceManager.ServiceAttribute ServiceAttribute;
private static TaskCompletionSource instanceTcs = new(TaskCreationOptions.RunContinuationsAsynchronously);
private static List? dependencyServices;
@@ -39,6 +36,16 @@ internal static class Service where T : IServiceType
?? throw new InvalidOperationException(
$"{nameof(T)} is missing {nameof(ServiceManager.ServiceAttribute)} annotations.");
+ // Prevent Service 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() != null;
if (exposeToPlugins)
ServiceManager.Log.Debug("Service<{0}>: Static ctor called; will be exposed to plugins", type.Name);