Thread Tools Display Modes
11/06/16, 11:03 AM   #1
sirinsidiator
 
sirinsidiator's Avatar
AddOn Author - Click to view addons
Join Date: Apr 2014
Posts: 1,567
attempt to access a private function 'UseItem' from insecure code

I finally took the time to investigate the old "attempt to access a private function 'UseItem' from insecure code" error.
The issue can be reliably reproduced when opening a container via the keybind, closing it and then reopening it again without moving the mouse away from the item while AdvancedFilters is active.
I picked the code apart and disabled it part by part, line by line and finally found that the issue happens when PLAYER_INVENTORY:UpdateList is called from an addon.
More precisly it happens when self:ApplySort is called inside that method. My guess is that ZO_ScrollList_Commit breaks the secure context for the keybind when it autoselects the inventory item entry.

Maybe ZOS could add a new method RequestPlayerInventoryUpdate or something which allows us to call this from a secure context.
Until a better solution is found, I suggest addon authors use the following method instead of directly calling PLAYER_INVENTORY:UpdateList.

Lua Code:
  1. local function SafeUpdateList(object, ...)
  2.     local isMouseVisible = SCENE_MANAGER:IsInUIMode()
  3.  
  4.     --if the mouse is visible, cycle its visibility to refresh the integrity of the control beneath it
  5.     if isMouseVisible then HideMouse() end
  6.  
  7.     object:UpdateList(...)
  8.  
  9.     if isMouseVisible then ShowMouse() end
  10. end
  11.  
  12. -- example usage for different inventories:
  13. SafeUpdateList(PLAYER_INVENTORY, INVENTORY_BACKPACK)
  14. SafeUpdateList(STORE_WINDOW)

Last edited by sirinsidiator : 11/07/16 at 04:29 PM. Reason: replaced code with a better solution found by Randactyl
  Reply With Quote
11/06/16, 11:14 AM   #2
Baertram
Super Moderator
 
Baertram's Avatar
WoWInterface Super Mod
AddOn Author - Click to view addons
Join Date: Mar 2014
Posts: 4,913
Thanks for your time and the workaround.
Will implement this function into my inventory updating addons.

If I understand your workaround function right it won't update the inventory if the mouse is currently over any control of an item in the inventory?
I guess this would break addons like FCOItemSaver as you right click an item to show the context menu, choose an entry and afterwards the inventory will be updated. The mouse is above any inventory item after the context menu closes... And thus the inventory update won't happen anymore.

I'll test this if the control is the context menu, as the update fires, or the inventory line again.
And if you mark an item via keybinding it will definately be the inventory item control..

Last edited by Baertram : 11/06/16 at 11:33 AM.
  Reply With Quote
11/06/16, 01:02 PM   #3
sirinsidiator
 
sirinsidiator's Avatar
AddOn Author - Click to view addons
Join Date: Apr 2014
Posts: 1,567
You are right. It won't help in situations where you do need the mouse over an item.
Right now I do not have a better idea how to solve this, but I will think a bit more about it during the week.
  Reply With Quote
11/06/16, 07:32 PM   #4
merlight
AddOn Author - Click to view addons
Join Date: Jul 2014
Posts: 671
I thought this was generally known already. Remember we've discussed this in so many threads... and then wondered why it's so hard to find more info. Apparently there's no sane way to search addon comments.

Edit: and also no working way to link to a specific comment. You have to switch to the #comments tab by hand and then paste the link to the address bar

Last edited by merlight : 11/06/16 at 07:36 PM.
  Reply With Quote
11/06/16, 08:54 PM   #5
Randactyl
AddOn Author - Click to view addons
Join Date: Apr 2014
Posts: 251
yes, merlight that is the current known workaround.

It also does not break if you simply double click to use the item rather than pressing the primary keybind (default: 'E').

Siri's solution pretty much only fixes the "rapid writ container opening" case and sets up an expected result that libFilters and addons which leverage it or their own inventory updating methods will no longer have issues. At least for libFilters, this now produces unexpected functionality when an addon keybind is used on an inventory slot.

Baertram has been working on a solution involving passing the "initiating control" as an optional argument to the updater and still updating the list if the initiator matches the current mouse over control (thereby negating siri's fix for another specific situation).

I'm still thinking about these proposed changes. Right now I'm somewhere around thinking it's adding unneeded complexity for us to a long standing API issue with two solid and easy user workarounds (moving cursor or double clicking to use).

There is a high possibility that addons implementing libFilters and a keybind can accomplish their tasks without a list update, as Siri suggested this morning. This would be ideal since then the library could implement the safe list update and addons could use better practice for their tasks.

I'm going to really dive into this on Tuesday, as I have a good deal of schoolwork to do tonight and tomorrow. In the meantime, please correct any of my assumptions and further work if it still tickles your fancy
  Reply With Quote
11/07/16, 06:44 AM   #6
sirinsidiator
 
sirinsidiator's Avatar
AddOn Author - Click to view addons
Join Date: Apr 2014
Posts: 1,567
Originally Posted by merlight View Post
I thought this was generally known already. Remember we've discussed this in so many threads... and then wondered why it's so hard to find more info. Apparently there's no sane way to search addon comments.

Edit: and also no working way to link to a specific comment. You have to switch to the #comments tab by hand and then paste the link to the address bar
I have to admit I never read the comments for that addon. The code was terrible enough.
Jokes aside, it looks like Randactyl has found an even better solution:

Lua Code:
  1. PLAYER_INVENTORY:UpdateList(INVENTORY_BACKPACK)
  2. HideMouse()
  3. ShowMouse()

I would suggest to use this in the following way:

Lua Code:
  1. local function SafePlayerInventoryUpdateList(inventoryType)
  2.     PLAYER_INVENTORY:UpdateList(inventoryType)
  3.     if SCENE_MANAGER:IsInUIMode() then
  4.         HideMouse()
  5.         ShowMouse()
  6.     end
  7. end

Btw. you should come and visit our gitter chat sometime. We discuss most problems over there and it would be awesome to have your input from time to time.

Last edited by sirinsidiator : 11/07/16 at 10:45 AM. Reason: replaced non-existing method
  Reply With Quote
11/07/16, 09:04 AM   #7
ZOS_ChipHilseberg
ZOS Staff!
Premium Member
Yes this person is from ZeniMax!
Join Date: Oct 2014
Posts: 551
I'm not immediately clear on how refreshing the list triggers a UseItem. What is that code path?
  Reply With Quote
11/07/16, 10:11 AM   #8
sirinsidiator
 
sirinsidiator's Avatar
AddOn Author - Click to view addons
Join Date: Apr 2014
Posts: 1,567
Originally Posted by ZOS_ChipHilseberg View Post
I'm not immediately clear on how refreshing the list triggers a UseItem. What is that code path?
The refresh itself does not trigger UseItem, but when the user does not move the mouse and then presses the keybind for the primary action the error gets triggered because the secure context of the highlighted item has been violated due to the refresh from an addon.

This happens quite often when I try to open several crafting writ containers one after another. I usually press E to open them and R to take the content and won't move the mouse inbetween.

Last edited by sirinsidiator : 11/07/16 at 10:14 AM.
  Reply With Quote
11/07/16, 11:07 AM   #9
Randactyl
AddOn Author - Click to view addons
Join Date: Apr 2014
Posts: 251
@Chip

The surefire way to reproduce this is to

1. Install Advanced Filters version 1.3.3.0
2. Get a container in your inventory (writ box, PTS template item distribution, etc.)
3. Hover over the container and do not move the mouse at all
4. press 'E' or double-click to open the container inventory
5. Press alt to close the container inventory
6. Press 'E', not double-click, to open the container inventory again
7. Error displays

Between steps 5 and 6 is when an addon call to PLAYER_INVENTORY:UpdateList is triggered with the cursor over an item slot (thereby causing that control to become untrusted somehow)

My fix cycles the mouse's state after the inventory update. This apparently allows the moused over control's integrity to be re-verified.
  Reply With Quote
11/07/16, 03:52 PM   #10
merlight
AddOn Author - Click to view addons
Join Date: Jul 2014
Posts: 671
Disclaimer: I haven't tried running ESO since TG PTS, and am pulling stuff off a very unreliable medium.

Originally Posted by ZOS_ChipHilseberg View Post
I'm not immediately clear on how refreshing the list triggers a UseItem. What is that code path?
AFAIR the culprit was that an inventory refresh triggers OnMouseExit+Enter on the item slot under mouse cursor.
Here: https://github.com/esoui/esoui/blob/...ates.lua#L1209
Keybinds are registered in OnMouseEnter, so if the refresh is invoked from an addon, they become "insecure". I think it should be possible to trigger this even without addons. Try
Lua Code:
  1. /script ZO_ScrollList_Commit(ZO_PlayerInventoryBackpack)
while hovering over a container/consumable and then use it.

Originally Posted by sirinsidiator View Post
I have to admit I never read the comments for that addon. The code was terrible enough.
Jokes aside, it looks like Randactyl has found an even better solution:

Lua Code:
  1. PLAYER_INVENTORY:UpdateList(INVENTORY_BACKPACK)
  2. HideMouse()
  3. ShowMouse()
I'd HideMouse before, UpdateList, ShowMouse after. That'll avoid superfluous OnMouseExit+Enter in UpdateList.


Originally Posted by Randactyl View Post
Between steps 5 and 6 is when an addon call to PLAYER_INVENTORY:UpdateList is triggered with the cursor over an item slot (thereby causing that control to become untrusted somehow)
It's not relevant, but I wonder why UpdateList is called from addon context when a container is opened/closed.
  Reply With Quote
11/07/16, 04:03 PM   #11
sirinsidiator
 
sirinsidiator's Avatar
AddOn Author - Click to view addons
Join Date: Apr 2014
Posts: 1,567
Originally Posted by merlight View Post
I'd HideMouse before, UpdateList, ShowMouse after. That'll avoid superfluous OnMouseExit+Enter in UpdateList.
That might be a nice optimization.

Originally Posted by merlight View Post
It's not relevant, but I wonder why UpdateList is called from addon context when a container is opened/closed.
It's because the filters are updated on scene show, which happens when you close a container.
  Reply With Quote
11/07/16, 04:27 PM   #12
ZOS_ChipHilseberg
ZOS Staff!
Premium Member
Yes this person is from ZeniMax!
Join Date: Oct 2014
Posts: 551
Ah yeah, I see the problem. The closure we're passing in to the menu system is tainted after the addon does the commit. We could fix it by passing data in to the closure call instead of closing over it so we don't regenerate it every time we recreate the actions list. This would require modifying the menu system to accept some params to feed into the callback. I can put it on the list, but it's not a super quick fix.
  Reply With Quote
11/07/16, 04:41 PM   #13
sirinsidiator
 
sirinsidiator's Avatar
AddOn Author - Click to view addons
Join Date: Apr 2014
Posts: 1,567
Sounds good. If you ever get to change the menu system, maybe you could also make it a bit easier for addons to change the behavior of an existing entry.

In the current system this is the simplest way I could come up with to change the primary action of the craft bag item actions for AGS' direct selling feature
Lua Code:
  1. local oAddSlotAction = ZO_InventorySlotActions.AddSlotAction
  2.  
  3. local currentInventorySlot
  4. ZO_PreHook("ZO_InventorySlot_DiscoverSlotActionsFromActionList", function(inventorySlot, slotActions) currentInventorySlot = inventorySlot end)
  5.  
  6. -- prepare AddSlotAction in order to redirect the action
  7. ZO_InventorySlotActions.AddSlotAction = function(slotActions, actionStringId, actionCallback, actionType, visibilityFunction, options)
  8.     if(actionStringId == SI_ITEM_ACTION_REMOVE_ITEMS_FROM_CRAFT_BAG and TRADING_HOUSE:IsAtTradingHouse()) then
  9.         if(self:IsItemAlreadyBeingPosted(currentInventorySlot)) then
  10.             actionStringId = SI_TRADING_HOUSE_REMOVE_PENDING_POST
  11.             actionCallback = function()
  12.                 self:ClearPendingItem()
  13.                 ZO_InventorySlot_OnMouseEnter(currentInventorySlot)
  14.             end
  15.         else
  16.             actionStringId = SI_TRADING_HOUSE_ADD_ITEM_TO_LISTING
  17.             actionCallback = function() TryInitiatingItemPost(currentInventorySlot) end
  18.         end
  19.         actionType = "primary"
  20.     end
  21.     oAddSlotAction(slotActions, actionStringId, actionCallback, actionType, visibilityFunction, options)
  22. end
  Reply With Quote

ESOUI » Developer Discussions » General Authoring Discussion » attempt to access a private function 'UseItem' from insecure code

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