Classic/BCC: CURRENT_BATTLEFIELD_QUEUES tainted by UIFrameFade manager

[Meorawr]

For addon authors and users, while this issue is being investigated I've written LibFrameFade which attempts to resolve UIFrameFade taint without any code changes required. Feedback on its effectiveness would be appreciated.

---

In Classic (1.13.7) and BCC (2.5.2) the UIFrameFade manager is capable of spreading taint to the CURRENT_BATTLEFIELD_QUEUES table managed by BattlefieldFrame_UpdateStatus. This taint spreads to the popup used for entering battlefields, preventing the AcceptBattlefieldPort function from being called.

A test case addon can be downloaded here. The steps to reproduce the issue with this addon are as follows:

1. Log in with the Meorawr_BattlefieldStatusTaintTest addon as the only insecure addon loaded.
2. Queue for a battleground or arena skirmish.
3. Wait until the chat frame has a message printed stating that CURRENT_BATTLEFIELD_QUEUES has tainted itself. This can take about a minute and is demonstrated below; note that the taint is actually immediate, but due to limitations with issecurevariable we can't test until BattlefieldFrame_UpdateStatus is called securely a second time.

2021-09-02_15-16-37.mp4

The minimum problematic snippet (present in the addon) which triggers this issue is as follows:

local frame = CreateFrame("Frame");
local timeToFade = 300;
local startAlpha = 0;
local endAlpha = 0;

UIFrameFadeIn(frame, timeToFade, startAlpha, endAlpha);

The root of the problem is that UIFrameFadeIn calls UIFrameFade, which toward the end of the function iterates over the contents of the (global) FADEFRAMES table to check if the frame being faded is already managed. If not, a new entry for the frame is inserted into the table. This table can contain tainted entries, so if - for example - the first frame in there was insecurely inserted, all frames inserted afterwards would have also been inserted insecurely.

The UIFrameFade system is, unfortunately, used by a lot of addons so it's quite easy to end up with taint being spread through there. In addition, tainting other unrelated globals can also cause otherwise secure code to insert tainted entries - a particularly problematic one is DEFAULT_CHATFRAME_ALPHA which is modified by many chat addons and is read by the FCF_FadeInChatFrame function.

This taint spreads through to the BattlefieldFrame_UpdateStatus function because it also interacts with the UIFrameFade manager. The taint spreads in the following manner:

1. The player queues for a battlefield. This triggers the UPDATE_BATTLEFIELD_STATUS event, which is handled and calls BattlefieldFrame_UpdateStatus in response. Note that tooltipOnly is always nil on this path.
2. The initial status of a newly queued battlefield should always be "queued". The IsAlreadyInQueue function is consulted which itself iterates over the contents of PREVIOUS_BATTLEFIELD_QUEUES and, if the requested map name is not found, leads to a call into UIFrameFadeIn - which may taint execution.
3. Afterwards, the map name is inserted into the CURRENT_BATTLEFIELD_QUEUES table. This insertion may be tainted, which is where the fun begins.

After any entry in CURRENT_BATTLEFIELD_QUEUES is tainted, it more or less becomes a self-sustaining cycle.

4. UPDATE_BATTLEFIELD_STATUS will trigger every so often from now on - usually within a minute of queuing for a battlefield. This triggers another call into BattlefieldFrame_UpdateStatus with tooltipOnly being nil.
5. Near the start of the function the CURRENT_BATTLEFIELD_QUEUES table is iterated over and its contents copied to PREVIOUS_BATTLEFIELD_QUEUES. The table is then replaced. If any entry inside CURRENT_BATTLEFIELD_QUEUES is tainted, the relocation of entries to the other table - and global re-assignment of CURRENT_BATTLEFIELD_QUEUES - are all tainted. In addition, if CURRENT_BATTLEFIELD_QUEUES is tainted then the read of that global at the start of the loop will also taint execution.
6. If the current queue(s) haven't transitioned beyond the "queued" state, the IsAlreadyInQueue check will taint since the contents of PREVIOUS_BATTLEFIELD_QUEUES were tainted at the start of the function, and this then leads to the taint being re-applied to the map names that are inserted into the (cleared) CURRENT_BATTLEFIELD_QUEUES table, leading to a cycle of taint for each call into BattlefieldFrame_UpdateStatus.

If any queue does transition to the "confirm" state, the loop mentioned in step 5 taints execution and causes the assignment of relevant fields on the confirmation dialog to be tainted, which breaks AcceptBattlefieldPort.

[silverselo]

Would it be possible to link where Blizzard acknowledged it? I am kind of frustrated by this bug and it seems like I am the only one. If everyone would have this bug the outcry would be big. Also I can't find it on here https://us.forums.blizzard.com/en/wow/t/burning-crusade-classic-252-known-issues-august-31-2021/1081424. Or would it be possible to write an Addon like in this Master Loot Frame case https://us.forums.blizzard.com/en/wow/t/ui-bug-w-master-looter-and-user-placed-frames/1010308.

[Meorawr]

Would it be possible to link where Blizzard acknowledged it?

Not really - it was reported privately to the UI team, who have said they've forward it onto the relevant people to take a look at the issue.

Or would it be possible to write an Addon like...

I wrote LibFrameFade today which may resolve this specific issue. From limited testing it seems to help, but I need more feedback before I can absolutely say yay or nay.

[Meorawr]

For Classic Era, this is fixed in 1.14.0.40140. The same build indicates the fix has also been merged into the BCC interface files, but I'll keep this open until that makes its way to an actual 2.5.2 build.

@silverselo Additionally regarding the linked issue for the master loot frame, a fix should show up for that in both 1.14.0 and (soon) 2.5.2.

[Meorawr]

Should be fixed as of 2.5.2.40203.