Hi all,

the following patch checks the format of parameters passed to a method called through the batch command. I picked the ConversionError for invalid parameters format but this choice can be discussed if you have better suggestions...


Fixes: https://fedorahosted.org/freeipa/ticket/5810

--
Florence Blanc-Renaud
Identity Management Team, Red Hat

From 3695130deb27584a3f154505d905f4cec91913c8 Mon Sep 17 00:00:00 2001
From: Florence Blanc-Renaud <fren...@redhat.com>
Date: Fri, 27 May 2016 08:19:39 +0200
Subject: [PATCH] batch command can be used to trigger internal errors on
 server

In ipalib, the batch command expects the following format for arguments:
api.Command['batch']([
    {'method':'method1', 'params': ([],{})}
    {'method':'method2', 'params': ([],{})}
])

ie a list containing dict, each dict corresponding to a method to call.
The parameters passed to the methods must be stored inside a tuple (list, dict)
with the list containing the positional arguments and the dict the
keyword arguments.

The code did not check the format of the parameters, which could trigger
internal errors on the server.
With this fix:
- a ConversionError is raised if the arg passed to batch() is not a list of dict
- the result appended to the batch results is a ConversionError if the 'params'
does not contain a tuple(list,dict)

https://fedorahosted.org/freeipa/ticket/5810
---
 ipalib/plugins/batch.py | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/ipalib/plugins/batch.py b/ipalib/plugins/batch.py
index 84a65057588e01e18a937c6e79f0974b25c529ed..41e98821b1fa715e4619099539b42ba8b5ff8d7b 100644
--- a/ipalib/plugins/batch.py
+++ b/ipalib/plugins/batch.py
@@ -90,6 +90,13 @@ class batch(Command):
     def execute(self, methods=None, **options):
         results = []
         for arg in (methods or []):
+            # As take_args = Any, no check is done before
+            # Need to make sure that methods contain dict objects
+            # https://fedorahosted.org/freeipa/ticket/5810
+            if not isinstance(arg, dict):
+                raise errors.ConversionError(
+                    name='methods',
+                    error=_(u'must contain dict objects'))
             params = dict()
             name = None
             try:
@@ -100,9 +107,22 @@ class batch(Command):
                 name = arg['method']
                 if name not in self.Command:
                     raise errors.CommandError(name=name)
-                a, kw = arg['params']
-                newkw = dict((str(k), v) for k, v in kw.items())
-                params = api.Command[name].args_options_2_params(*a, **newkw)
+
+                # If params are not formated as a tuple(list, dict)
+                # the following lines will raise an exception
+                # that triggers an internal server error
+                # Raise a ConversionError instead to report the issue
+                # to the client
+                # https://fedorahosted.org/freeipa/ticket/5810
+                try:
+                    a, kw = arg['params']
+                    newkw = dict((str(k), v) for k, v in kw.items())
+                    params = api.Command[name].args_options_2_params(
+                        *a, **newkw)
+                except (AttributeError, ValueError, TypeError):
+                    raise errors.ConversionError(
+                        name='params',
+                        error=_(u'must contain a tuple (list, dict)'))
                 newkw.setdefault('version', options['version'])
 
                 result = api.Command[name](*a, **newkw)
-- 
2.5.5

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to