AddonLifecycle Extra Safety (#1483)

* Disallow attempting to hook base AtkEventListener.ReceiveEvent

* Fix incorrect address resolution

* Add exception guards

* Remove debug logging
This commit is contained in:
MidoriKami 2023-10-14 12:48:21 -07:00 committed by GitHub
parent 954ab4d8d6
commit 0c554fe47d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 81 additions and 13 deletions

View file

@ -26,6 +26,8 @@ internal unsafe class AddonLifecycle : IDisposable, IServiceType
[ServiceManager.ServiceDependency] [ServiceManager.ServiceDependency]
private readonly Framework framework = Service<Framework>.Get(); private readonly Framework framework = Service<Framework>.Get();
private readonly nint disallowedReceiveEventAddress;
private readonly AddonLifecycleAddressResolver address; private readonly AddonLifecycleAddressResolver address;
private readonly CallHook<AddonSetupDelegate> onAddonSetupHook; private readonly CallHook<AddonSetupDelegate> onAddonSetupHook;
private readonly CallHook<AddonSetupDelegate> onAddonSetup2Hook; private readonly CallHook<AddonSetupDelegate> onAddonSetup2Hook;
@ -47,6 +49,9 @@ internal unsafe class AddonLifecycle : IDisposable, IServiceType
this.address = new AddonLifecycleAddressResolver(); this.address = new AddonLifecycleAddressResolver();
this.address.Setup(sigScanner); this.address.Setup(sigScanner);
// We want value of the function pointer at vFunc[2]
this.disallowedReceiveEventAddress = ((nint*)this.address.AtkEventListener)![2];
this.framework.Update += this.OnFrameworkUpdate; this.framework.Update += this.OnFrameworkUpdate;
this.onAddonSetupHook = new CallHook<AddonSetupDelegate>(this.address.AddonSetup, this.OnAddonSetup); this.onAddonSetupHook = new CallHook<AddonSetupDelegate>(this.address.AddonSetup, this.OnAddonSetup);
@ -169,13 +174,18 @@ internal unsafe class AddonLifecycle : IDisposable, IServiceType
try try
{ {
// Hook the addon's ReceiveEvent function here, but only enable the hook if we have an active listener. // Hook the addon's ReceiveEvent function here, but only enable the hook if we have an active listener.
// Disallows hooking the core internal event handler.
var addonName = MemoryHelper.ReadStringNullTerminated((nint)addon->Name); var addonName = MemoryHelper.ReadStringNullTerminated((nint)addon->Name);
var receiveEventHook = Hook<AddonReceiveEventDelegate>.FromAddress((nint)addon->VTable->ReceiveEvent, this.OnReceiveEvent); var receiveEventAddress = (nint)addon->VTable->ReceiveEvent;
this.receiveEventHooks.TryAdd(addonName, receiveEventHook); if (receiveEventAddress != this.disallowedReceiveEventAddress)
if (this.eventListeners.Any(listener => (listener.EventType is AddonEvent.PostReceiveEvent or AddonEvent.PreReceiveEvent) && listener.AddonName == addonName))
{ {
receiveEventHook.Enable(); var receiveEventHook = Hook<AddonReceiveEventDelegate>.FromAddress(receiveEventAddress, this.OnReceiveEvent);
this.receiveEventHooks.TryAdd(addonName, receiveEventHook);
if (this.eventListeners.Any(listener => (listener.EventType is AddonEvent.PostReceiveEvent or AddonEvent.PreReceiveEvent) && listener.AddonName == addonName))
{
receiveEventHook.Enable();
}
} }
} }
catch (Exception e) catch (Exception e)
@ -197,7 +207,14 @@ internal unsafe class AddonLifecycle : IDisposable, IServiceType
Log.Error(e, "Exception in OnAddonSetup pre-setup invoke."); Log.Error(e, "Exception in OnAddonSetup pre-setup invoke.");
} }
addon->OnSetup(valueCount, values); try
{
addon->OnSetup(valueCount, values);
}
catch (Exception e)
{
Log.Error(e, "Caught exception when calling original AddonSetup. This may be a bug in the game or another plugin hooking this method.");
}
try try
{ {
@ -240,7 +257,14 @@ internal unsafe class AddonLifecycle : IDisposable, IServiceType
Log.Error(e, "Exception in OnAddonFinalize pre-finalize invoke."); Log.Error(e, "Exception in OnAddonFinalize pre-finalize invoke.");
} }
this.onAddonFinalizeHook.Original(unitManager, atkUnitBase); try
{
this.onAddonFinalizeHook.Original(unitManager, atkUnitBase);
}
catch (Exception e)
{
Log.Error(e, "Caught exception when calling original AddonFinalize. This may be a bug in the game or another plugin hooking this method.");
}
} }
private void OnAddonDraw(AtkUnitBase* addon) private void OnAddonDraw(AtkUnitBase* addon)
@ -254,7 +278,14 @@ internal unsafe class AddonLifecycle : IDisposable, IServiceType
Log.Error(e, "Exception in OnAddonDraw pre-draw invoke."); Log.Error(e, "Exception in OnAddonDraw pre-draw invoke.");
} }
addon->Draw(); try
{
addon->Draw();
}
catch (Exception e)
{
Log.Error(e, "Caught exception when calling original AddonDraw. This may be a bug in the game or another plugin hooking this method.");
}
try try
{ {
@ -277,7 +308,14 @@ internal unsafe class AddonLifecycle : IDisposable, IServiceType
Log.Error(e, "Exception in OnAddonUpdate pre-update invoke."); Log.Error(e, "Exception in OnAddonUpdate pre-update invoke.");
} }
addon->Update(delta); try
{
addon->Update(delta);
}
catch (Exception e)
{
Log.Error(e, "Caught exception when calling original AddonUpdate. This may be a bug in the game or another plugin hooking this method.");
}
try try
{ {
@ -291,6 +329,8 @@ internal unsafe class AddonLifecycle : IDisposable, IServiceType
private byte OnAddonRefresh(AtkUnitManager* atkUnitManager, AtkUnitBase* addon, uint valueCount, AtkValue* values) private byte OnAddonRefresh(AtkUnitManager* atkUnitManager, AtkUnitBase* addon, uint valueCount, AtkValue* values)
{ {
byte result = 0;
try try
{ {
this.InvokeListeners(AddonEvent.PreRefresh, new AddonRefreshArgs this.InvokeListeners(AddonEvent.PreRefresh, new AddonRefreshArgs
@ -305,7 +345,14 @@ internal unsafe class AddonLifecycle : IDisposable, IServiceType
Log.Error(e, "Exception in OnAddonRefresh pre-refresh invoke."); Log.Error(e, "Exception in OnAddonRefresh pre-refresh invoke.");
} }
var result = this.onAddonRefreshHook.Original(atkUnitManager, addon, valueCount, values); try
{
result = this.onAddonRefreshHook.Original(atkUnitManager, addon, valueCount, values);
}
catch (Exception e)
{
Log.Error(e, "Caught exception when calling original AddonRefresh. This may be a bug in the game or another plugin hooking this method.");
}
try try
{ {
@ -340,7 +387,14 @@ internal unsafe class AddonLifecycle : IDisposable, IServiceType
Log.Error(e, "Exception in OnRequestedUpdate pre-requestedUpdate invoke."); Log.Error(e, "Exception in OnRequestedUpdate pre-requestedUpdate invoke.");
} }
addon->OnUpdate(numberArrayData, stringArrayData); try
{
addon->OnUpdate(numberArrayData, stringArrayData);
}
catch (Exception e)
{
Log.Error(e, "Caught exception when calling original AddonRequestedUpdate. This may be a bug in the game or another plugin hooking this method.");
}
try try
{ {
@ -375,8 +429,15 @@ internal unsafe class AddonLifecycle : IDisposable, IServiceType
Log.Error(e, "Exception in OnReceiveEvent pre-receiveEvent invoke."); Log.Error(e, "Exception in OnReceiveEvent pre-receiveEvent invoke.");
} }
var addonName = MemoryHelper.ReadStringNullTerminated((nint)addon->Name); try
this.receiveEventHooks[addonName].Original(addon, eventType, eventParam, atkEvent, data); {
var addonName = MemoryHelper.ReadStringNullTerminated((nint)addon->Name);
this.receiveEventHooks[addonName].Original(addon, eventType, eventParam, atkEvent, data);
}
catch (Exception e)
{
Log.Error(e, "Caught exception when calling original AddonReceiveEvent. This may be a bug in the game or another plugin hooking this method.");
}
try try
{ {

View file

@ -44,6 +44,12 @@ internal class AddonLifecycleAddressResolver : BaseAddressResolver
/// </summary> /// </summary>
public nint AddonOnRefresh { get; private set; } public nint AddonOnRefresh { get; private set; }
/// <summary>
/// Gets the address of AtkEventListener base vTable.
/// This is used to ensure that we do not hook ReceiveEvents that resolve back to the internal handler.
/// </summary>
public nint AtkEventListener { get; private set; }
/// <summary> /// <summary>
/// Scan for and setup any configured address pointers. /// Scan for and setup any configured address pointers.
/// </summary> /// </summary>
@ -57,5 +63,6 @@ internal class AddonLifecycleAddressResolver : BaseAddressResolver
this.AddonUpdate = sig.ScanText("FF 90 ?? ?? ?? ?? 40 88 AF"); this.AddonUpdate = sig.ScanText("FF 90 ?? ?? ?? ?? 40 88 AF");
this.AddonOnRequestedUpdate = sig.ScanText("FF 90 98 01 00 00 48 8B 5C 24 30 48 83 C4 20"); this.AddonOnRequestedUpdate = sig.ScanText("FF 90 98 01 00 00 48 8B 5C 24 30 48 83 C4 20");
this.AddonOnRefresh = sig.ScanText("48 89 5C 24 08 57 48 83 EC 20 41 8B F8 48 8B DA"); this.AddonOnRefresh = sig.ScanText("48 89 5C 24 08 57 48 83 EC 20 41 8B F8 48 8B DA");
this.AtkEventListener = sig.GetStaticAddressFromSig("4C 8D 3D ?? ?? ?? ?? 49 8D 8E");
} }
} }