Skip to content

Commit

Permalink
[REF] base: safe_eval rewrite using AST
Browse files Browse the repository at this point in the history
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/
  • Loading branch information
joda-odoo committed May 6, 2024
1 parent 3787631 commit a88fcb2
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 4 deletions.
8 changes: 6 additions & 2 deletions src/util/domains.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
from .const import NEARLYWARN
from .helpers import _dashboard_actions, _validate_model
from .inherit import for_each_inherit
from .misc import SelfPrintEvalContext
from .misc import SelfPrintEvalContext, version_gte
from .pg import column_exists, get_value_or_en_translation, table_exists
from .records import edit_view

Expand Down Expand Up @@ -207,7 +207,11 @@ def _adapt_one_domain(cr, target_model, old, new, model, domain, adapter=None, f
# pre-check domain
if isinstance(domain, basestring):
try:
eval_dom = expression.normalize_domain(safe_eval(domain, evaluation_context, nocopy=True))
if version_gte("saas~17.2"):
eval_dom = expression.normalize_domain(safe_eval(domain, locals_dict=evaluation_context))
else:
eval_dom = expression.normalize_domain(safe_eval(domain, evaluation_context, nocopy=True))

except Exception as e:
oops = ustr(e)
_logger.log(NEARLYWARN, "Cannot evaluate %r domain: %r: %s", model, domain, oops)
Expand Down
10 changes: 8 additions & 2 deletions src/util/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,10 @@ def clean_context(context):

# clean dashboard's contexts
for id_, action in _dashboard_actions(cr, r"\y{}\y".format(fieldname), model):
context = safe_eval(action.get("context", "{}"), SelfPrintEvalContext(), nocopy=True)
if version_gte("saas~17.2"):
context = safe_eval(action.get("context", "{}"), SelfPrintEvalContext())
else:
context = safe_eval(action.get("context", "{}"), SelfPrintEvalContext(), nocopy=True)
changed = clean_context(context)
action.set("context", unicode(context))
if changed:
Expand All @@ -174,7 +177,10 @@ def clean_context(context):
[model, r"\y{}\y".format(fieldname)],
)
for id_, name, context_s in cr.fetchall():
context = safe_eval(context_s or "{}", SelfPrintEvalContext(), nocopy=True)
if version_gte("saas~17.2"):
context = safe_eval(context_s or "{}", SelfPrintEvalContext())
else:
context = safe_eval(context_s or "{}", SelfPrintEvalContext(), nocopy=True)
changed = clean_context(context)
cr.execute("UPDATE ir_filters SET context = %s WHERE id = %s", [unicode(context), id_])
if changed:
Expand Down
3 changes: 3 additions & 0 deletions src/util/misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -435,5 +435,8 @@ class SelfPrintEvalContext(collections.defaultdict):
def __init__(self, *args, **kwargs):
super(SelfPrintEvalContext, self).__init__(None, *args, **kwargs)

def copy(self):
return self

def __missing__(self, key):
return SelfPrint(key)

0 comments on commit a88fcb2

Please sign in to comment.