I'm very sorry I didn't include rest_firewall.py and rest_qos.py in my patch.
I suggested for ofctl_rest.py only...

Here is the patch for rest_firewall.py and rest_qos.py.

---------------------------------------------------------------
Subject: [PATCH] fix security problem of some RESTful apps

It is not safe to use eval function because input data(request body) is not 
checked
For example, someone can send this data to remove all files in the directory
"import('os').system('rm -rf .')"

I suggest to use json.loads to parse the request body if the data is json format
or disable builtin functions like:
eval(req.body, {"__builtins__":None})

Signed-off-by: Takeshi <a86487...@gmail.com>
Signed-off-by: IWASE Yusuke <iwase.yusu...@gmail.com>
---
 ryu/app/rest_firewall.py | 4 ++--
 ryu/app/rest_qos.py      | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/ryu/app/rest_firewall.py b/ryu/app/rest_firewall.py
index 01eb6e2..4e52b1f 100644
--- a/ryu/app/rest_firewall.py
+++ b/ryu/app/rest_firewall.py
@@ -492,7 +492,7 @@ class FirewallController(ControllerBase):
 
     def _set_rule(self, req, switchid, vlan_id=VLANID_NONE):
         try:
-            rule = eval(req.body)
+            rule = json.loads(req.body)
         except SyntaxError:
             FirewallController._LOGGER.debug('invalid syntax %s', req.body)
             return Response(status=400)
@@ -516,7 +516,7 @@ class FirewallController(ControllerBase):
 
     def _delete_rule(self, req, switchid, vlan_id=VLANID_NONE):
         try:
-            ruleid = eval(req.body)
+            ruleid = json.loads(req.body)
         except SyntaxError:
             FirewallController._LOGGER.debug('invalid syntax %s', req.body)
             return Response(status=400)
diff --git a/ryu/app/rest_qos.py b/ryu/app/rest_qos.py
index 057a3fd..537639f 100644
--- a/ryu/app/rest_qos.py
+++ b/ryu/app/rest_qos.py
@@ -499,7 +499,7 @@ class QoSController(ControllerBase):
 
     def _access_switch(self, req, switchid, vlan_id, func, waiters):
         try:
-            rest = eval(req.body) if req.body else {}
+            rest = json.loads(req.body) if req.body else {}
         except SyntaxError:
             QoSController._LOGGER.debug('invalid syntax %s', req.body)
             return Response(status=400)
-- 
1.9.1



On 2014年11月10日 13:26, FUJITA Tomonori wrote:
> On Mon, 10 Nov 2014 09:04:36 +0900
> Yusuke Iwase <iwase.yusu...@gmail.com> wrote:
> 
>> Hi
>>
>> I'm just trying to make and test the same modification.
>> Thanks.
>>
>> I think ofctl_rest.py needs to be compatible with hexadecimal value
>> or ascii byte array (e.g. "\x00\x00\x00\x01" in Experimenter)
>> in order to keep usability.
>> But json.loads can not get hexadecimal value or ascii byte array.
>>
>> I suggest to use another function, e.g. ast.literal_eval() in ofctl_rest.py.
>>
>>
>> How about this?
>> I modified your patch.
>>
>> ---------------------------
>>
>> It is not safe to use eval function because input data(request body) is not 
>> checked
>> For example, someone can send this data to remove all files in the directory
>> "import('os').system('rm -rf .')"
>>
>> I suggest to use json.loads to parse the request body if the data is json 
>> format
>> or disable builtin functions like:
>> eval(req.body, {"__builtins__":None})
>>
>> In this patch, ast.literal_eval() is used to evaluate REST body,
>> because ofctl_rest needs to be compatible with hexadecimal value
>> or ascii byte array (e.g. "\x00\x00\x00\x01" in Experimenter)
>> in order to keep usability.
>>
>> Signed-off-by: Takeshi <a86487...@gmail.com>
>> Signed-off-by: IWASE Yusuke <iwase.yusu...@gmail.com>
>> ---
>>  ryu/app/ofctl_rest.py | 13 +++++++------
>>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> Applied, thanks a lot!
> 

------------------------------------------------------------------------------
_______________________________________________
Ryu-devel mailing list
Ryu-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ryu-devel

Reply via email to