Does blocking keywords prevent code injection inside this interactive Python file?

Question

We recently had issues with people messing around inside our system. To prevent code injections within my python code, I implemented the following if block:

#! /usr/bin/python3
#-*- coding:utf-8 -*-
def main():
    print("Your interactive Python shell!")
    text = input('>>> ')
    for keyword in ['eval', 'exec', 'import', 'open', 'os', 'read', 'system', 'write']:
        if keyword in text:
            print("You are not allowed to do this!")
            return;
    else:
        exec(text)
        print('Executed your code!')
if __name__ == "__main__":
    main()

A few (user) people can run this python file with sudo rights inside our Ubuntu system. I know this sounds like a security hole, but I don't see any possibility to escape from this. Is there any possibility to inject inside my code block? Do you have any tips to prevent code injections?

Answer 1

Simple blacklists are about the worst way to patch a vulnerability. They'll create a lot of false positives and any determined attacker will usually find a way around them.

Web Application Firewalls are a good example. The detection rules they employ are way more complicated than your simplistic blacklist, yet they don't catch everything, and every now and then, bypasses come up for things they are supposed to catch.

I don't see any possibility to escape from this.

Or so you think. You just haven't looked long enough.

vars(__builtins__)['ex'+'ec']("print('pwned')")

This sails right through your filter. It calls the exec() function which then goes on to print 'pwned'.

Now you can modify your blacklist to catch this as well, but someone else will come up with another way.

---

99% of the time I see someone using something like exec or eval, it's completely unnecessary. If you can avoid using exec, do yourself a favor and get rid of this vulnerability waiting to be exploited. If, however, you absolutely need to use exec, then do as @amon suggested in the comments: Create a new pipe, fork and drop as many privileges in the child as possible, and then execute the code. Use the pipe to communicate between the child and the parent processes. Or sandbox it as Steffen Ullrich suggested.

Answer 2

The problem I see with your blacklist is that in Python, one could write Python code that generates additional Python code, and then run the additional code. So one could generate the word 'eval' from inside of a program that itself doesn't contain the word. Part of the problem is the dynamism of Python itself, whereas a more strongly typed, static language might be safer.

The Computer Science Theory way of accepting a valid string is by constructing a grammar, and then using a grammar parser.

https://en.wikipedia.org/wiki/Formal_grammar

You'd need to understand how grammar parsers work in order to construct one. However, in this case your grammar parser would be a subset of python's grammar parser itself, so it would be a lot of work, if you had unlimited resources I'd go for it but otherwise don't pick this solution.

Answer 3

I wanted to suggest the approach to set __builtins__ to {} but even this is not safe

def myfunc(a, b):
    return a + b
    

global_namespace = {"__builtins__": {}, "myfunc": myfunc}
exec(yourstring, global_namespace, {})

This disables all builtin keywords, allows only the explicitly passed globals (in this example myfunc) and no locals.

You have to declare explicitly all builtin, global, local functions / objects that you want to allow.

However

in above example one could escape with object.__globals__["__builtins__"].print("I escaped") (Thanks @user236968 for pointing this out)

So the only thing that seems to be safe is to disable all builtins, globals and locals with

global_namespace = {"__builtins__": {}}
exec(yourstring, global_namespace, {})

But perhaps even there could be a loop hole.

As others pointeds out. Probably the only safe way is to not use exec and eval but to implement a parser and then use eval (but your parse must be watertight) or to use you rown parser and own interpreter