Stanzilla / WoWUIBugs

World of Warcraft UI Bug Tracker
153 stars 7 forks source link

ToggleDropDownMenu() can taint Encounter Journal #532

Closed exochron closed 4 months ago

exochron commented 4 months ago

Hello fellow engineers,

I've noticed for some time now that my addons can get sometimes blocked by the client because of taint. Particularly it's noticeable with the Encounter Journal. So my search lead to the DropDownMenu as (potential) culprit. I know there exist already similar issues with the DropDownMenu and workarounds. (TaintLess ftw. :smile:) But even with these workarounds this error occurs.

It's enough to call ToggleDropDownMenu() anytime before the Encounter Journal gets initially loaded. During its OnLoad C_EncounterJournal.SetTab() gets called, which is a secure function, and therefore errors because of taint. That's quite nasty, because there are potentially a few more OnDemand Blizzard-Addons which initialize a DropDownMenu and call some secure method.

So how did OnLoad get tainted? There are 2 calls of UIDropDownMenu_Initialize(), which look highly suspicious. - Although I don't know the exact reason for the taint of that calls. (That's why I'm posting here for you to maybe figure out. :sweat_smile:)

At least I was able to replicate it quite easily. (Should work on any toon.)

/reload
/run ToggleDropDownMenu(1, nil, CreateFrame("Frame"), "cursor"); LoadAddOn("Blizzard_EncounterJournal")

It's enough to use a very simple menu frame. It doesn't even need to have buttons etc.

There is probably also a way to avoid this error. If you open a blizzard dropdown menu before loading a risky addon, it still works. Like:

1. /reload
2. /run ToggleDropDownMenu(1, nil, CreateFrame("Frame"), "cursor") -- cause some taint
3. right click on ChatFrameTab1 (resets taint)
4. open EJ (all good)
exochron commented 4 months ago

okay I think I found the cause. ToggleDropDownMenu() sets and taints UIDROPDOWNMENU_MENU_LEVEL and UIDROPDOWNMENU_MENU_VALUE. During UIDropDownMenu_Initialize() the given initializer function gets called. And the initializer of EJ checks for UIDROPDOWNMENU_MENU_VALUE. Hence everything on OnLoad gets tainted as well.

Blizz could easily fix this by resetting UIDROPDOWNMENU_MENU_VALUE as well in UIDropDownMenu_InitializeHelper().

Any ideas how to reset that var? :thinking:

Meorawr commented 4 months ago

This is going to be implicitly fixed by other changes in 11.0 anyway.

exochron commented 4 months ago

But that's still so long until Pre-Patch :(

For anybody interested: I was able to whip up some workaround. Kudos to TaintLess for inspiration. :)

hooksecurefunc("ToggleDropDownMenu", function()
    if UIDROPDOWNMENU_MENU_VALUE==nil and not issecurevariable("UIDROPDOWNMENU_MENU_VALUE") then
        UIDROPDOWNMENU_MENU_VALUE=1 -- set a value in order to reset to nil
        TextureLoadingGroupMixin.RemoveTexture({textures = _G}, "UIDROPDOWNMENU_MENU_VALUE")
    end
end)