openstack/python-neutronclient
Code Issues Proposed changes

Replace insecure function eval

Browse Source 
In neutronclient we use a eval function for processing
CLI neutron arguments. This function possible insecure because
eval get argument from client side. Instead of it we can use
a dict with allowed types which is more secure.

Closes-Bug: #1762938
Change-Id: Idde55d1b9206e9ef8742464825709f098d488a8e
Co-Authored-By: Akihiro Motoki <amotoki@gmail.com>
This commit is contained in:
Gleb Zimin 2018-03-22 17:18:03 +04:00 committed by Akihiro Motoki
parent 19d0609888
commit 2cf52672b7
2 changed files with 39 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
neutronclient/neutron/v2_0/__init__.py
View File

@ -185,6 +185,14 @@ def parse_args_to_dict(values_specs):
current_item = None
# the str after 'type='
current_type_str = None
# dict of allowed types
allowed_type_dict = {
'bool': utils.str2bool,
'dict': utils.str2dict,
'int': int,
'str': str,
}
for _item in values_specs_copy:
if _item.startswith('--'):
# Deal with previous argument if any
@ -215,12 +223,16 @@ def parse_args_to_dict(values_specs):
_("Invalid values_specs %s") % ' '.join(values_specs))
if 'type' not in current_arg:
current_type_str = _item.split('=', 2)[1]
current_arg.update({'type': eval(current_type_str)})
if current_type_str == 'bool':
current_arg.update({'type': utils.str2bool})
elif current_type_str == 'dict':
current_arg.update({'type': utils.str2dict})
continue
if current_type_str in allowed_type_dict:
current_arg['type'] = allowed_type_dict[current_type_str]
continue
else:
raise exceptions.CommandError(
_("Invalid value_specs {valspec}: type {curtypestr}"
" is not supported").format(
valspec=' '.join(values_specs),
curtypestr=current_type_str))
elif _item == 'list=true':
_list_flag = True
continue
@ -240,7 +252,6 @@ def parse_args_to_dict(values_specs):
if _item.startswith('---'):
raise exceptions.CommandError(
_("Invalid values_specs %s") % ' '.join(values_specs))
_values_specs.append(_item)
# Deal with last one argument

24
neutronclient/tests/unit/test_casual_args.py
View File

@ -14,6 +14,7 @@
# under the License.
#
import six
import testtools
from neutronclient.common import exceptions
@ -41,6 +42,13 @@ class CLITestArgs(testtools.TestCase):
_mydict = neutronV20.parse_args_to_dict(_specs)
self.assertFalse(_mydict['my_bool'])
def test_int_and_str(self):
_specs = ['--my-int', 'type=int', '10',
'--my-str', 'type=str', 'value1']
_mydict = neutronV20.parse_args_to_dict(_specs)
self.assertEqual(10, _mydict['my_int'])
self.assertEqual('value1', _mydict['my_str'])
def test_nargs(self):
_specs = ['--tag', 'x', 'y', '--arg1', 'value1']
_mydict = neutronV20.parse_args_to_dict(_specs)
@ -102,15 +110,25 @@ class CLITestArgs(testtools.TestCase):
self.assertEqual('value1', arg1[0]['key1'])
self.assertEqual('value2', arg1[0]['key2'])
def test_parse_args_to_dict_bad_type(self):
_specs = ['--badtypearg', 'type=badtype', 'val1']
ex = self.assertRaises(exceptions.CommandError,
neutronV20.parse_args_to_dict, _specs)
self.assertEqual('Invalid value_specs --badtypearg type=badtype val1: '
'type badtype is not supported',
six.text_type(ex))
def test_clear_action(self):
_specs = ['--anyarg', 'action=clear']
args = neutronV20.parse_args_to_dict(_specs)
self.assertIsNone(args['anyarg'])
def test_bad_values_str(self):
def test_bad_values_str_without_value(self):
_specs = ['--strarg', 'type=str']
self.assertRaises(exceptions.CommandError,
neutronV20.parse_args_to_dict, _specs)
ex = self.assertRaises(exceptions.CommandError,
neutronV20.parse_args_to_dict, _specs)
self.assertEqual('Invalid values_specs --strarg type=str',
six.text_type(ex))
def test_bad_values_list(self):
_specs = ['--listarg', 'list=true', 'type=str']