Nicolas Weeger wrote:
Hello.
Currently, a plugin can easily crash the server, which doesn't check
parameters (just call a function with a NULL pointer, nice crash
guaranteed). Also, server doesn't checks parameters and such, which can
lead to invalid values (Str of 50 for a player...).
So should that be fixed in a way a plugin can *not* crash the server
through a callback, or send invalid parameters?
Note that preventing a plugin ever crashing the server is hard, since
the plugin itself can crash leading to server crash (and that isn't
something easy to avoid i think).
IMO no is an acceptable answer. We can after all trust a plugin to
do the right thing.
If we choose the yes, let's harden the plugin system option, here's my
suggestion for hardening:
* when plugin requests an object/map/archetype, server keeps the pointer
in an array and sends the index, which is used in subsequent functions
* this way, it's easy to check the pointer's validity - check index, if
inbounds ok, else issue
* when the plugin returns, server knows what objects were affected
(array can contain a set_parameter field, set when a setter is
called), and can send updates to player accordingly - should also
simplify plugin's code. Also can recalculate stuff when object is
removed, whatever.
Presumably for that last point to work, all the functions the change values in
the plugin code would need to set some flag. Otherwise, I don't see how the
server can know an object changes.
For example, plugin is called with an object whose object pointer is deadbeef.
The plugin makes some changes (say changes name) and returns. The object
pointer is still deadbeef.
Unless the server keeps a copy of the object, and then runs a comparison, I
don't see how the server can really know the object has changed. And keeping a
copy, running a comparison, and updating it would seem to be a fairly costly
operation (maybe not a big deal, but I could potentially see that as preventing
really wide spread use of the plugin if the performance is bad).
I personally do think that the plugin should do basic checking - making sure
set values are valid, etc. I don't think that stuff is too hard. And I think
that may catch errors object comparison can't do.
For example, if a field in the object type is an sint16, the valid value is
-32768 to 32767. If something tries to set the valid beyond that, it is bad.
However, object comparison won't catch that - some plugin sets the value to
123,456. The value gets truncated, so is a legal value, but probably no what
the plugin wants, and thus causes problems. OTOH, if the actual plugin code
checks the valid for the -32768 to 32767 range, it can log an error right there.
Another thing (unrelated to this) is plugin verification of compatibility.
More than once on my own working copy, I've done a make but not done a make
install, only to track it down to the plugin being out of date.
What I think could be done is that the plugin code has code someplace like:
#define PY_OB_SIZE sizeof(object)
#define PY_PL_SIZE sizeof(player)
etc for major structures.
The server could have similar code, but different name. In the server init
code, it could initialize some global variables to those values.
The plugin could then compare the size it thinks the structures should be with
what the server thinks they are, and if any difference, logs an error and either
exits (because it is sure to crash otherwise), or disables itself.
This isn't foolproof - if fields are moved in the structure but size remains
the same, that won't catch the difference (if really paranoid, could add some
offset checks). However, I think the case of fields moving about is low - the
more common case is new fields being added or old fields being removed, and
sizeof checks should catch that.
___
crossfire mailing list
crossfire@metalforge.org
http://mailman.metalforge.org/mailman/listinfo/crossfire
