[IMP] refactoring of check_access_rule to avoid SQL injection and simplify code
bzr revid: odo@openerp.com-20100330172806-p1zkvrmupw5zosai
This commit is contained in:
parent
01578fff22
commit
6c23c63afe
|
@ -2572,17 +2572,16 @@ class orm(orm_template):
|
||||||
return '"%s"' % (f,)
|
return '"%s"' % (f,)
|
||||||
fields_pre2 = map(convert_field, fields_pre)
|
fields_pre2 = map(convert_field, fields_pre)
|
||||||
order_by = self._parent_order or self._order
|
order_by = self._parent_order or self._order
|
||||||
for i in range(0, len(ids), cr.IN_MAX):
|
for sub_ids in cr.split_for_in_conditions(ids):
|
||||||
sub_ids = ids[i:i+cr.IN_MAX]
|
|
||||||
if d1:
|
if d1:
|
||||||
cr.execute('SELECT %s FROM %s WHERE %s.id = ANY (%%s) AND %s ORDER BY %s' % \
|
cr.execute('SELECT %s FROM %s WHERE %s.id IN %%s AND %s ORDER BY %s' % \
|
||||||
(','.join(fields_pre2 + [self._table + '.id']), ','.join(tables), self._table, ' and '.join(d1),
|
(','.join(fields_pre2 + [self._table + '.id']), ','.join(tables), self._table, ' and '.join(d1),
|
||||||
order_by),[sub_ids,]+d2)
|
order_by),[sub_ids,]+d2)
|
||||||
if not cr.rowcount == len({}.fromkeys(sub_ids)):
|
if cr.rowcount != len(sub_ids):
|
||||||
raise except_orm(_('AccessError'),
|
raise except_orm(_('AccessError'),
|
||||||
_('You try to bypass an access rule while reading (Document type: %s).') % self._description)
|
_('You try to bypass an access rule while reading (Document type: %s).') % self._description)
|
||||||
else:
|
else:
|
||||||
cr.execute('SELECT %s FROM \"%s\" WHERE id = ANY (%%s) ORDER BY %s' %
|
cr.execute('SELECT %s FROM \"%s\" WHERE id IN %%s ORDER BY %s' %
|
||||||
(','.join(fields_pre2 + ['id']), self._table,
|
(','.join(fields_pre2 + ['id']), self._table,
|
||||||
order_by), (sub_ids,))
|
order_by), (sub_ids,))
|
||||||
res.extend(cr.dictfetchall())
|
res.extend(cr.dictfetchall())
|
||||||
|
@ -2734,29 +2733,24 @@ class orm(orm_template):
|
||||||
if res and res[0]:
|
if res and res[0]:
|
||||||
raise except_orm('ConcurrencyException', _('Records were modified in the meanwhile'))
|
raise except_orm('ConcurrencyException', _('Records were modified in the meanwhile'))
|
||||||
|
|
||||||
def check_access_rule(self, cr, uid, ids, mode, context=None):
|
def check_access_rule(self, cr, uid, ids, operation, context=None):
|
||||||
d1, d2, tables = self.pool.get('ir.rule').domain_get(cr, uid, self._name, mode, context=context)
|
"""Verifies that the operation given by ``operation`` is allowed for the user
|
||||||
if d1:
|
according to ir.rules.
|
||||||
d1 = ' and '+' and '.join(d1)
|
@param ``operation``: one of ``'read'``, ``'write'``, ``'unlink'``
|
||||||
|
@raise ``except_orm``: if current ir.rules do not permit this operation.
|
||||||
for i in range(0, len(ids), cr.IN_MAX):
|
@return: ``None`` if the operation is allowed
|
||||||
sub_ids = ids[i:i+cr.IN_MAX]
|
"""
|
||||||
if d1:
|
where_clause, where_params, tables = self.pool.get('ir.rule').domain_get(cr, uid, self._name, operation, context=context)
|
||||||
cr.execute('SELECT '+self._table+'.id FROM '+','.join(tables)+' ' \
|
if where_clause:
|
||||||
'WHERE '+self._table+'.id IN %s'+d1, (tuple(sub_ids),d2))
|
where_clause = ' and ' + ' and '.join(where_clause)
|
||||||
if not cr.rowcount == len(sub_ids):
|
for sub_ids in cr.split_for_in_conditions(ids):
|
||||||
|
cr.execute('SELECT ' + self._table + '.id FROM ' + ','.join(tables) +
|
||||||
|
' WHERE ' + self._table + '.id IN %s' + where_clause,
|
||||||
|
[sub_ids] + where_params)
|
||||||
|
if cr.rowcount != len(sub_ids):
|
||||||
raise except_orm(_('AccessError'),
|
raise except_orm(_('AccessError'),
|
||||||
_('You try to bypass an access rule to '+mode+
|
_('Operation prohibited by access rules (Operation: %s, Document type: %s).')
|
||||||
' (Document type: %s).') % self._name)
|
% (operation, self._name))
|
||||||
else:
|
|
||||||
cr.execute('SELECT id FROM "'+self._table+'" WHERE id IN %s',
|
|
||||||
(tuple(sub_ids),))
|
|
||||||
if not cr.rowcount == len(sub_ids):
|
|
||||||
raise except_orm(_('AccessError'),
|
|
||||||
_('You try to ' +mode+ ' a record that doesn\'t exist (Document type: %s).')
|
|
||||||
% self._name)
|
|
||||||
#TODO: this is a SQL injection pattern again, need to refactor it
|
|
||||||
return ','.join(map(str,ids))
|
|
||||||
|
|
||||||
def unlink(self, cr, uid, ids, context=None):
|
def unlink(self, cr, uid, ids, context=None):
|
||||||
if not ids:
|
if not ids:
|
||||||
|
@ -2787,12 +2781,13 @@ class orm(orm_template):
|
||||||
# ids2 = [x[self._inherits[key]] for x in res]
|
# ids2 = [x[self._inherits[key]] for x in res]
|
||||||
# self.pool.get(key).unlink(cr, uid, ids2)
|
# self.pool.get(key).unlink(cr, uid, ids2)
|
||||||
|
|
||||||
ids_str = self.check_access_rule(cr, uid, ids, 'unlink', context=context)
|
self.check_access_rule(cr, uid, ids, 'unlink', context=context)
|
||||||
cr.execute('delete from '+self._table+' ' \
|
for sub_ids in cr.split_for_in_conditions(ids):
|
||||||
'where id in ('+ids_str+')', ids)
|
cr.execute('delete from ' + self._table + ' ' \
|
||||||
|
'where id in %s', sub_ids)
|
||||||
|
|
||||||
for order, object, store_ids, fields in result_store:
|
for order, object, store_ids, fields in result_store:
|
||||||
if object<>self._name:
|
if object != self._name:
|
||||||
obj = self.pool.get(object)
|
obj = self.pool.get(object)
|
||||||
cr.execute('select id from '+obj._table+' where id in ('+','.join(map(str, store_ids))+')')
|
cr.execute('select id from '+obj._table+' where id in ('+','.join(map(str, store_ids))+')')
|
||||||
rids = map(lambda x: x[0], cr.fetchall())
|
rids = map(lambda x: x[0], cr.fetchall())
|
||||||
|
@ -2888,9 +2883,10 @@ class orm(orm_template):
|
||||||
upd1.append(user)
|
upd1.append(user)
|
||||||
|
|
||||||
if len(upd0):
|
if len(upd0):
|
||||||
ids_str = self.check_access_rule(cr, user, ids, 'write', context=context)
|
self.check_access_rule(cr, user, ids, 'write', context=context)
|
||||||
cr.execute('update '+self._table+' set '+string.join(upd0, ',')+' ' \
|
for sub_ids in cr.split_for_in_conditions(ids):
|
||||||
'where id in ('+ids_str+')', upd1)
|
cr.execute('update ' + self._table + ' set ' + ','.join(upd0) + ' ' \
|
||||||
|
'where id in %s', upd1 + [sub_ids])
|
||||||
|
|
||||||
if totranslate:
|
if totranslate:
|
||||||
# TODO: optimize
|
# TODO: optimize
|
||||||
|
@ -2921,11 +2917,9 @@ class orm(orm_template):
|
||||||
for table in self._inherits:
|
for table in self._inherits:
|
||||||
col = self._inherits[table]
|
col = self._inherits[table]
|
||||||
nids = []
|
nids = []
|
||||||
for i in range(0, len(ids), cr.IN_MAX):
|
for sub_ids in cr.split_for_in_conditions(ids):
|
||||||
sub_ids = ids[i:i+cr.IN_MAX]
|
|
||||||
ids_str = string.join(map(str, sub_ids), ',')
|
|
||||||
cr.execute('select distinct "'+col+'" from "'+self._table+'" ' \
|
cr.execute('select distinct "'+col+'" from "'+self._table+'" ' \
|
||||||
'where id in ('+ids_str+')', upd1)
|
'where id in %s', (sub_ids,))
|
||||||
nids.extend([x[0] for x in cr.fetchall()])
|
nids.extend([x[0] for x in cr.fetchall()])
|
||||||
|
|
||||||
v = {}
|
v = {}
|
||||||
|
|
Loading…
Reference in New Issue