View Issue Details
|ID||Project||Category||View Status||Date Submitted||Last Update|
|0000395||LDMud 3.3||Efuns||public||2005-07-16 06:03||2008-09-09 05:30|
|Summary||0000395: Raise privilege violation for set_this_player|
|Description||I would like to suggest to have efun::set_this_player() raise a privilege violation. |
Since unrestricted access to this efun allows to easily bypass mudlib security it would be nice to have it "by default forbidden" instead of "by default allowed" as is the case in the current implementation which requires an explicit definition of a nomask simul_efun to disable/restrict the use.
Additionally I consider it a good idea to have the control over potentially dangerous efuns in one place - e.g. in the MASTER->privilege_violation().
|Tags||No tags attached.|
||Actually, I like this, as it should not be too complicated.|
Does anyone think this is a bad idea? If not, I will have a look at it.
I had a look at this issue. While adding the privilege violation would be easy, it has one problem: the move hook in the master usually uses set_this_player() before calling the init()s. The lambda implementing the move hook is bound to the moved object or this_object(), which is most often a non-privileged object. If set_this_player() is disabled/restricted in simul_efun.c the lambda in the master can still use efun::set_this_player() (because the lambda is compiled by the master and not the object it is bound to) and circumvent the restriction.
Checking the privilege violation would then have to include the possibility to check which program (blueprint object) contained the call to set_this_player()...
Additionally, when I think of the potentially large number of calls to set_this_player() during moves into full environments, I don't know if it would be a good idea to add an additional master apply each time... What do you think?
While I really like the idea of having a privilege-violation for set_this_player(), I agree that especially functions concerned with moving objects around should be kept free of any unnecessary overhead.
So if there is no easy way to "label" efuns contained in code compiled by the master object as being privileged, probably it is better to keep this the "old way".
||Mhmm. Don't know if it would be reasonable to check for something like current_lambda.u.lambda.prog_ob.name or current_prog->name in f_set_this_player() and only call privilege_violation() if that is not the master object... Seems to me a bit ugly though, because it is a deviation from the usual privilege checking. Mudlib programmers have to be careful who gets a reference to unbound lambdas compiled in the master, but that is no change to the current situation.|
||Maybe we can design the privilege check similar to the "nomask simul_efun" privilege violations. These are raised by the compiler when a call to an efun is detected, where a nomask simul_efun of the same name exists. For set_this_player this could also be done, even if there is no nomask simul_efun. (Privilege checking at compile time and not runtime.) But nevertheless it would be quite unusual and the difference has to be made clear in the documentation.|
Ok, that would be a better solution, especially in terms of runtime behaviour.
But I am not familiar enough with the LPC compiler for messing around with it right now and would like to leave that to you or Fuchur.
|2005-07-16 06:03||Sorcerer||New Issue|
|2008-07-02 05:17||zesstra||Note Added: 0000655|
|2008-07-02 05:17||zesstra||Status||new => acknowledged|
|2008-07-16 17:46||zesstra||Status||acknowledged => assigned|
|2008-07-16 17:46||zesstra||Assigned To||=> zesstra|
|2008-07-16 17:47||zesstra||Note Added: 0000735|
|2008-07-16 18:17||zesstra||Note Edited: 0000735|
|2008-09-07 10:04||zesstra||Note Added: 0000763|
|2008-09-08 04:47||Sorcerer||Note Added: 0000764|
|2008-09-08 08:02||zesstra||Note Added: 0000765|
|2008-09-09 04:39||Gnomi||Note Added: 0000766|
|2008-09-09 05:18||zesstra||Note Added: 0000767|
|2008-09-09 05:30||zesstra||Assigned To||zesstra =>|
|2008-09-09 05:30||zesstra||Status||assigned => acknowledged|