Thread Tools Display Modes
02/09/17, 05:54 PM   #1
Phinix
 
Phinix's Avatar
AddOn Author - Click to view addons
Join Date: Apr 2014
Posts: 186
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?

Last edited by Phinix : 02/09/17 at 06:32 PM.
  Reply With Quote
02/09/17, 05:56 PM   #2
Rhyono
AddOn Author - Click to view addons
Join Date: Sep 2016
Posts: 659
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.

Last edited by Rhyono : 02/09/17 at 06:35 PM.
  Reply With Quote
02/09/17, 06:05 PM   #3
Phinix
 
Phinix's Avatar
AddOn Author - Click to view addons
Join Date: Apr 2014
Posts: 186
Nothing to do but start commenting out blocks of code and seeing which is causing it. Starting with all hooks...
  Reply With Quote
02/09/17, 06:21 PM   #4
Randactyl
AddOn Author - Click to view addons
Join Date: Apr 2014
Posts: 251
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.
  Reply With Quote
02/09/17, 06:26 PM   #5
Phinix
 
Phinix's Avatar
AddOn Author - Click to view addons
Join Date: Apr 2014
Posts: 186
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

Last edited by Phinix : 02/09/17 at 06:36 PM.
  Reply With Quote
02/09/17, 06:45 PM   #6
Randactyl
AddOn Author - Click to view addons
Join Date: Apr 2014
Posts: 251
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.
  Reply With Quote
02/09/17, 06:56 PM   #7
Phinix
 
Phinix's Avatar
AddOn Author - Click to view addons
Join Date: Apr 2014
Posts: 186
So what you're saying is, we walk in insecure lands.

I wonder if this might be related to the inventory issues...
  Reply With Quote
02/09/17, 07:09 PM   #8
Randactyl
AddOn Author - Click to view addons
Join Date: Apr 2014
Posts: 251
That's why I identified the object pool corruption in the other thread.

edit: at least for me
  Reply With Quote
02/09/17, 07:29 PM   #9
Phinix
 
Phinix's Avatar
AddOn Author - Click to view addons
Join Date: Apr 2014
Posts: 186
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?
  Reply With Quote
02/09/17, 07:51 PM   #10
Randactyl
AddOn Author - Click to view addons
Join Date: Apr 2014
Posts: 251
There is no safe post hook.

See: http://www.esoui.com/forums/showthre...light=posthook
  Reply With Quote
02/09/17, 08:37 PM   #11
Randactyl
AddOn Author - Click to view addons
Join Date: Apr 2014
Posts: 251
@Diesosoon

That error looks completely unrelated to this discussion.

Please start a thread in addon help with that error and your addon list.
  Reply With Quote
02/09/17, 09:28 PM   #12
Phinix
 
Phinix's Avatar
AddOn Author - Click to view addons
Join Date: Apr 2014
Posts: 186
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?
  Reply With Quote
02/09/17, 10:36 PM   #13
Diesosoon
 
Diesosoon's Avatar
Join Date: Oct 2016
Posts: 15
Originally Posted by Randactyl View Post
@Diesosoon

That error looks completely unrelated to this discussion.

Please start a thread in addon help with that error and your addon list.
Apologies.
  Reply With Quote
02/10/17, 07:50 AM   #14
Phinix
 
Phinix's Avatar
AddOn Author - Click to view addons
Join Date: Apr 2014
Posts: 186
EDIT: Forgive this last update was premature.

Last edited by Phinix : 02/10/17 at 11:48 AM.
  Reply With Quote
02/10/17, 11:48 AM   #15
Phinix
 
Phinix's Avatar
AddOn Author - Click to view addons
Join Date: Apr 2014
Posts: 186
Lightbulb

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.
  Reply With Quote
02/10/17, 11:54 AM   #16
votan
 
votan's Avatar
AddOn Author - Click to view addons
Join Date: Oct 2014
Posts: 577
Originally Posted by Phinix View Post
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?
  Reply With Quote
02/10/17, 12:12 PM   #17
Ayantir
 
Ayantir's Avatar
AddOn Author - Click to view addons
Join Date: Jul 2014
Posts: 1,019
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.
  Reply With Quote
02/10/17, 12:42 PM   #18
Phinix
 
Phinix's Avatar
AddOn Author - Click to view addons
Join Date: Apr 2014
Posts: 186
Originally Posted by votan View Post
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.)

Last edited by Phinix : 02/10/17 at 12:48 PM.
  Reply With Quote
02/10/17, 12:48 PM   #19
votan
 
votan's Avatar
AddOn Author - Click to view addons
Join Date: Oct 2014
Posts: 577
Originally Posted by Phinix View Post
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)

Last edited by votan : 02/10/17 at 12:53 PM.
  Reply With Quote
02/10/17, 12:59 PM   #20
Phinix
 
Phinix's Avatar
AddOn Author - Click to view addons
Join Date: Apr 2014
Posts: 186
Originally Posted by votan View Post
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.
  Reply With Quote

ESOUI » Developer Discussions » Lua/XML Help » More 'insecure code' errors, now in my addons!

Thread Tools
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

vB code is On
Smilies are On
[IMG] code is On
HTML code is Off