Seite 1 von 1

lambda expressions als Strings, eval und die Sicherheit

Verfasst: Freitag 18. September 2009, 15:46
von Hyperion
Hallo,

ich möchte in einer Konfigurationsdatei dem User die Möglichkeit geben, simple Tests zu definieren, anhand derer dann später ein Datensatz aus einer gegebenen Datenbasis extrahiert wird. Beides liegt im JSON-Format vor.

Ein Test könnte beispielhaft so aussehen:

Code: Alles auswählen

"name": "titles > 350 or titles < 200"
Wobei "name" nur ein Bezeichner für den Test ist. In diesem Falle sollen nur die Daten übernommen werden, bei denen die Eigenschaft titles den Test besteht. Es sollen also nur simple Vergleiche auf Zahlenbasis (evtl. auch lexikographisch) und einfache Logik a la "and, or und not" möglich sein. Keine Bedingungen oder andere Kontrollstrukturen.

Ich würde da jetzt folgendermaßen vorgehen, indem ich einfach lambda-Ausdrücke nutze:

Code: Alles auswählen

In [33]: tests = {"title": "titles > 350 or titles < 200"}

In [34]: test = eval("lambda titles:" + tests["title"])

In [35]: test(400)
Out[35]: True

In [36]: test(300)
Out[36]: False
Ok, ich muss noch die Attributnamen im dict speichern, die im lambda-Ausdruck gebraucht werden, aber das sind ja Kleinigkeiten.

So weit so gut, aber was ich mich nun frage ist vor allem, wie "sicher" wäre so eine Lösung? Zum einen in bezug auf bösartigen Code, zum anderen bezüglich auf Fehler? (reicht z.B. das Abfangen von SyntaxError beim eval?)

Oder wäre ein anderer Ansatz sinnvoller? Man könnte ja auch für derart einfache Tests natürlich eine simple Logik-Sprache entwickeln - wie ich sma kenne, wäre das etwas für ihn ;-)

Verfasst: Freitag 18. September 2009, 17:51
von BlackJack
@Hyperion: Wenn Du das mit `eval()` machen willst, dann solltest Du auf jeden Fall die beiden optionalen Argumente kontrollieren und bei den `globals` explizit dafür sorgen, dass `__builtin__` mit dem Wert `None` belegt ist. Sonst kann ein Angreifer zum Beispiel `__import__()` verwenden.

Verfasst: Freitag 18. September 2009, 18:02
von Hyperion
BlackJack hat geschrieben:@Hyperion: Wenn Du das mit `eval()` machen willst, dann solltest Du auf jeden Fall die beiden optionalen Argumente kontrollieren und bei den `globals` explizit dafür sorgen, dass `__builtin__` mit dem Wert `None` belegt ist. Sonst kann ein Angreifer zum Beispiel `__import__()` verwenden.
Danke für den Hinweis. Das werd ich mir auf jeden Fall mal angucken.

Hättest Du denn evtl. eine Alternative anzubieten zu eval + lambda?

Verfasst: Samstag 19. September 2009, 02:14
von Trundle
Nein, es muss nicht `__builtin__` mit dem Wert `None` belegt sein sondern `__builtins__`. Wenn überhaupt. Bringt nämlich absolut gar nichts. Folgender Code (Python 2.6):

Code: Alles auswählen

ns = dict(__builtins__=None)

print eval("""(lambda d={}, t=(1).__class__.__class__:
                (t(lambda: 23)(
                    t((_ for _ in []).gi_code)(
                        0, 1, 4, 67,"""
                        # SETUP_EXCEPT
                        r"""'y\x0c\x00'"""
                        # LOAD_CONST 1
                        r"""'d\x01\x00'"""
                        # LOAD_CONST 0
                        r"""'d\x02\x00'"""
                        # BINARY_DIVIDE
                        r"""'\x15'"""
                        # POP_TOP
                        r"""'\x01'"""
                        # POP_BLOCK
                        r"""'W'"""
                        # JUMP FORWARD 9
                        r"""'n\x09\x00'"""
                        # POP_TOP
                        r"""'\x01'"""
                        # POP_TOP
                        r"""'\x01'"""
                        # The traceback object is now on TOS
                        # STORE_GLOBAL tb
                        r"""'a\x00\x00'"""
                        # JUMP_FORWARD 1
                        r"""'n\x01\x00'"""
                        # END_FINALLY
                        r"""'X'"""
                        # LOAD_CONST None
                        r"""'d\x00\x00'"""
                        # RETURN_VALUE
                        """'S',
                        (None, 1, 0), ('tb', ), (),
                        'evil.py', 'evil', 1, ''
                    ), d, None, ()
                )(),d['tb'].tb_frame.f_back.f_back.f_back.f_globals)[1])()""",
                ns, ns)
Et voilà, da ist das richtige `__builtins__` wieder.

Wem das zuviel ist, der kann sich ja

Code: Alles auswählen

''.__class__.__bases__[0].__bases__[0].__subclasses__()[20]('/etc/passwd', 'r').read()
anschauen, was birkenfeld in einem anderen Thread einmal gepostet hat.

Fazit: `eval()` ist nicht dafür gedacht, um sicher zu sein, einen eigenen Parser zu bauen geht einfach und vor allem auch viel schmerzfreier.

Verfasst: Samstag 19. September 2009, 09:29
von snafu
@Trundle: Die 20 ist in meinem Python <class 'warnings.WarningMessage'>. Falls auch for-Schleifen ohne `__builtins__` gemacht werden können, ist vielleicht Folgendes unabhängiger vom Interpreter:

Code: Alles auswählen

for f in ''.__class__.__bases__[0].__bases__[0].__subclasses__():
    if f.__name__ == 'file':
        f('/etc/passwd', 'r').read()

Verfasst: Samstag 19. September 2009, 09:40
von BlackJack
@snafu: Für `eval()` müsste man die Schleife als "list comprehension" oder so ähnlich ausdrücken, weil man ja nur Ausdrücke und keine Anweisungen verwenden kann.

Verfasst: Samstag 19. September 2009, 09:55
von snafu

Code: Alles auswählen

In [28]: eval("[f('test.py', 'r').read() for f in ''.__class__.__bases__[0].__bases__[0].__subclasses__() if f.__name__ == 'file'][0]", {'__builtins__': None})
---------------------------------------------------------------------------
IOError                                   Traceback (most recent call last)

/home/sebastian/<ipython console> in <module>()

/home/sebastian/<string> in <module>()

IOError: file() constructor not accessible in restricted mode

Verfasst: Samstag 19. September 2009, 11:24
von sma
Andere sagten ja schon, dass es keine sichere Sandbox für Pythons `eval` gibt. Muss man selbst machen. Ist aber eigentlich nicht so schwer. Hier ist ein Beispiel, welches das ast-Modul von Python 2.6 nutzt:

Code: Alles auswählen

import ast

expr = ast.parse("3+a", mode="eval")
print ast.dump(expr)

class Methods:
    def Expression_eval(expr, c):
        return expr.body.eval(c)
    
    def BinOp_eval(op, c):
        left = op.left.eval(c)
        right = op.right.eval(c)
        return op.op(left, right)
        
    def Num_eval(num, c):
        return num.n
    
    def Name_eval(name, c):
        return c[name.id]
    
    def Add___call__(add, l, r):
        return l + r
    
    @classmethod
    def install(cls):
        for name, method in cls.__dict__.items():
            if name[0].isupper():
                c, n = name.split("_", 1)
                setattr(getattr(ast, c), n, method)

Methods.install()

print expr.eval({"a": 4})
Stefan

Verfasst: Samstag 19. September 2009, 12:20
von Hyperion
Vielen Dank für das Snippet. Ich werde es mir mal angucken; ist für mich doch viel unbekanntes drin. Irgend wie hat dieses Problem von mir wieder mal das Interesse an Parsern bei mir geweckt :-)

Ich hab jetzt zunächst einmal eine eval()-Lösung bei mir implementiert, bin aber sehr motiviert, diese durch eine Parsing-Lösung zu ersetzen :-)

Verfasst: Sonntag 20. September 2009, 10:35
von sma
Ich mache da eigentlich nichts Besonderes. Ich nutze `ast.parse()` um mir einen Baum von AST-Knoten erzeugen zu lassen, den ich mir danach mal ausgeben lassen, um zu lernen was für Klassen und Attribute die Knoten haben. Dann schiebe ich (Stichwort Monkey-Patching) den Klassen passende `eval`-Methoden unter, die ich aus reiner Bequemlichkeit in der Klasse `Methods` sammle.

Ein Ausdruck scheint immer ein Exemplar von `Expression` zu sein, was ein Attribut `body` hat, wo in meinem Beispiel ein `BinOp`-Exemplar drin in. Das hat zwei Attribute `left` und `right` und eines namens `op`, wo ein ansonsten wohl funktionsloses Exemplar von `Add` drin steckt, was sagt, dass es sich um eine Addition handelt. Der Rest ist ein normaler rekursiv absteigender Interpreter.

Ich fand das so einfacher und übersichtlicher, als `ast.walk` zu benutzen.

Eine Alternative zum Interpretieren wäre, statisch zu prüfen, ob der Code okay ist, und den AST dann mit `compile` zu einem Code-Objekt machen, das man mit `eval` ausführen kann. Ich bezweifle allerdings, dass eine statische Prüfung ausreicht. Daher müsste man wohl -- was geht -- den AST umschreiben und an strategisch wichtigen Stellen Prüfungen einbauen, bevor man übersetzt und ausführt. Was für ein Aufwand das sein kann, kann man an dem Caja-Projekt für JavaScript sehen.

Stefan

Verfasst: Dienstag 13. Oktober 2009, 21:38
von ms4py
Dieses Code-Beispiel ist relativ leicht nachzuvollziehen.
Allerdings komme ich bei komplizierteren Fällen nicht weiter.

Wie kann ich z.B. vorgehen, wenn ich den gesamten Ausdruck nicht komplett auswerten will, sondern Variablen als Variablen erhalten bleiben sollen?

Außerdem weiß ich nicht, wie ich ``Call`` und ``Subscript`` behandeln soll.

Background:
Versuche Benutzerdefinierte Funktionen in meinem Programm einzubinden, Bsp.:

Code: Alles auswählen

funcstring = 'lambda x, p: p["height"] * exp(-((x-p["mu"])/p["sigma"])**2)'

Verfasst: Mittwoch 14. Oktober 2009, 20:45
von sma

Code: Alles auswählen

def Call_eval(call, c):
    func = call.func.eval(c)
    args = [a.eval(c) for a in call.args]
    # TODO: keywords, starargs, kwargs
    return func(*args)
    
def Subscript_eval(subscr, c):
    value = subscr.value.eval(c)
    slice = subscr.slice.eval(c)
    return value[slice]

def Index_eval(index, c):
    return index.value.eval(c)
Ein Lambda zu übersetzen ist tricky, denn da muss man ja eine Funktion zurück geben, die erst wenn sie aufgerufen wird, den AST auswertet. Stattdessen bietet sich vielleicht an, lieber eine neue geprüfte Funktion als String zurückzugeben, die dann dem "echten" eval() übergeben wird. statisches Prüfen kann aber nicht ausreichend sein sodass man Prüffunktionen zur Laufzeit in diesen String hineingenerieren muss.

Hatte ich schon mal das Paper über "Caja", einer sicheren JavaScript-Variante erwähnt. Das ist in diesem Kontext durchaus lesenswert.

Stefan

Verfasst: Donnerstag 15. Oktober 2009, 12:19
von ms4py
So, hab da mal was gebastelt:
http://paste.pocoo.org/show/145054/
Ist das das, was du unter "statischem Prüfen" verstehst oder wäre das an sich schon sicher genug?

Hinweis an die anderen:
Die __call__ Methoden müssen in der Mitte drei Unterstriche haben,
bin daran fast verzweifelt... :?

Verfasst: Sonntag 18. Oktober 2009, 10:52
von sma
Nein, das meinte ich nicht mit statisch prüfen. Du berechnest das Ergebnis.

Bei einer statischen Prüfung würdest du "3+sin(x)" anschauen und feststellen, dass hier nichts passieren kann, weil "sin" eine der erlaubten Funktionen ist und "+" auch harmlos ist und dann am Ende einfach das normale "eval" benutzen. Die Alternative, die ich ansprach, könnte den Code zu "3+__checked_call__(sin, x)" umschreiben, was dann zur Laufzeit prüft, ob die Funktion aus "sin" erlaubt ist.

Da du an globalen Variablen aber nur sin und exp erlaubst sowie wie als lokale Variable und x als Konstante für 1 verhinderst du IMHO alle (die meisten?) Tricks. Zu beachten ist, dass du nicht aus Versehen eine benutzerdefinierte Funktion hineinreichst. Diese hat ja ein Code-Objekt, dessen Klasse du dir holen könntest und dann könntest du wahrscheinlich dessen Konstruktor nutzen, um ein neues Code-Objekt und darauf ein neues Funktionsobjekt zu bauen und dir so wieder jede beliebige Funktion im Bytecode bauen.

Stefan

Verfasst: Sonntag 18. Oktober 2009, 11:01
von ms4py
sma hat geschrieben:Nein, das meinte ich nicht mit statisch prüfen. Du berechnest das Ergebnis.

Bei einer statischen Prüfung würdest du "3+sin(x)" anschauen und feststellen, dass hier nichts passieren kann, weil "sin" eine der erlaubten Funktionen ist und "+" auch harmlos ist und dann am Ende einfach das normale "eval" benutzen.
Ähm ja, das mache ich ja dann auch (Zeile 100). Dass ich beim Parsen das Ergebnis berechne, soll im Prinzip nur noch zusätzliche Sicherheit bieten. Ich will damit meine erstellte lambda-Funktion noch evaluieren (Z. 101).
Mit der erstellten Funktion ``f`` will ich dann weiter arbeiten.
sma hat geschrieben: Da du an globalen Variablen aber nur sin und exp erlaubst sowie wie als lokale Variable und x als Konstante für 1 verhinderst du IMHO alle (die meisten?) Tricks. Zu beachten ist, dass du nicht aus Versehen eine benutzerdefinierte Funktion hineinreichst. Diese hat ja ein Code-Objekt, dessen Klasse du dir holen könntest und dann könntest du wahrscheinlich dessen Konstruktor nutzen, um ein neues Code-Objekt und darauf ein neues Funktionsobjekt zu bauen und dir so wieder jede beliebige Funktion im Bytecode bauen.
Wenn ich nur Strings parse, dürfte dieses Problem ja nicht auftreten?

Verfasst: Sonntag 18. Oktober 2009, 11:16
von sma
Das parallele Ausführen führt IMHO nicht zu mehr sondern zu weniger Sicherheit. Das könnte ja schon Schadcode ausführen, der dann weitere Tests aushebelt.

Das du Strings parst, verhindert nicht, dass jemand ausnutzen kann, wenn du über dein Environment eine benutzerdefinierte Funktion hineinreichst, diese nutzen kann.

Code: Alles auswählen

def a(): pass

print a.__class__(a.func_code.__class__(0, 0, 1, 67, "d\x01\x00S", (None, 7), (), (), "", "", 1, ""), {})()
Stefan

Verfasst: Sonntag 18. Oktober 2009, 11:50
von ms4py
Sorry, aber ich versteh echt gar nichts von deinem Post, weder das erste noch das zweite. :?
Das erste ist mir einfach nur unklar und beim zweiten kann ich mir nicht vorstellen, wie so ein Konstrukt über meinen Parser ausgeführt werden kann.

Verfasst: Sonntag 18. Oktober 2009, 14:44
von birkenfeld
In der Tat fehlen dazu einige AST-Elemente, z.B. `Attribute`. Das sollte dann aber auch so bleiben...

Verfasst: Sonntag 18. Oktober 2009, 19:05
von ms4py
birkenfeld hat geschrieben:In der Tat fehlen dazu einige AST-Elemente, z.B. `Attribute`. Das sollte dann aber auch so bleiben...
Also findest du meinen Parser sicher genug?
(Mehr kommt auch nicht hinzu, höchstens noch die Generierung der Funktion als LaTeX-String)

Verfasst: Sonntag 18. Oktober 2009, 21:00
von birkenfeld
Dazu hab ich nicht genau genug hingeschaut. "Das ist 100% sicher" is aber so ne Aussage, die man besser nicht leichtsinnig macht :)