Replace show result with debug evaluate command
authorJeremy Stanley <fungi@yuggoth.org>
Sat, 3 Oct 2020 20:16:02 +0000 (20:16 +0000)
committerJeremy Stanley <fungi@yuggoth.org>
Wed, 7 Oct 2020 14:28:45 +0000 (14:28 +0000)
The show result subcommand was quite unsafe. Even though it limited
the globals for strings passed to the env() builtin, it was still
possible for admins to do things like call the exec() builtin and
then import other modules, or use open() to overwrite files
writeable by the user under which the engine was running.

Introduce a new evaluate command as a substitute and remove the show
result subcommand. Use the debugging framework to limit access to
evaluate so that it's only available if debug mode is enabled in the
configuration at the time the daemon is started. Further limit
evaluate to not have most of the normal builtins, and explicitly
reject any strings containing a double-underscore (__) so that base
attributes of other modules such as __builtins__ can't be called
into easily, or "lambda" so that lambda functions can't be used to
work around protections. Also add some selftests to make sure
evaluate can still use the expressions we previously tested with
show result, and that only administrators can use it, and that it's
only available to them when debug mode is enabled.

The evaluate command is still to be considered quite unsafe, and
debug mode should only be engaged when all administrators with
access to the service are trusted with the same permissions the
system account running the service also possesses.

doc/source/admin.rst
doc/source/coder.rst
mudpy/command.py
mudpy/tests/selftest.py
share/command.yaml

index b25d6ae..3ece3f7 100644 (file)
@@ -60,7 +60,7 @@ troubleshooting
 
 The administrative :command:`show` command provides a number of
 useful inspection tools. Here's an example testing with the
-:command:`show result` subcommand from an active session with a
+:command:`evaluate` debug command from an active session with a
 couple of avatars awake, comparing with the output from related
 :command:`show group` and :command:`show element` invocations::
 
@@ -71,7 +71,7 @@ couple of avatars awake, comparing with the output from related
        actor.avatar_admin_0
        actor.avatar_luser0_0
 
-    > show result actor.universe.groups['actor'].keys()
+    > evaluate actor.universe.groups['actor'].keys()
 
     dict_keys(['avatar_admin_0', 'avatar_luser0_0'])
 
@@ -85,10 +85,17 @@ couple of avatars awake, comparing with the output from related
        location: area.0,0,0
        name: Keyo
 
-    > show result actor.universe.contents['actor.avatar_luser0_0'].get('name')
+    > evaluate actor.universe.contents['actor.avatar_luser0_0'].get('name')
 
     'Keyo'
 
-Note that for safety the :command:`show result` executes within the context of
-a command handler with only Python's :code:`__builtins__`, the :code:`mudpy`
-library package, and the active :code:`universe` available.
+Note that for safety the :command:`evaluate` executes within the context of
+a command handler with limited Python :code:`__builtins__`, the
+:code:`mudpy` library package, and the active :code:`universe` available,
+and also blocks evaluation of any statement containing double-underscores
+(:code:`__`) as well as :code:`lambda` functions. For admins to gain access
+to unsafe debugging commands, the ``.mudpy.limit.debug`` option must be
+enabled in configuration first and the service completely restarted. It
+should still be considered unsafe, since the engine's file handling
+functions could easily alter accessible files or expressions like
+``9**9**9`` could be used to hang the service for indeterminate periods.
index b3ed6da..e91c464 100644 (file)
@@ -208,7 +208,7 @@ how I'd go about implementing it:
    skill, spell, item, or however finding that information would
    best fit into your setting). This is stored in the element's
    :code:`.key` attribute, and you can play around with it on the
-   command line with :command:`show result` like this::
+   command line with :command:`evaluate` like this::
 
     > look
     Center Sample Location
@@ -218,7 +218,7 @@ how I'd go about implementing it:
     A sample prop sits here.
     Utso is here.
 
-    > show result [
+    > evaluate [
           a.key.split('_', 1)[1]
           for a in actor.universe.contents[
               actor.get('location')
index 279da78..5ee7ce8 100644 (file)
@@ -134,6 +134,34 @@ def error(actor, input_data):
     return True
 
 
+def evaluate(actor, parameters):
+    """Evaluate a Python expression."""
+
+    if not parameters:
+        message = "You need to supply a Python expression."
+    elif "__" in parameters:
+        message = "Double-underscores (__) are not allowed in expressions."
+    elif "lambda" in parameters:
+        message = "Lambda functions are not allowed in expressions."
+    else:
+        # Strictly limit the allowed builtins and modules
+        eval_globals = {"__builtins__": dict()}
+        for allowed in ("dir", "globals", "len", "locals"):
+            eval_globals["__builtins__"][allowed] = __builtins__[allowed]
+        eval_globals["mudpy"] = mudpy
+        eval_globals["universe"] = actor.universe
+        try:
+            # there is no other option than to use eval() for this, since
+            # its purpose is to evaluate arbitrary expressions, so do what
+            # we can to secure it and allow it for bandit analysis
+            message = repr(eval(parameters, eval_globals))  # nosec
+        except Exception as e:
+            message = ("$(red)Your expression raised an exception...$(eol)"
+                       "$(eol)$(bld)%s$(nrm)" % e)
+    actor.send(message)
+    return True
+
+
 def halt(actor, parameters):
     """Halt the world."""
     if actor.owner:
@@ -548,20 +576,6 @@ def show(actor, parameters):
                             (facet, str(facets[facet])))
         else:
             message = 'Element "' + arguments[1] + '" does not exist.'
-    elif arguments[0] == "result":
-        if len(arguments) < 2:
-            message = "You need to specify an expression."
-        else:
-            try:
-                # there is no other option than to use eval() for this, since
-                # its purpose is to evaluate arbitrary expressions, so do what
-                # we can to secure it and allow it for bandit analysis
-                message = repr(eval(  # nosec
-                    " ".join(arguments[1:]),
-                    {"mudpy": mudpy, "universe": actor.universe}))
-            except Exception as e:
-                message = ("$(red)Your expression raised an exception...$(eol)"
-                           "$(eol)$(bld)%s$(nrm)" % e)
     elif arguments[0] == "log":
         if len(arguments) == 4:
             if re.match(r"^\d+$", arguments[3]) and int(arguments[3]) >= 0:
index 0bd2dd9..c6f3398 100644 (file)
@@ -340,17 +340,32 @@ test_show_element = (
         r'  \x1b\[32mgender: \x1b\[31mfemale.*> ', ""),
 )
 
-test_show_result = (
-    (2, "> ", "show result 12345*67890"),
-    (2, r"\r\n838102050\r\n.*> ", "show result 1/0"),
-    (2, r"Your expression raised an exception.*division by zero.*> ",
-     "show result mudpy"),
-    (2, r"<module 'mudpy' from.*> ", "show result re"),
-    (2, r"Your expression raised an exception.*name 're' is not defined.*> ",
-     "show result universe"),
-    (2, r"<mudpy\.misc\.Universe object at 0x.*> ", "show result actor"),
-    (2, r"Your expression raised an exception.*name 'actor' is not "
-        r"defined.*> ", ""),
+test_evaluate = (
+    (2, "> ", "evaluate 12345*67890"),
+    (2, r"\r\n838102050\r\n.*> ", "evaluate 1/0"),
+    (2, "Your expression raised an exception.*division by zero.*> ",
+     "evaluate mudpy"),
+    (2, "<module 'mudpy' from.*> ", "evaluate re"),
+    (2, "Your expression raised an exception.*name 're' is not defined.*> ",
+     "evaluate universe"),
+    (2, r"<mudpy\.misc\.Universe object at 0x.*> ", "evaluate actor"),
+    (2, "Your expression raised an exception.*name 'actor' is not defined.*> ",
+        "evaluate dir(mudpy)"),
+    (2, "__builtins__.*> ", "evaluate mudpy.__builtins__.open"),
+    (2, "not allowed.*> ", "evaluate (lambda x: x + 1)(2)"),
+    (2, "not allowed.*> ", ""),
+)
+
+test_debug_restricted = (
+    (0, "> ", "help evaluate"),
+    (0, r"That is not an available command\.", "evaluate"),
+    (0, '(not sure what "evaluate" means|Arglebargle, glop-glyf)', ""),
+)
+
+test_debug_disabled = (
+    (2, "> ", "help evaluate"),
+    (2, r"That is not an available command\.", "evaluate"),
+    (2, '(not sure what "evaluate" means|Arglebargle, glop-glyf)', ""),
 )
 
 test_show_log = (
@@ -426,7 +441,9 @@ dialogue = odict((
     (test_show_groups, "show groups"),
     (test_show_group, "show group"),
     (test_show_element, "show element"),
-    (test_show_result, "show result of a python expression"),
+    (test_evaluate, "show results of python expressions"),
+    (test_debug_restricted, "only admins can run debug commands"),
+    (test_debug_disabled, "debugging commands only in debug mode"),
     (test_show_log, "show log"),
     (test_custom_loglevel, "custom loglevel"),
     (test_invalid_loglevel, "invalid loglevel"),
@@ -435,9 +452,11 @@ dialogue = odict((
 ))
 
 debug_tests = (
+    test_evaluate,
 )
 
 nondebug_tests = (
+    test_debug_disabled,
 )
 
 
index 0f4df40..87da9a3 100644 (file)
@@ -20,6 +20,20 @@ command.create.description: Create a new element in the universe.
 command.create.help: Ways to create an element:$(eol)$(eol)   create
     actor.avatar_fred_1$(eol)   create other.garply foo/bar/baz
 
+command.evaluate.debugging: true
+command.evaluate.description: Evaluate a Python expression.
+command.evaluate.help: For debugging purposes, you can use this to run certain
+    Python language expressions within the running engine's context, though for
+    safety reasons only a limited set of builtins are allowed, as well as
+    objects in the mudpy package namespace and the active universe object.
+    Expressions containing "__" or "lambda" are also prohobited for additional
+    safety. Everything following the word "evaluate" is assumed to be a Python
+    expression, and is passed to the eval() built-in, outputting a string
+    representation of whatever it returns. Any exceptions are caught in an
+    attempt to avoid accidentally crashing the engine. This command is mostly
+    useful for inspecting the contents of in-memory objects, for
+    example:$(eol)$(eol) evaluate universe.groups['actor'].keys()
+
 command.delete.administrative: true
 command.delete.description: Delete an existing facet from an element.
 command.delete.help: You can delete any facet of an element as
@@ -95,6 +109,5 @@ command.show.help: Here are the possible incantations (<parameter> is required,
     data files)$(eol)   show group <group> (list the elements in a
     group)$(eol)   show groups (list all element group names)$(eol)   show log
     [level [start [stop]]] (list logs above level from start to
-    stop)$(eol)   show result <expression> (evaluate a python
-    expression)$(eol)   show time (return several current timer
+    stop)$(eol)   show time (return several current timer
     values)$(eol)   show version (display running version and dependencies)