8

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?

16
  • 29
    Why would you need to let them run code as root if you don't trust them? Most code can be executed as normal user - which limits the security implications...
    – vidarlo
    Mar 15 at 14:41
  • 36
    @user252790 No, just checking for known-vulnerable code patterns is NOT sufficient as a security mechanism. I could show an exploit for your mechanism, you could patch it, and we could repeat the cycle for infinity. Instead, use actual security barriers. Under Linux, the usual technique is to set up a pipe and fork, then drop as many privileges in the child process as possible (and/or use containers), and then execute the less-trusted code. The child process can communicate with the privileged root process via the pipe.
    – amon
    Mar 15 at 14:44
  • 34
    “ I don't see any possibility to escape from this.” Yet there are plenty of ways. Mar 15 at 19:27
  • 66
    Can you elaborate on "We recently had issues with people messing around inside our system."? There's something odd about this question.
    – Owen
    Mar 16 at 6:57
  • 20
    @MJ713 The story is made up. This user is just trying to solve an old CTF puzzle. Owen's comment linked to a write-up about it. Mar 16 at 21:28
86

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.

4

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.

3
  • 6
    However, deciding whether a given Python fragment is “safe” is an undecidable problem (for any interesting definition of safety). The usual syntax-driven approaches could be used to allow a tiny safe subset of the language though. Due to these constraints, methods that audit security-sensitive behaviour at runtime are more popular, e.g. running a process with untrusted code under a restrictive seccomp profile.
    – amon
    Mar 16 at 20:31
  • You could generate additional Python code (into say a string), but the typical way to execute generated code would be using eval or by writing a file and then executing it with OS commands. How would you execute the code without eval or any imports?
    – NotThatGuy
    Mar 17 at 14:18
  • @NotThatGuy vars(__builtins__)['ev'+'al']("print('pwned')") (credit for this comment goes to a modified version of security.stackexchange.com/a/246169/240562 where he used 'exec' instead of 'eval') Mar 17 at 18:56
0

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

2
  • 1
    You can still get the builtins yourstring = "myfunc.__globals__['__builtins__'].print('Escaped')"
    – user236968
    Mar 18 at 22:26
  • OK. so as soon as one object is exported all is lost.
    – KlausF
    Mar 19 at 6:35

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.