Ensure unsafe code is not used accidentally

Question

A function f() uses eval() (or something as dangerous) with data which I created and stored in local_file on the machine running my program:

import local_file

def f(str_to_eval):
    # code....
    # .... 
    eval(str_to_eval)    
    # ....
    # ....    
    return None


a = f(local_file.some_str)

f() is safe to run since the strings I provide to it are my own.

However, if I ever decide to use it for something unsafe (e.g. user input) things could go terribly wrong. Also, if the local_file stops being local then it would create a vulnerability since I would need to trust the machine that provides that file as well.

How should I ensure that I never "forget" that this function is unsafe to use (unless specific criteria are met)?

_{Note: eval() is dangerous and can usually be replaced by something safe.}

score 25 · Accepted Answer · edited Mar 04 '16 at 03:51

Define a type to decorate “safe” inputs. In your function f(), assert that the argument has this type or throw an error. Be smart enough to never define a shortcut def g(x): return f(SafeInput(x)).

Example:

class SafeInput(object):
  def __init__(self, value):
    self._value = value

  def value(self):
    return self._value

def f(safe_string_to_eval):
  assert type(safe_string_to_eval) is SafeInput, "safe_string_to_eval must have type SafeInput, but was {}".format(type(safe_string_to_eval))
  ...
  eval(safe_string_to_eval.value())
  ...

a = f(SafeInput(local_file.some_str))

While this can be easily circumvented, it makes it harder to do so by accident. People might criticize such a draconian type check, but they would miss the point. Since the SafeInput type is just a datatype and not an object-oriented class that could be subclassed, checking for type identity with type(x) is SafeInput is safer than checking for type compatibility with isinstance(x, SafeInput). Since we do want to enforce a specific type, ignoring the explicit type and just doing implicit duck typing is also not satisfactory here. Python has a type system, so let's use it to catch possible mistakes!

Murphy · Answer 2 · 2016-03-03T20:52:00.373

As Joel once pointed out, make wrong code look wrong (scroll down to section "The Real Solution", or better yet read it completely; it's worth it, even if it addresses variable instead of function names). So I humbly suggest to rename your function to something like f_unsafe_for_external_use() - you'll most probably come up with some abbreviation scheme yourself.

Edit: I think amon's suggestion is a very good, OO based variation on the same theme :-)

score 0 · Answer 3 · answered Mar 04 '16 at 08:22

Complementing the suggestion by @amon, you can also think about combining the strategy pattern here, as well as the method object pattern.

class AbstractFoobarizer(object):
    def handle_cond(self, foo, bar):
        """
        Handle the event or something.
        """
        raise NotImplementedError

    def run(self):
        # do some magic
        pass

And then a 'normal' implementation

class PrintingFoobarizer(AbstractFoobarizer):
    def handle_cond(self, foo, bar):
        print("Something went very well regarding {} and {}".format(foo, bar))

And an admin-input-eval implementation

class AdminControllableFoobarizer(AbstractFoobarizer):
    def handle_cond(self, foo, bar):
        # Look up code in database/config file/...
        code = get_code_from_settings()
        eval(code)

(Note: with a real-world-ish example, I might be able to give a better example).

Ensure unsafe code is not used accidentally

3 Answers3