Skip to content

Fix null dereference when hooking a vertical box with a generic slot - #428

Open
DegradingAnt wants to merge 1 commit into
satisfactorymodding:masterfrom
DegradingAnt:fix/verticalbox-slot-null-deref
Open

Fix null dereference when hooking a vertical box with a generic slot#428
DegradingAnt wants to merge 1 commit into
satisfactorymodding:masterfrom
DegradingAnt:fix/verticalbox-slot-null-deref

Conversation

@DegradingAnt

Copy link
Copy Markdown

UWidgetBlueprintHookSlot_Generic::SetupPanelSlot dereferences the wrong variable in its vertical box branch:

} else if (UVerticalBoxSlot* VerticalBoxSlot = Cast<UVerticalBoxSlot>(InPanelSlot)) {
    HorizontalBoxSlot->SetSize(Size);
    VerticalBoxSlot->SetPadding(Padding);

HorizontalBoxSlot is declared by the preceding else if. If control reaches the vertical box branch, that cast returned null, so the very first statement calls a member function on a null pointer.

What it does

Any mod that registers a UWidgetBlueprintHookData whose parent widget is a vertical box, with SlotConfiguration set to a UWidgetBlueprintHookSlot_Generic, crashes the game during UGameInstance::Init. It happens before the main menu, so the game never becomes usable.

Unhandled Exception: EXCEPTION_ACCESS_VIOLATION reading address 0x0000000000000038

UHorizontalBoxSlot::SetSize()
UWidgetBlueprintHookSlot_Generic::SetupPanelSlot()
AttachWidgetToWidgetTreeArchetype()
UWidgetBlueprintHookManager::RegisterWidgetBlueprintHook()
UGameInstanceModule::RegisterDefaultContent()
UGameInstanceModule::DispatchLifecycleEvent()
UGameInstanceModuleManager::Initialize()

0x38 is the offset of the member SetSize writes, which is what a null this produces here.

The workaround is to leave SlotConfiguration null, since both call sites already guard on it. That avoids the crash but gives up control of padding and alignment for the hooked widget.

The fix

One word. HorizontalBoxSlot->SetSize(Size) becomes VerticalBoxSlot->SetSize(Size).

UVerticalBoxSlot::SetSize(FSlateChildSize) exists and takes the same argument type as the horizontal box version, so the intent of the line is met rather than worked around. Every other branch in the function calls setters on its own slot, which is why this reads as a copy-paste from the horizontal box branch directly above it.

Testing

Reproduced on 3.12.0 with a mod hooking mGameProgressionVBox in Widget_PlayerHUD. Three consecutive launches crashed at the same address, before the main menu, every time. With the patch applied and SML rebuilt, the same build reached the main menu and loaded a save with no crash at the hook site.

UWidgetBlueprintHookSlot_Generic::SetupPanelSlot dereferences
HorizontalBoxSlot inside the UVerticalBoxSlot branch. That variable
belongs to the preceding else-if, so it is always null by the time the
vertical box branch runs.

Any mod that registers a widget hook whose parent is a vertical box and
whose slot configuration is the generic one will crash the game during
UGameInstance::Init, before the main menu appears. The access violation
reads address 0x38, which is the offset of the member SetSize touches on
a null this.

The other branches in the function each call their own slot's setters, so
this looks like a copy-paste from the horizontal box branch above.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

1 participant