ESOUI

ESOUI (https://www.esoui.com/forums/index.php)
-   General Authoring Discussion (https://www.esoui.com/forums/forumdisplay.php?f=174)
-   -   [SOLUTION] Insecure Code Errors (https://www.esoui.com/forums/showthread.php?t=6806)

Randactyl 02/09/17 10:50 PM

[SOLUTION] Insecure Code Errors
 
Let's see if we can consolidate all of these.

Preface:
Addon code is considered insecure. ZOS code is considered secure.
Protected functions may be called from insecure code through CallSecureProtected("FunctionName", ...).
Private functions may not be called from insecure code.
Protected and private functions may be called from secure code.

With ESO 2.7 (Homestead) we've seen an explosion of "attempt to access a private function 'FunctionName' from insecure code" errors. The following is a list of currently identified causes.

1. Overriding a ZOS function
Overriding a global ZOS function moves the function into insecure code. If the overridden function makes calls to other functions, the insecure execution context cascades down the entire call chain. This is fine as long as there are no possible ways calling the overriding function could lead to a protected or private function being called somewhere along the chain.

2. Manual Pre and Post Hooking
Largely the same as item 1. Manually hooking involves calling a normally secured function from an insecure context. For prehooks, ZO_PreHook or ZO_PreHookHandler should be used. Unfortunately, there is currently no safe way to posthook secure code.
See:
http://www.esoui.com/forums/showthread.php?t=6152
http://www.esoui.com/forums/showthre...6803#post29737

3. Object Pools
ZOS creates a number of different object pools and preallocates a certain number of objects in each pool. If an addon draws from the pool, the preallocated objects will be returned first. If there are no more preallocated objects, a new one is created and returned until the pool reaches its high water mark.
The preallocated objects, since they were created in secure code, are secure. When an addon draws from a pool and gets a newly allocated object, that object is insecure. If the call chain of the insecure object's event handlers contains a protected or private function, the call will fail with the insecure code error.

If you are absolutely positive you've done all you can to prevent these errors but are still receiving them, there is a black magic workaround. Say your error is with UseItem:
Lua Code:
  1. function UseItem(...)
  2.     CallSecureProtected("UseItem", ...)
  3. end

If it is with another protected function, just replace both instances of UseItem with the name of the appropriate function.

Edit - a note from AssemblerManiac: Overridding UseItem in this way will prevent the use of items from the inventory screen while in combat. This means that items like potions and food cannot be used from the inventory menu. They can still be used from quick slots without any issue.

Phinix 02/09/17 11:06 PM

Excellent!

Now, we just need some sort of "go/no-go" list for commonly hooked functions that are "safe" and ones that are "not safe."

However, as we have seen with Homestead, the chains that bind these functions together can change in the source and result in previously safe hooks becoming unsafe.

As a general rule I suppose you could say never post-hook unless it is ABSOLUTELY necessary.

Scootworks 02/09/17 11:12 PM

Quote:

Originally Posted by Phinix (Post 29753)
Excellent!

Now, we just need some sort of "go/no-go" list for commonly hooked functions that are "safe" and ones that are "not safe."

However, as we have seen with Homestead, the chains that bind these functions together can change in the source and result in previously safe hooks becoming unsafe.

As a general rule I suppose you could say never post-hook unless it is ABSOLUTELY necessary.

did they change? http://wiki.esoui.com/API#Protected_Functions

Randactyl 02/09/17 11:15 PM

Quote:

Originally Posted by Phinix (Post 29753)
Excellent!

Now, we just need some sort of "go/no-go" list for commonly hooked functions that are "safe" and ones that are "not safe."

However, as we have seen with Homestead, the chains that bind these functions together can change in the source and result in previously safe hooks becoming unsafe.

As a general rule I suppose you could say never post-hook unless it is ABSOLUTELY necessary.

In general, hooking or overriding code which sets up objects will be prone to error. It's on the author to research their code paths and determine the outcomes (dropped the ball here myself this patch).

A manual posthook does not have to be regarded as a sort of boogyman, just use care when researching if it will work as expected.

Phinix 02/09/17 11:15 PM

Quote:

Originally Posted by Scootworks (Post 29755)

The problem with Homestead wasn't so much that protected functions changed (though I think some did?). The problem was that in the source ZOS changed some of the functions CALLED by functions and these functions call other functions which are protected, so previously unprotected functions that were hooked and made insecure are now suddenly invoking these new linked functions that make CALLS to protected functions, so multiple steps down the line they are resulting in the errors.

For example hooking into the header buttons for container objects used to be safe because there were no functions using them that made calls to protected functions. That changed in Homestead and BAM! Error city.

Quote:

Originally Posted by Randactyl (Post 29757)
In general, hooking or overriding code which sets up objects will be prone to error. It's on the author to research their code paths and determine the outcomes (dropped the ball here myself this patch).

A manual posthook does not have to be regarded as a sort of boogyman, just use care when researching if it will work as expected.

Fair enough. Teaches us not to get complacent with reading PTS updates. :p

Thanks for your work tracking this down, and for explaining it in a way that makes sense.

Randactyl 02/09/17 11:17 PM

Quote:

Originally Posted by Scootworks (Post 29755)

The list didn't change. Phinix's issue was a manual prehook. My issue was that ZOS made changes to specific object pools that slipped past me.

Phinix 02/09/17 11:35 PM

It may also be helpful to know what specific code Inventory Grid View was using that caused the problem, as while I am loath to admit it, I have not been using this most excellent addon of late, yet have been having the same problems with occasional insecure code errors double-clicking items in the inventory or transferring to the bank.

Since it is totally random and can work for literally hours before failing, tracking down what addon is having the same problem is difficult, though if it helps, here are the ones I am using ATM:

Warning: Spoiler


Slowly working my way through my own projects to make sure they aren't contributing to this, however I have already eliminated the insecure hooks Master Recipe List made into bags for the tracking icon.

Will have to take a look at Show Motifs but I think that is the only other one I contribute to that touches bags.

Randactyl 02/09/17 11:45 PM

Inventory Grid View has its own ScrollList_Update function to take care of a gridded scroll list. Within this, calls to listObject.dataTypes[typeId].pool:AquireObject() occurred more times than there were preallocated, secure objects in the pool.

This resulted in items in grid slots greater than or equal to 13 being insecure controls. From that insecure context, the primary action handler for inventory slots made calls to protected functions UseItem, PickupInventoryItem, PlaceItemInTransfer, etc.

Noobanidus 02/20/17 08:57 PM

AddMenuItem
 
I'm not sure where I got the original code (potentially the concept came from MasterMerchant), but I've noticed the usual "cannot use E to remove items from bank" issue commonly associated with this after creating an addon which adds an option to the context menu.

It uses ZO_PreHook against ZO_InventorySlot_ShowContextMenu with a lambda that then calls my custom function, which uses a zo_callLater to call AddMenuItem and ShowMenu after 50ms.

I can't recall if this is the correct & safe way to do things. Does anyone have any reference for the safest method of doing this?

Alternatively, the increase in insecure code error could be unrelated, but I just want to make sure.

Randactyl 02/20/17 09:13 PM

Quote:

Originally Posted by Noobanidus (Post 29998)
I'm not sure where I got the original code (potentially the concept came from MasterMerchant), but I've noticed the usual "cannot use E to remove items from bank" issue commonly associated with this after creating an addon which adds an option to the context menu.

It uses ZO_PreHook against ZO_InventorySlot_ShowContextMenu with a lambda that then calls my custom function, which uses a zo_callLater to call AddMenuItem and ShowMenu after 50ms.

I can't recall if this is the correct & safe way to do things. Does anyone have any reference for the safest method of doing this?

Alternatively, the increase in insecure code error could be unrelated, but I just want to make sure.

Without seeing code, the only suggestion I have is to use LibCustomMenu rather than using AddMenuItem directly.


All times are GMT -6. The time now is 12:38 PM.

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