[REF] base: safe_eval rewrite using AST #138611

ghost · 2023-10-13T09:57:49Z

Context

safe_eval is Odoo's sandbox, this mechanism allows users and developpers to write templates, server actions, and more without worrying about the security risks associated with arbitrary code execution.

The current version of the sandbox heavily relies on Python's bytecodes and compile-time verifications. This causes 2 major problems:

In every release of Python, its bytecodes are updated or modified.
Which makes Odoo unusable until the security / framework team updates the
whitelist of bytecode.
Most sandboxing issues we have faced for the last years was due to a
lack of runtime checks (functions inputs (arguments) and outputs
(return values)). All most every times those kind of issues were
fixed with "dirty" hacks such as adding a list of
"unsafe attribute" or adding a wrapper for modules that are exposing
unsafe objects (such as the sys module)

Goal of the change

During this rewrite we had a few goals:

Retain compatibility with the original version:
- Find a way to keep the old checks (deny dunders, attribute storing
  and deleting)
- Keep the same exposed API, limiting the amount of code that needs
  to be rewritten as much as possible
Add runtime checks to verify that every types passed and returns are
safe by checking their type. The way that the sandbox does it is by
using two set of types. One for the types we allow to instanciate
(the ones that we have absolute trust, most of them are primitive
types such as str and int) and the ones that we only allow as
instance, this means that you CANNOT instanciate them inside of the
sandbox (for example the sql cursor or the Odoo environement).
Eliminate the issues with the .format and .format_map.
This is a well known issue within the Python security community, if
you want more info : https://lucumr.pocoo.org/2016/12/29/careful-with-str-format/

Linked with https://github.com/odoo/enterprise/pull/48919

robodoo · 2023-10-13T09:58:49Z

std-odoo

@joda-odoo Hi :)

Some comments and ideas while passing :)

Cheers

odoo/tools/safe_eval.py

odoo/addons/base/tests/test_base.py

std-odoo

@joda-odoo Hi :)

A few more comments / ideas

Cheers

odoo/tools/safe_eval.py

std-odoo · 2023-10-19T06:30:50Z

odoo/tools/safe_eval.py

+    def __getitem__(self, item):
+        self.__type_checker(item)
+        return self.__type_checker(self.__obj.__getitem__(item))
+
+    def __setitem__(self, key, value):
+        self.__type_checker(key)


SafeWrapper is not used for standad dict, for which types is it useful so ? (__setitem__)
Can't we just don't allow those operation ?

It's mostly used for lists

same, list is in allowed_type, so it won't be encapsulated in SafeWrapper isn't it ?

fn = lambda: [1, 2, 3] safe_eval('print(fn())', {'fn': fn, 'print': print}

Edit: nevermind, explore_Subscript always encapsulate in a CheckerWrapper (even if the type is in allowed_type)

if isinstance(key, str) and "__" in key: raise NameError(f"Forbidden name '{key}'.")

but that part is still useless, because you can call the update method

I'm working on that ;)

std-odoo · 2023-10-19T06:34:17Z

odoo/tools/safe_eval.py

+
+    d = new_dict
+
+def safe_eval(expr, globals_dict=None, locals_dict=None, mode="eval",


If you give in the context

test = [__import__('os')]

This will crash, which is good

test[0]

but you can just do that to bypass the check

a, = test

is there anything you can do ?

same for dict if you do list(dict.values()))[0]

Either we fix that issue, or there's no point for explore_Subscript (the less code the better, so we know excatly what is checked and what is not, and we don't think something if safe while there's a way to bypass it)

odoo/tools/safe_eval.py

Context ======= `safe_eval` is Odoo's sandbox, this mechanism allows users and developpers to write templates, server actions, and more without worrying about the security risks associated with arbitrary code execution. The current version of the sandbox heavily relies on Python's bytecodes and compile-time verifications. This causes 2 major problems: 1) In every release of Python, its bytecodes are updated or modified. Which makes Odoo unusable until the security / framework team updates the whitelist of bytecode. 2) Most sandboxing issues we have faced for the last years was due to a lack of runtime checks (functions inputs (arguments) and outputs (return values)). All most every times those kind of issues were fixed with "dirty" hacks such as adding a list of "unsafe attribute" or adding a wrapper for modules that are exposing unsafe objects (such as the `sys` module) Goal of the change ================== During this rewrite we had a few goals: 1) Retain compatibility with the original version: * Find a way to keep the old checks (deny dunders, attribute storing and deleting) * Keep the same exposed API, limiting the amount of code that needs to be rewritten as much as possible 2) Add runtime checks to verify that every types passed and returns are safe by checking their type. The way that the sandbox does it is by using two set of types. One for the types we allow to instanciate (the ones that we have absolute trust, most of them are primitive types such as `str` and `int`) and the ones that we only allow as instance, this means that you CANNOT instanciate them inside of the sandbox (for example the sql cursor or the Odoo environement). 3) Eliminate the issues with the `.format` and `.format_map`. This is a well known issue within the Python security community, if you want more info : https://lucumr.pocoo.org/2016/12/29/careful-with-str-format/

str.format are now denied in the new safe_eval

C3POdoo requested review from a team, clbr-odoo, HydrionBurst and ryv-odoo and removed request for a team October 13, 2023 09:59

C3POdoo added the RD research & development, internal work label Oct 13, 2023

ghost marked this pull request as draft October 13, 2023 10:15

ghost force-pushed the master-safe_eval_redo-joda branch 2 times, most recently from eedba39 to cda00a6 Compare October 13, 2023 11:59

ghost deleted a comment from Xavier-Do Oct 13, 2023

ghost force-pushed the master-safe_eval_redo-joda branch 5 times, most recently from c413563 to 5bd7a60 Compare October 18, 2023 11:26

std-odoo reviewed Oct 18, 2023

View reviewed changes

std-odoo reviewed Oct 19, 2023

View reviewed changes

beledouxdenis force-pushed the master-safe_eval_redo-joda branch 3 times, most recently from 8a1652b to 6213d34 Compare October 30, 2023 08:32

ghost force-pushed the master-safe_eval_redo-joda branch 2 times, most recently from 960b060 to 256d96b Compare April 8, 2024 08:57

ghost marked this pull request as ready for review April 8, 2024 11:49

C3POdoo requested review from a team and Julien00859 and removed request for a team April 8, 2024 11:52

ghost force-pushed the master-safe_eval_redo-joda branch from 256d96b to 95c3bd9 Compare April 15, 2024 13:32

ghost force-pushed the master-safe_eval_redo-joda branch 10 times, most recently from 7ba40bb to a7fe9f9 Compare April 29, 2024 07:15

KangOl mentioned this pull request Apr 29, 2024

[REF] base: safe_eval rewrite using AST odoo/upgrade-util#75

Draft

ghost force-pushed the master-safe_eval_redo-joda branch 2 times, most recently from f8f43ef to a4a700f Compare May 8, 2024 12:17

joda-odoo added 4 commits May 14, 2024 08:49

[FIX] *: Fix python implmentations according to new safe_eval's API

090a194

[FIX] *: Fix formatting in templates

f765d0c

str.format are now denied in the new safe_eval

[FIX] *: Fix templates that trigger safe_eval exceptions

2dad991

ghost force-pushed the master-safe_eval_redo-joda branch from a4a700f to 2dad991 Compare May 14, 2024 06:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[REF] base: safe_eval rewrite using AST #138611

[REF] base: safe_eval rewrite using AST #138611

ghost commented Oct 13, 2023 •

edited by ghost

Loading

robodoo commented Oct 13, 2023 •

edited

Loading

std-odoo left a comment

std-odoo left a comment

std-odoo Oct 19, 2023 •

edited

Loading

ghost Nov 6, 2023

std-odoo Nov 6, 2023

std-odoo Nov 6, 2023 •

edited

Loading

std-odoo Nov 6, 2023

ghost Nov 6, 2023

std-odoo Oct 19, 2023


		d = new_dict

		def safe_eval(expr, globals_dict=None, locals_dict=None, mode="eval",

[REF] base: safe_eval rewrite using AST #138611

Are you sure you want to change the base?

[REF] base: safe_eval rewrite using AST #138611

Conversation

ghost commented Oct 13, 2023 • edited by ghost Loading

Context

Goal of the change

robodoo commented Oct 13, 2023 • edited Loading

std-odoo left a comment

Choose a reason for hiding this comment

std-odoo left a comment

Choose a reason for hiding this comment

std-odoo Oct 19, 2023 • edited Loading

Choose a reason for hiding this comment

ghost Nov 6, 2023

Choose a reason for hiding this comment

std-odoo Nov 6, 2023

Choose a reason for hiding this comment

std-odoo Nov 6, 2023 • edited Loading

Choose a reason for hiding this comment

std-odoo Nov 6, 2023

Choose a reason for hiding this comment

ghost Nov 6, 2023

Choose a reason for hiding this comment

std-odoo Oct 19, 2023

Choose a reason for hiding this comment

ghost commented Oct 13, 2023 •

edited by ghost

Loading

robodoo commented Oct 13, 2023 •

edited

Loading

std-odoo Oct 19, 2023 •

edited

Loading

std-odoo Nov 6, 2023 •

edited

Loading