View Issue Details

IDProjectCategoryView StatusLast Update
0000395LDMud 3.3Efunspublic2008-09-09 03:30
ReporterSorcerer Assigned To 
PrioritynormalSeverityfeatureReproducibilityN/A
Status acknowledgedResolutionopen 
Product Version3.3 
Summary0000395: Raise privilege violation for set_this_player
DescriptionI 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().
TagsNo tags attached.

Activities

zesstra

2008-07-02 03:17

administrator   ~0000655

Actually, I like this, as it should not be too complicated.

zesstra

2008-07-16 15:47

administrator   ~0000735

Last edited: 2008-07-16 16:17

Does anyone think this is a bad idea? If not, I will have a look at it.

zesstra

2008-09-07 08:04

administrator   ~0000763

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?

Sorcerer

2008-09-08 02:47

updater   ~0000764

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".

zesstra

2008-09-08 06:02

administrator   ~0000765

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.

Gnomi

2008-09-09 02:39

manager   ~0000766

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.

zesstra

2008-09-09 03:18

administrator   ~0000767

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.

Issue History

Date Modified Username Field Change
2005-07-16 04:03 Sorcerer New Issue
2008-07-02 03:17 zesstra Note Added: 0000655
2008-07-02 03:17 zesstra Status new => acknowledged
2008-07-16 15:46 zesstra Status acknowledged => assigned
2008-07-16 15:46 zesstra Assigned To => zesstra
2008-07-16 15:47 zesstra Note Added: 0000735
2008-07-16 16:17 zesstra Note Edited: 0000735
2008-09-07 08:04 zesstra Note Added: 0000763
2008-09-08 02:47 Sorcerer Note Added: 0000764
2008-09-08 06:02 zesstra Note Added: 0000765
2008-09-09 02:39 Gnomi Note Added: 0000766
2008-09-09 03:18 zesstra Note Added: 0000767
2008-09-09 03:30 zesstra Assigned To zesstra =>
2008-09-09 03:30 zesstra Status assigned => acknowledged