From 12634e1227ab8446e34b747e02865f62087b5e86 Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Fri, 9 Sep 2016 15:08:10 +0200 Subject: [PATCH 1/3] [FIX] base_import_module, tools: raise a clear error for unsupported file types Also restrict XML data attribute evaluation context even for real module data files. This will prevent accidentally depending on context parameters that would not be available inside base_import_module. --- addons/base_import_module/models/ir_module.py | 4 ++ openerp/tools/convert.py | 41 +++++++++---------- 2 files changed, 23 insertions(+), 22 deletions(-) diff --git a/addons/base_import_module/models/ir_module.py b/addons/base_import_module/models/ir_module.py index df5587569ff..e81035bda31 100644 --- a/addons/base_import_module/models/ir_module.py +++ b/addons/base_import_module/models/ir_module.py @@ -41,6 +41,10 @@ class view(osv.osv): for kind in ['data', 'init_xml', 'update_xml']: for filename in terp[kind]: + ext = os.path.splitext(filename)[1].lower() + if ext not in ('.xml', '.csv', '.sql'): + _logger.info("module %s: skip unsupported file %s", module, filename) + continue _logger.info("module %s: loading %s", module, filename) noupdate = False if filename.endswith('.csv') and kind in ('init', 'init_xml'): diff --git a/openerp/tools/convert.py b/openerp/tools/convert.py index be30b32c3cd..da1db9bf4c0 100644 --- a/openerp/tools/convert.py +++ b/openerp/tools/convert.py @@ -61,11 +61,8 @@ from misc import pickle, unquote from openerp import SUPERUSER_ID -# Import of XML records requires the unsafe eval as well, -# almost everywhere, which is ok because it supposedly comes -# from trusted data, but at least we make it obvious now. -unsafe_eval = eval -from safe_eval import safe_eval as eval +from safe_eval import safe_eval as s_eval +safe_eval = lambda expr, ctx={}: s_eval(expr, ctx, nocopy=True) class ParseError(Exception): def __init__(self, msg, text, filename, lineno): @@ -130,7 +127,7 @@ def _eval_xml(self, node, pool, cr, uid, idref, context=None): idref2 = {} if f_search: idref2 = _get_idref(self, cr, uid, f_model, context, idref) - q = unsafe_eval(f_search, idref2) + q = safe_eval(f_search, idref2) ids = pool[f_model].search(cr, uid, q) if f_use != 'id': ids = map(lambda x: x[f_use], pool[f_model].read(cr, uid, ids, [f_use])) @@ -147,7 +144,7 @@ def _eval_xml(self, node, pool, cr, uid, idref, context=None): if a_eval: idref2 = _get_idref(self, cr, uid, f_model, context, idref) try: - return unsafe_eval(a_eval, idref2) + return safe_eval(a_eval, idref2) except Exception: logging.getLogger('openerp.tools.convert.init').error( 'Could not eval(%s) for %s in %s', a_eval, node.get('name'), context) @@ -219,7 +216,7 @@ def _eval_xml(self, node, pool, cr, uid, idref, context=None): # FIXME: should probably be exclusive if a_eval: idref['ref'] = lambda x: self.id_get(cr, x) - args = unsafe_eval(a_eval, idref) + args = safe_eval(a_eval, idref) for n in node: return_val = _eval_xml(self,n, pool, cr, uid, idref, context) if return_val is not None: @@ -255,12 +252,12 @@ class xml_import(object): for ctx in (data_node_context, node_context): if ctx: try: - ctx_res = unsafe_eval(ctx, eval_dict) + ctx_res = safe_eval(ctx, eval_dict) if isinstance(context, dict): context.update(ctx_res) else: context = ctx_res - except NameError: + except ValueError, NameError: # Some contexts contain references that are only valid at runtime at # client-side, so in that case we keep the original context string # as it is. We also log it, just in case. @@ -299,7 +296,7 @@ form: module.record_id""" % (xml_id,) if d_search: idref = _get_idref(self, cr, self.uid, d_model, context={}, idref={}) try: - ids = self.pool[d_model].search(cr, self.uid, unsafe_eval(d_search, idref)) + ids = self.pool[d_model].search(cr, self.uid, safe_eval(d_search, idref)) except ValueError: _logger.warning('Skipping deletion for failed search `%r`', d_search, exc_info=True) pass @@ -332,14 +329,14 @@ form: module.record_id""" % (xml_id,) if rec.get(field): res[dest] = rec.get(field).encode('utf8') if rec.get('auto'): - res['auto'] = eval(rec.get('auto','False')) + res['auto'] = safe_eval(rec.get('auto','False')) if rec.get('sxw'): sxw_content = misc.file_open(rec.get('sxw')).read() res['report_sxw_content'] = sxw_content if rec.get('header'): - res['header'] = eval(rec.get('header','False')) + res['header'] = safe_eval(rec.get('header','False')) - res['multi'] = rec.get('multi') and eval(rec.get('multi','False')) + res['multi'] = rec.get('multi') and safe_eval(rec.get('multi','False')) xml_id = rec.get('id','').encode('utf8') self._test_xml_id(xml_id) @@ -359,12 +356,12 @@ form: module.record_id""" % (xml_id,) id = self.pool['ir.model.data']._update(cr, self.uid, "ir.actions.report.xml", self.module, res, xml_id, noupdate=self.isnoupdate(data_node), mode=self.mode) self.idref[xml_id] = int(id) - if not rec.get('menu') or eval(rec.get('menu','False')): + if not rec.get('menu') or safe_eval(rec.get('menu','False')): keyword = str(rec.get('keyword', 'client_print_multi')) value = 'ir.actions.report.xml,'+str(id) replace = rec.get('replace', True) self.pool['ir.model.data'].ir_set(cr, self.uid, 'action', keyword, res['name'], [res['model']], value, replace=replace, isobject=True, xml_id=xml_id) - elif self.mode=='update' and eval(rec.get('menu','False'))==False: + elif self.mode=='update' and safe_eval(rec.get('menu','False'))==False: # Special check for report having attribute menu=False on update value = 'ir.actions.report.xml,'+str(id) self._remove_ir_values(cr, res['name'], value, res['model']) @@ -448,7 +445,7 @@ form: module.record_id""" % (xml_id,) context = self.get_context(data_node, rec, eval_context) try: - domain = unsafe_eval(domain, eval_context) + domain = safe_eval(domain, eval_context) except NameError: # Some domains contain references that are only valid at runtime at # client-side, so in that case we keep the original domain string @@ -486,7 +483,7 @@ form: module.record_id""" % (xml_id,) if rec.get('target'): res['target'] = rec.get('target','') if rec.get('multi'): - res['multi'] = eval(rec.get('multi', 'False')) + res['multi'] = safe_eval(rec.get('multi', 'False')) id = self.pool['ir.model.data']._update(cr, self.uid, 'ir.actions.act_window', self.module, res, xml_id, noupdate=self.isnoupdate(data_node), mode=self.mode) self.idref[xml_id] = int(id) @@ -643,7 +640,7 @@ form: module.record_id""" % (xml_id,) if rec_id: ids = [self.id_get(cr, rec_id)] elif rec_src: - q = unsafe_eval(rec_src, eval_dict) + q = safe_eval(rec_src, eval_dict) ids = self.pool[rec_model].search(cr, uid, q, context=context) if rec_src_count: count = int(rec_src_count) @@ -674,7 +671,7 @@ form: module.record_id""" % (xml_id,) for test in rec.findall('./test'): f_expr = test.get("expr",'').encode('utf-8') expected_value = _eval_xml(self, test, self.pool, cr, uid, self.idref, context=context) or True - expression_value = unsafe_eval(f_expr, globals_dict) + expression_value = safe_eval(f_expr, globals_dict) if expression_value != expected_value: # assertion failed self.assertion_report.record_failure() msg = 'assertion "%s" failed!\n' \ @@ -693,7 +690,7 @@ form: module.record_id""" % (xml_id,) rec_id = rec.get("id",'').encode('ascii') rec_context = rec.get("context", None) if rec_context: - rec_context = unsafe_eval(rec_context) + rec_context = safe_eval(rec_context) self._test_xml_id(rec_id) # in update mode, the record won't be updated if the data node explicitely # opt-out using @noupdate="1". A second check will be performed in @@ -732,7 +729,7 @@ form: module.record_id""" % (xml_id,) f_val = False if f_search: - q = unsafe_eval(f_search, self.idref) + q = safe_eval(f_search, self.idref) assert f_model, 'Define an attribute model="..." in your .XML file !' f_obj = self.pool[f_model] # browse the objects searched From ba7880a249cb2c469bff6c2aef3ae3300c2a3e44 Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Mon, 3 Oct 2016 17:43:44 +0200 Subject: [PATCH 2/3] [FIX] base: force local cursor in case of mismatch --- openerp/addons/base/res/res_users.py | 1 + 1 file changed, 1 insertion(+) diff --git a/openerp/addons/base/res/res_users.py b/openerp/addons/base/res/res_users.py index a181e141eb4..58e0d216a07 100644 --- a/openerp/addons/base/res/res_users.py +++ b/openerp/addons/base/res/res_users.py @@ -511,6 +511,7 @@ class res_users(osv.osv): if not passwd: # empty passwords disallowed for obvious security reasons raise openerp.exceptions.AccessDenied() + db = self.pool.db_name if self.__uid_cache.setdefault(db, {}).get(uid) == passwd: return cr = self.pool.cursor() From 3d55aff1c83d3157228852d1fe28efe82d7cc329 Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Mon, 3 Oct 2016 23:41:44 +0200 Subject: [PATCH 3/3] [FIX] convert: properly catch Value/Name Errors --- openerp/tools/convert.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openerp/tools/convert.py b/openerp/tools/convert.py index da1db9bf4c0..a24f6640896 100644 --- a/openerp/tools/convert.py +++ b/openerp/tools/convert.py @@ -257,7 +257,7 @@ class xml_import(object): context.update(ctx_res) else: context = ctx_res - except ValueError, NameError: + except (ValueError, NameError): # Some contexts contain references that are only valid at runtime at # client-side, so in that case we keep the original context string # as it is. We also log it, just in case. @@ -446,7 +446,7 @@ form: module.record_id""" % (xml_id,) try: domain = safe_eval(domain, eval_context) - except NameError: + except (ValueError, NameError): # Some domains contain references that are only valid at runtime at # client-side, so in that case we keep the original domain string # as it is. We also log it, just in case.