Premade Groups Filter

Premade Groups Filter

9M Downloads

ADDON_ACTION_BLOCKED due to protected function GetPlaystyleString() since 9.1.5

matton300 opened this issue · 14 comments

commented

Hi, im getting this error on the addon.

5x [ADDON_ACTION_BLOCKED] AddOn 'PremadeGroupsFilter' tried to call the protected function 'GetPlaystyleString()'.
[string "@!BugGrabber\BugGrabber.lua"]:519: in function <!BugGrabber\BugGrabber.lua:519>
[string "=[C]"]: in function GetPlaystyleString' [string "@FrameXML\LFGList.lua"]:3508: in function <FrameXML\LFGList.lua:3499> [string "=[C]"]: in function LFGListUtil_SetSearchEntryTooltip'
[string "@FrameXML\LFGList.lua"]:2576: in function <FrameXML\LFGList.lua:2573>
[string "=[C]"]: ?

commented

Yes this is a known issue which IMO I cannot fix without Blizzard unprotecting the function GetPlaystyleString(). Problem is not the tooltip modification although it looks like that, but the core functionality of PGF itself which removes groups from the list that do not match the filter. Please ignore it for now.

commented

It's not just GetPlaystyleString(). It can also break auto-creation of groups from the quest list.

Interface\FrameXML\LFGList.lua:256
An action was blocked because of taint from PremadeGroupsFilter - CreateListing()
    Interface\FrameXML\LFGList.lua:1025 LFGListEntryCreation_ListGroupInternal()
    Interface\FrameXML\LFGList.lua:1085 LFGListEntryCreation_CheckAutoCreate()
    Interface\FrameXML\LFGList.lua:656 LFGListEntryCreation_Show()
    Interface\FrameXML\LFGList.lua:2057

The result IDs get tainted so protected functions don't work with them anymore. Haven't found a workaround yet.

commented

Haven't found a workaround neither. Removing groups from the result list taints the list and its contents. It also prevents the playstyle string from being shown at the top of the tooltip.

Sadly GetPlaystyleString() is still protected on PTR as of today (Dec 3rd 2021). Let's hope they fix it.

commented

I found a way to fix this. By checking code of another addon LFM+ and API descriptions, I found that getplaystylestring() and getactivityinfo() are connected with each other. It's true that getactivityinfo() will give you "playstyle" but "playstyle" is protected by getplaystylestring(), so it needs to be used for once even the additional function does nothing

So I copied following lines from LFM+ and added them to the addon's main.lua

function LFMPlus_GetPlaystyleString(playstyle,activityInfo)
  if activityInfo and playstyle ~= (0 or nil) and C_LFGList.GetLfgCategoryInfo(activityInfo.categoryID).showPlaystyleDropdown then
    local typeStr
    if activityInfo.isMythicPlusActivity then
      typeStr = "GROUP_FINDER_PVE_PLAYSTYLE"
    elseif activityInfo.isRatedPvpActivity then
      typeStr = "GROUP_FINDER_PVP_PLAYSTYLE"
    elseif activityInfo.isCurrentRaidActivity then
      typeStr = "GROUP_FINDER_PVE_RAID_PLAYSTYLE"
    elseif activityInfo.isMythicActivity then
      typeStr = "GROUP_FINDER_PVE_MYTHICZERO_PLAYSTYLE"
    end
    return typeStr and _G[typeStr .. tostring(playstyle)] or nil
  else
    return nil
  end
end

C_LFGList.GetPlaystyleString = function(playstyle,activityInfo)
  return LFMPlus_GetPlaystyleString(playstyle, activityInfo)
end

and then it just works perfectly

commented

Interesting solution. This overwrites the global function C_LFGList.GetPlaystyleString with a custom implementation. I wonder if the code is exactly the same as Blizzard's implementation and how they did get or replicate it.

Also it disables LFGListEntryCreation_SetTitleFromActivityInfo to avoid more errors.

I would still prefer a solution on Blizzards end and wonder if they are already aware of the problem.

Also see Stanzilla/WoWUIBugs#195

commented

@rodneycheung
Where exactly did you insert this code? What lines ?

commented

Probably anywhere. It reimplements the original function, so it shouldn't generate errors when called by tainted code. Auto-creation of groups will still be broken sometimes though.

commented

how can i incorporate this into Wolrd Quest Tracker?

commented

While it has not been confirmed, I am of the belief that C_LFGList.GetPlaystyleString is meant to be a protected event as its used by other protected functions, such as when you create a new m+ group and the key level is automatically inserted into the title.

Since Blizzard does not want players automating group listings (especially the title), non-secure code is not able to edit that EditBox. However, in order for Blizzard to enable the ability for listing titles to be automatically filled in from their code the code path that does the auto fill needs to be secure. Thus C_LFGList.GetPlaystyleString being protected.

I did not want to remove functionality from the default UI but the auto-fill feature not working is a small price to pay for being able to enhance pretty much every other aspect of LFG.

commented

Since the dropdown menus can provide a path for determining if something was clicked via a hardware event reliably as well.

The only extra work you should have as a result of using the fix is removing the code that replaces C_LFGList.GetPlaystyleString if blizzard ever removes the protection. The new C_LFGList.GetPlaystyleString uses the same params as the normal C_LFGList.GetPlaystyleString so you wont even need to update any code where you reference C_LFGList.GetPlaystyleString.

As far as Blizzard being notified, Stanzilla's UI Bug Repo is the unofficial official method for making blizzard aware. Many of the issues opened there have been fixed as a direct result of an issue being opened.

commented

I am a bit skeptical about the theory. Blizzard's own code is always considered secure, so it should not matter if Blizzard calls an unprotected or protected function, even if the values returned from this function are used in another protected function later on. Secure code remains secure as long as it is not tainted by an addon. The same applies for hardware protection. Hardware protection means that there must be a hardware event somwhere in the call stack. It does not mean that functions called during the process must be protected as well. So in both cases it should not matter if C_LFGList.GetPlaystyleString is protected for calling C_LFGList.SetEntryTitle (which is also a protected function). A strong argument for my thesis is that C_LFGList.GetActivityInfoTable is also not protected but called in the process.

Anyway, I do not know of any way to contact Blizzard and ask them if they intend to unprotect the function or if they are even aware of the problem. On the PTR it is still protected. So I will add the mentioned code fixes even if I really do not like that solution very much as it means extra work for me on every new patch.

commented

Fix is now rolling out, thank you for your solution, @ChrisKader !

commented

I sadly did not get to finish my statement.

C_LFGList.GetPlaystyleString is used to set values in dropdown menus. Blizzards UIDropDownMenu menu can taint execution the execution path when setting values (https://www.townlong-yak.com/bugs/PfF9rr-UIDropDownMenu) from unsecure functions. These taints can happen even if another addon is using UIDropDownMenu on a completely unrelated Widget. As such, C_LFGList.GetPlaystyleString is protected to prevent execution taints resulting from UIDropDownMenu.

commented

Yes, the default dropdown menus are pretty much unusable for that reason. I got my fingers burned, too, and switched to a custom implementation. Still, would it matter if values from a protected function vs. values from a secure code path are used in the dropdown menu? Both will cause tainting issues.

But you are right, there must be some reason why it is protected and why Blizzard chose to implement this trivial function in C instead of LUA like you did. Either this or the trainee programmed it. ;)

The extra work is to check if they fixed it and to check if there is some new code path that could cause tainting issues since touching both GetPlaystyleString and LFGListEntryCreation_SetTitleFromActivityInfo might cause future problems.

Good to know that they check this repository. I still think there should be some way like a forum to contact the game UI developers as an addon developer for important topics. We are improving their game and revenue for free. But I also know why it does not exist, would probably be flooded with trivial inquiries and begging about gameplay changes. Nevertheless they could start with a good documentation of the API.