ESOUI

ESOUI (https://www.esoui.com/forums/index.php)
-   Lua/XML Help (https://www.esoui.com/forums/forumdisplay.php?f=175)
-   -   More 'insecure code' errors, now in my addons! (https://www.esoui.com/forums/showthread.php?t=6803)

Phinix 02/09/17 05:54 PM

More 'insecure code' errors, now in my addons!
 
So ZOS changed something. I can't find any reference to what it was, but it is causing many addons to generate random 'insecure code' LUA errors, with no mention of the addon or any hooks it uses in the traceback, making it something of a challenge to track down.

For example Master Recipe List. It is causing the following errors when changing categories on the Crown Store purchase tab of the Housing Editor:

Code:

EsoUI/Ingame/HousingEditor/FurnitureClasses_Shared.lua:450: attempt to access a private function 'GetMarketProductPricingByPresentation' from insecure code
stack traceback:
        EsoUI/Ingame/HousingEditor/FurnitureClasses_Shared.lua:450: in function 'ZO_HousingMarketProduct:GetMarketProductPricingByPresentation'
        EsoUI/Ingame/HousingEditor/Keyboard/HousingFurnitureProducts_Keyboard.lua:80: in function 'ZO_HousingFurnitureProducts_Keyboard:SetupMarketProductFurnitureRow'
        EsoUI/Ingame/HousingEditor/Keyboard/HousingFurnitureProducts_Keyboard.lua:59: in function 'setupCallback'
        EsoUI/Libraries/ZO_Templates/ScrollTemplates.lua:1462: in function 'ZO_ScrollList_UpdateScroll'
        EsoUI/Libraries/ZO_Templates/ScrollTemplates.lua:1222: in function 'ZO_ScrollList_Commit'
        EsoUI/Libraries/ZO_SortFilterList/ZO_SortFilterList.lua:155: in function 'ZO_SortFilterList:CommitScrollList'
        EsoUI/Ingame/HousingEditor/Keyboard/FurnitureClasses_Keyboard.lua:402: in function 'ZO_HousingFurnitureList:ContentsCommitScrollList'
        EsoUI/Ingame/HousingEditor/Keyboard/FurnitureClasses_Keyboard.lua:320: in function 'CommitScrollList'
        EsoUI/Libraries/ZO_SortFilterList/ZO_SortFilterList.lua:138: in function 'ZO_SortFilterList:RefreshData'
        EsoUI/Ingame/HousingEditor/Keyboard/FurnitureClasses_Keyboard.lua:372: in function 'ZO_HousingFurnitureList:OnCategorySelected'

If you look at EsoUI/Ingame/HousingEditor/FurnitureClasses_Shared.lua:450:

Code:

function ZO_HousingMarketProduct:GetMarketProductPricingByPresentation()
    return GetMarketProductPricingByPresentation(self.marketProductId, self.presentationIndex)
end

Master Recipe List doesn't touch this function, or either of it's values. Further it does not hook any of the functions listed in the traceback.

So, how are we supposed to troubleshoot a thing like this?

Rhyono 02/09/17 05:56 PM

The E issue can also apply to other things (like confirmation windows, not just inventory) and does not trigger an error message, which makes it extra special. They've definitely broken something and it seems to be affecting seemingly unrelated facets of the game.

Phinix 02/09/17 06:05 PM

Nothing to do but start commenting out blocks of code and seeing which is causing it. Starting with all hooks...

Randactyl 02/09/17 06:21 PM

here's the diffs from 2.6.4 to 2.7.5, beginning with the inventory files: https://github.com/esoui/esoui/commi...de21c06da19cf6

I'm looking through them, but more eyes would be helpful.

Phinix 02/09/17 06:26 PM

OK, so I managed to track down this specific case, and (HOPEFULLY) it will help others to eliminate this from their addons as well.

It appears to be a change in the way ZOS is handling hooks to certain global functions. For example in Master Recipe List, I narrowed the above errors down to two specific hooks. By changing these to use ZO_PreHook() I was able to keep the functionality of the hook and avoid these insecure code errors. So:

Code:

        local ESOMRL_ZO_TreeHeader_OnMouseUp = ZO_TreeHeader_OnMouseUp
        ZO_TreeHeader_OnMouseUp = function(self, upInside)
                if GetCraftingInteractionType() == CRAFTING_TYPE_PROVISIONING then
                        local treeNode = self.node.tree
                        if treeNode.exclusive or treeNode.open then
                                treeNode.exclusive = false
                                treeNode.open = false
                        end
                        stationNav = 1
                        stationItem = 1
                        ClearStationSelection()
                end
                ESOMRL_ZO_TreeHeader_OnMouseUp(self, upInside)
        end

Becomes:

Code:

        ZO_PreHook("ZO_TreeHeader_OnMouseUp",
        function(self, upInside)
                if GetCraftingInteractionType() == CRAFTING_TYPE_PROVISIONING then
                        local treeNode = self.node.tree
                        if treeNode.exclusive or treeNode.open then
                                treeNode.exclusive = false
                                treeNode.open = false
                        end
                        stationNav = 1
                        stationItem = 1
                        ClearStationSelection()
                end
        end)

...and:

Code:

        local ESOMRL_ZO_TreeSetNodeOpen = ZO_Tree.SetNodeOpen
        ZO_Tree.SetNodeOpen = function(self, treeNode, open, userRequested)
                if GetCraftingInteractionType() == CRAFTING_TYPE_PROVISIONING then
                        if stationNode == 0 then open = false stationNode = 1 end
                        if stationItem == 0 or stationNav == 0 then open = false end
                end
                ESOMRL_ZO_TreeSetNodeOpen(self, treeNode, open, userRequested)
        end

Becomes:

Code:

        ZO_PreHook(ZO_Tree, "SetNodeOpen",
        function(self, treeNode, open, userRequested)
                if GetCraftingInteractionType() == CRAFTING_TYPE_PROVISIONING then
                        if stationNode == 0 then open = false stationNode = 1 end
                        if stationItem == 0 or stationNav == 0 then open = false end
                end
        end)

YMMV but at least it a clue. ;)

EDIT:

So the obvious question for Master Chip (or anyone more knowledgeable) would be, WHY? :)

If I had to guess it would be the way internal references are cleared out from the active context, and it not happening with custom hooks because of some "waiting for return" type state, and some other, newer function that requires code states be "unburdened" by such waiting states in order to be considered "secure."

/stabdark

Randactyl 02/09/17 06:45 PM

Addon land is all considered insecure.

When you do:
Code:

local func = ZO_Func
ZO_Func becomes insecure. And then any functions called from within ZO_Func (and any called within those) will be considered insecure for that call.

ZO_PreHook has always been the correct way because it doesn't move ZO_Func into addon land.

Phinix 02/09/17 06:56 PM

So what you're saying is, we walk in insecure lands. :p

I wonder if this might be related to the inventory issues...

Randactyl 02/09/17 07:09 PM

That's why I identified the object pool corruption in the other thread.

edit: at least for me

Phinix 02/09/17 07:29 PM

So, what would be the accepted method for modifying function result data AFTER the original function has run?

Copying the entire original function into the prehook then modifying it and returning true seems bad. It would seemingly be mutually exclusive to other addons hooking the same function, no?

Randactyl 02/09/17 07:51 PM

There is no safe post hook.

See: http://www.esoui.com/forums/showthre...light=posthook

Randactyl 02/09/17 08:37 PM

@Diesosoon

That error looks completely unrelated to this discussion.

Please start a thread in addon help with that error and your addon list.

Phinix 02/09/17 09:28 PM

So In fixing this I am going back through my code and replacing hooks that don't need post processing with ZO_PreHook. I am wondering though, would it be "safe" to post-hook tooltips? It seems like everyone does, and it would need some fancy workaround to avoid the added lines going to the top of the tooltip instead of the bottom if using prehooks.

Here is what I am doing now:

Code:

        local ESOMRLSetBagItemTooltip = ItemTooltip.SetBagItem
        ItemTooltip.SetBagItem = function(control, bagId, slotIndex)
                local itemLink = GetItemLink(bagId, slotIndex)
                local itemId = GetItemIdFromLink(itemLink)
                ESOMRLSetBagItemTooltip(control, bagId, slotIndex)
                if ESOMRL.RecipeMaterials[itemId] ~= nil then
                        AddRecipeTooltipLine(control, itemId, itemLink, 1)
                elseif ESOMRL.IngredientMaterials[itemId] ~= nil then
                        AddIngredientTooltipLine(control, itemId)
                elseif ESOMRL.ResultMaterials[itemId] ~= nil then
                        AddRecipeTooltipLine(control, itemId, itemLink, 0)
                end
        end

Any reason this should cause problems?

Diesosoon 02/09/17 10:36 PM

Quote:

Originally Posted by Randactyl (Post 29747)
@Diesosoon

That error looks completely unrelated to this discussion.

Please start a thread in addon help with that error and your addon list.

Apologies.

Phinix 02/10/17 07:50 AM

EDIT: Forgive this last update was premature.

Phinix 02/10/17 11:48 AM

OK, so I have narrowed this inventory drag insecure LUA error down to a combination of TWO specific addons.

Map Coordinates by Garkin
Votan's Mini Map

With no other addons running, I can reliably generate the error with these two loaded together.

votan 02/10/17 11:54 AM

Quote:

Originally Posted by Phinix (Post 29777)
OK, so I have narrowed this inventory drag insecure LUA error down to a combination of TWO specific addons.

Map Coordinates by Garkin
Votan's Mini Map

With no other addons running, I can reliably generate the error with these two loaded together.

I use both, too. What do you do to reproduce?

Ayantir 02/10/17 12:12 PM

I don't see why Map Coordinates (and in a larger scope map addons) would trigger an insecure code. There's nothing in its code touching it. Used it daily for months and 0 issue.

Phinix 02/10/17 12:42 PM

Quote:

Originally Posted by votan (Post 29778)
I use both, too. What do you do to reproduce?

Just enable both together with no other addons loaded. Then open your inventory, click and drag any item (doesn't matter what type, if it is stolen, etc.), and you should get the error.

Here is a video showing the process:
How to duplicate the error

If for whatever reason you can't duplicate this on the first try (as I said the interaction seems random), just enable any other addon, reload, then disable that addon, and reload again, and repeat the above steps with the inventory.

Just in case something is not up to date on my end here are the versions of the two addons I have:
TestAddOns.rar

EDIT:
You may or may not have to open the map first, mouse over it to show some coordinate changes from Map Coordinates,
THEN open the inventory and drag an item. (Not confirmed.)

votan 02/10/17 12:48 PM

Quote:

Originally Posted by Phinix (Post 29780)
Just enable both together with no other addons loaded. Then open your inventory, click and drag any item (doesn't matter what type, if it is stolen, etc.), and you should get the error.

Here is a video showing the process:
How to duplicate the error

If for whatever reason you can't duplicate this on the first try (as I said the interaction seems random), just enable any other addon, reload, then disable that addon, and reload again, and repeat the above steps with the inventory.

Just in case something is not up to date on my end here are the versions of the two addons I have:
TestAddOns.rar

EDIT:
You may or may not have to open the map first, mouse over it to show some coordinate changes from Map Coordinates, THEN open the inventory and drag an item. (Not confirmed.)

Please try this:
Locate this line: WORLD_MAP_FRAGMENT.duration = 100
Set it to zero: WORLD_MAP_FRAGMENT.duration = 0

I think, it is the pin mouse-over animation of the hooked pins (over the scale pins feature request)

Phinix 02/10/17 12:59 PM

Quote:

Originally Posted by votan (Post 29782)
Please try this:
Locate this line: WORLD_MAP_FRAGMENT.duration = 100
Set it to zero: WORLD_MAP_FRAGMENT.duration = 0

Making this change I so far am not able to reproduce the error. If I change it back to 100 and /reloadui the error occurs immediately, same steps.

UPDATE:
I have confirmed that to generate this error you must follow a specific sequence:

1) Reload the game with Map Coordinates and Minimap loaded (and the above setting at 100).
2) Hit your keybind to open the map. The map MUST be the FIRST scene that you activate after reloading.
3) Click the bag icon at the top bar after the map is shown to move to the inventory.
4) Drag an item.


All times are GMT -6. The time now is 05:04 AM.

vBulletin © 2024, Jelsoft Enterprises Ltd
© 2014 - 2022 MMOUI