[REF] base: safe_eval rewrite using AST #75
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ghost
force-pushed
the
master-safe_eval_redo-joda
branch
from
db008b3
to
89e9aa1
Compare
KangOl
reviewed
Apr 29, 2024
| @@ -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)) | |||
There was a problem hiding this comment.
Why passing the evaluation_context as locals_dict in this case?
There was a problem hiding this comment.
iirc, if you pass it in the globals it will break.
src/util/fields.py
Outdated
Comment on lines
1151
to
1154
| if version_gte("saas~17.2"): | ||
| context = safe_eval(act.get("context", "{}"), eval_context, nocopy=True) | ||
| else: | ||
| context = safe_eval(act.get("context", "{}"), eval_context, nocopy=True) |
ghost
force-pushed
the
master-safe_eval_redo-joda
branch
2 times, most recently
from
22fa28d
to
a88fcb2
Compare
|
upgradeci retry with always only base hr in 17.0 master |
ghost
force-pushed
the
master-safe_eval_redo-joda
branch
from
a88fcb2
to
8a9ff1f
Compare
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/
ghost
force-pushed
the
master-safe_eval_redo-joda
branch
from
8a9ff1f
to
6d34f37
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context
safe_evalis 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
sysmodule)Goal of the change
During this rewrite we had a few goals:
Retain compatibility with the original version:
and deleting)
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
strandint) and the ones that we only allow asinstance, 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
.formatand.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/
--
See odoo/odoo#138611 odoo/enterprise#48919 odoo/upgrade#5653