ESOUI

ESOUI (https://www.esoui.com/forums/index.php)
-   General Authoring Discussion (https://www.esoui.com/forums/forumdisplay.php?f=174)
-   -   @ZOS: Improve "Access a private function" error message (https://www.esoui.com/forums/showthread.php?t=6926)

votan 03/29/17 01:14 AM

@ZOS: Improve "Access a private function" error message
 
@Chip: Any chance that the error message "attempt to access a private function '...' from insecure code" could include more information why the exception is raised?
The call-stack does not really help most times.

If this could cause some slowdown due to overhead, you may add a switch in the UserSettings.txt. So that one can enable it to track down the problem.

Or does such a switch exists already?

Any other advices?

sirinsidiator 03/29/17 01:43 AM

Yes please! Since homestead I see this issue (along with the lua memory error) all the time and users report it left and right.

Rhyono 03/29/17 07:46 AM

I don't believe any of my addons cause this, but an addon(s) I'm using does, but it's so infrequent that trying to disable addons or track it down isn't viable.

ZOS_ChipHilseberg 03/29/17 07:49 AM

What kind of information are you looking to add to the error?

votan 03/29/17 08:54 AM

Quote:

Originally Posted by ZOS_ChipHilseberg (Post 30361)
What kind of information are you looking to add to the error?

Well, it depends on what can cause the error beside directly calling a private function.
Maybe the control/scene/animation creator (including call-stack or addon name) which marked something insecure.

sirinsidiator 03/29/17 08:58 AM

Quote:

Originally Posted by ZOS_ChipHilseberg (Post 30361)
What kind of information are you looking to add to the error?

I would like to know which line of code is causing the violation. Not sure if this can be done though. Other than that an addon name would be a good start.

ZOS_ChipHilseberg 03/29/17 09:46 AM

It might be possible to note which closure tainted the callstack. But the callstack that created the closure in a tainted state would be long gone at that point.

AssemblerManiac 03/29/17 10:45 AM

How does it detect that the callstack is tainted?

I've seen false positives turn up for Inventory Insight when trying to learn a recipe or motif, and I don't modify anything in that call path. The error points to a detail row IN the IIfA window, but the Use was called from the ESO inventory window (and IIfA doesn't modify the ESO inventory window in any way). Not sure how it happens, but am pretty sure that all of the "phantom" errors are occurring for the same reason - the secure stack is getting corrupted somewhere, and it's not reporting a real problem.

Someone posted an example in the Inventory Insight comments, it's on the last page, 03/11/17, 05:35 PM (most recent copy of the error). How can a drag event on the regular Inventory window point to the IIFA detail row?

votan 03/29/17 11:32 AM

I wonder what kind of operation a high level language Lua addon could do to taint a callstack.

I think any information you could add to the mesage would help.

Thanks Chip.

ZOS_ChipHilseberg 03/29/17 02:44 PM

Since there seems to be some confusion about the security mechanism I thought I'd try to explain it from the start.

The main concept here is the Lua closure. A closure is not just the byte code that is generated from the lua code you write, it also contains references to the environment it is executing in and upvalues which allow it to reference local variables that were in scope when the closure was created. You can look at https://www.lua.org/pil/6.1.html for more information on closures.

Our security system adds another flag to closures which indicates if they are trusted or not. Closures made by the stock UI are trusted, while closures made by add-ons are not trusted. These trusted states are used along with the callstack to determine if a private function is allowed to be run. Any time an untrusted closure is run, the callstack from that point on is tainted and any closures run from that point on are treated as untrusted (even stock UI code). Additionally any closures made from that point on are untrusted The state resets back to trusted when the callstack unwinds.

One example of this is a situation we had with the context menu system. The context menu can show a number of options that the player can click on (same as the right click menu in Windows). These options come from an object pool which dynamically creates the controls as needed. As part of making the control the XML makes mouse enter, exit, down, and up handlers which are closures. The particular situation we encountered was an addon would request to show a large context menu which would require making more options within the pool. Even though the context menu system closures are trusted, the callstack is tainted because it was an addon closure that was invoking the menu system. This means that the mouse handlers are created as untrusted. This was generally fine for the addon because it wasn't trying to call private functions from the menu anyway. However, these controls with the untrusted closures would be placed back into the pool when the menu was hidden. Later, the user would right click an inventory item and the context menu system would pull a control out of the pool that had untrusted mouse handler closures and set it to call the UseItem function. When the user clicked this control, the UseItem would fail because the callstack began with an untrusted mouse handler.

So these are the types of situations that generally lead to addons causing the stock UI to fail in running its functions. Some of these situations can be fixed by changing the addon, while others are easier to adjust on our side. In the case above we pre-populated the context menu with 30 entries during the stock UI load so that the handler closures were created as trusted. These types of bugs can be hard to track down because it's usually an action at some point in the past that setup the error that you are seeing now. There isn't really a great way to make it obvious what to fix in an error message. We know at which part of the callstack it became tainted. However we don't have the callstack that was used to create that untrusted closure which would be the point at which the problem happened. Unfortunately storing a full callstack for each closure is pretty memory intensive. But if you guys have any ideas on what you'd like to see to help diagnose these problems I'm happy to hear them.

Rhyono 03/29/17 05:37 PM

Using the context menu example: I've had it where the keybind (E) was tainted, but the right click > Use was not. Is there any means (on your end) of always creating keybind/mouse handlers as trusted? Is there actually a risk associated with that?

votan 03/29/17 11:10 PM

What about scene fade durations greather or equal of those from built-in scenes. (0 for HUD and 200ms elsewhere)

Why could a scene fading last cause this? And is this something the system could know and warn ahead?

Sounomi 03/30/17 01:06 AM

Calling "SCENE_MANAGER:Show" to show a built-in scene causes that scene to become tainted as well or at least that's what seems to happen. If you call a scene that way that in turn uses the player inventory fragment before its been used by something else, that in turn becomes tainted as well and remains tainted. Is there anything that can be done about this?

Also perhaps what's considered a protected function could be redetermined as well. For instance, "PickupInventoryItem" could possibly be made non-protected as there's things to prevent you from switching gear while in combat anyways and I don't see why else it would need to be protected than that. Since its used for other things like deleting items and moving items between the bank as well, which can get problematic in cases where you want to do it but a party member forces you into combat. "PickupAbility" is another similar function that would be nice to have non-protected too and has a mechanism to prevent you from switching abilities while in combat. Though in this case, the desire is more of how you can switch abilities while "soft combat," i.e. you're in combat but haven't enagaged in it recently. The game can tell the difference but the protected function mechanism can't. Also, both of these get in the way of using custom textures for item and ability icons through the "RedirectTexture" function. Though its not a common thing, it'd still be nice to do it.

sirinsidiator 03/30/17 03:38 AM

I hope I understood this correctly. Every closure that is created by the stock UI has a "trusted" flag set to "true" and every closure created by an addon has it set to "false".
When you execute a function you have a separate flag which starts as true. You then look at the flag for each closure you encounter along the way and set that flag to false on the first addon closure. Once it is false, it is no longer possible to execute private functions until the callstack unwinds.

I am not sure how helpful it would be to know at which step of the callstack the closure became tainted. That wouldn't tell us more than what we already know, or would it? But it might be a good start anyways.

When saving the whole callstack is problematic memorywise, maybe you could at least save the last entry of it? If I am not mistaken, that should be the line of code that we need to look at and would tell us a lot already. Or if that is still too much, maybe save a number which represents the addon index from the addon manager when you set the trusted flag to false? That way we at least know who originally caused the violation and it shouldn't use as much memory.

Or as votan said, make it a flag in the usersettings.txt so we can turn it on when we are debugging. That way it shouldn't matter how much memory it takes.

ZOS_ChipHilseberg 03/30/17 09:34 AM

Quote:

Originally Posted by votan (Post 30377)
What about scene fade durations greather or equal of those from build-in scenes. (0 for HUD and 200ms elsewhere)

Why could a scene fading last cause this? And is this something the system could know and warn ahead?

That likely happens because the last fragment that finishes in a scene will trigger the scene state change. So if the scene is calling private functions on state change (hidden, showing, etc), then that could trigger it.

AssemblerManiac 03/30/17 09:40 AM

Chip:
How would these trusted/untrusted closures explain an error I got this morning?

I was changing out jewelry (I do this every day, before heist & sacrament, and after ), dbl clicked the first two items, and tried to drag the third (a ring). Upon trying to drag the ring, I got an error from Furniture Catalogue window, drag event, trying to call a trusted function. The error turned up while I was dragging ON THE INVENTORY window, not from the Furniture Catalogue item list. I had not used Furniture Catalogue today, have not updated it recently (I do my own updates), and have had it running for weeks, doing the exact same things.

How can we track down phantom events, from places they shouldn't be occurring?

Is it possible that pool'd, re-used, items could have the closure flag set wrong, or have events leftover from a diff object source?

ZOS_ChipHilseberg 03/30/17 11:14 AM

Quote:

Originally Posted by sirinsidiator (Post 30379)
I hope I understood this correctly. Every closure that is created by the stock UI has a "trusted" flag set to "true" and every closure created by an addon has it set to "false".
When you execute a function you have a separate flag which starts as true. You then look at the flag for each closure you encounter along the way and set that flag to false on the first addon closure. Once it is false, it is no longer possible to execute private functions until the callstack unwinds.

I am not sure how helpful it would be to know at which step of the callstack the closure became tainted. That wouldn't tell us more than what we already know, or would it? But it might be a good start anyways.

When saving the whole callstack is problematic memorywise, maybe you could at least save the last entry of it? If I am not mistaken, that should be the line of code that we need to look at and would tell us a lot already. Or if that is still too much, maybe save a number which represents the addon index from the addon manager when you set the trusted flag to false? That way we at least know who originally caused the violation and it shouldn't use as much memory.

Or as votan said, make it a flag in the usersettings.txt so we can turn it on when we are debugging. That way it shouldn't matter how much memory it takes.

The top of the callstack would tell you where the closure was created, but it wouldn't show down to the addon triggering it. In the case of the context menu it would give you the ZO_ObjectPool_CreateControl on line 177 in ZO_ObjectPool.lua and that is it. A setting could be an option. A rough guess on storing all of the stacks for all of the stock UI closures is probably somewhere in the 30 MB range. This would require a couple days of VM modifications, but it isn't impossible.

votan 03/30/17 12:46 PM

Quote:

Originally Posted by ZOS_ChipHilseberg (Post 30382)
The top of the callstack would tell you where the closure was created, but it wouldn't show down to the addon triggering it. In the case of the context menu it would give you the ZO_ObjectPool_CreateControl on line 177 in ZO_ObjectPool.lua and that is it. A setting could be an option. A rough guess on storing all of the stacks for all of the stock UI closures is probably somewhere in the 30 MB range. This would require a couple days of VM modifications, but it isn't impossible.

I would not worry about 30MB.

There must be a moment where a flag switches from true to false, right?
Could this be raised as an error (after setting a switch to do so)?
/edit: If it affects a scene or control?

ZOS_ChipHilseberg 03/30/17 03:47 PM

Quote:

Originally Posted by votan (Post 30383)
I would not worry about 30MB.

There must be a moment where a flag switches from true to false, right?
Could this be raised as an error (after setting a switch to do so)?
/edit: If it affects a scene or control?

The stack is tainted anytime that an untrusted piece of code is run so that would happen every time an addon runs a function. The closures themselves do not change their trusted state once they are created.

Rhyono 03/30/17 04:33 PM

I at least didn't see it mentioned: how did this change with Homestead for it to suddenly become so prevalent?


All times are GMT -6. The time now is 06:03 PM.

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