From 30aa1411e54aaba51cce5a279bbf6c7c389aaf48 Mon Sep 17 00:00:00 2001 From: florcabral Date: Wed, 24 Apr 2024 21:27:03 -0300 Subject: [PATCH 1/3] patch unsafe yaml load, add tests --- package.xml | 2 ++ scripts/dynparam | 4 ++-- test/CMakeLists.txt | 2 ++ test/test_dynparam.launch | 5 +++++ test/test_safe_yaml.py | 30 ++++++++++++++++++++++++++++++ test/test_unsafe_yaml.yaml | 1 + 6 files changed, 42 insertions(+), 2 deletions(-) create mode 100644 test/test_dynparam.launch create mode 100755 test/test_safe_yaml.py create mode 100644 test/test_unsafe_yaml.yaml diff --git a/package.xml b/package.xml index 6ec01d8..da1b82b 100644 --- a/package.xml +++ b/package.xml @@ -37,4 +37,6 @@ roslib rospy rosservice + + rosbash diff --git a/scripts/dynparam b/scripts/dynparam index 2812b98..e63012b 100755 --- a/scripts/dynparam +++ b/scripts/dynparam @@ -110,7 +110,7 @@ Examples: node = args[0] if len(args) == 2: node, value = args[0], args[1] - values_dict = yaml.load(value, Loader=yaml.Loader) + values_dict = yaml.safe_load(value) if type(values_dict) != dict: parser.error('invalid arguments. Please specify either a node name, parameter name and parameter value, or a node name and a YAML dictionary') elif len(args) == 3: @@ -159,7 +159,7 @@ def do_load(): f = open(path, 'r') try: params = {} - for doc in yaml.load_all(f.read(), Loader=yaml.Loader): + for doc in yaml.safe_load_all(f.read()): params.update(doc) finally: f.close() diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 8b8012a..0992dab 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -17,3 +17,5 @@ target_link_libraries(dynamic_reconfigure-test_client dynamic_reconfigure_config add_dependencies(tests dynamic_reconfigure-test_client) add_rostest(test_python_simple_client.launch) + +add_rostest(test_dynparam.launch) diff --git a/test/test_dynparam.launch b/test/test_dynparam.launch new file mode 100644 index 0000000..c3b54c6 --- /dev/null +++ b/test/test_dynparam.launch @@ -0,0 +1,5 @@ + + + + + diff --git a/test/test_safe_yaml.py b/test/test_safe_yaml.py new file mode 100755 index 0000000..9d10b55 --- /dev/null +++ b/test/test_safe_yaml.py @@ -0,0 +1,30 @@ +#!/usr/bin/env python + +import unittest +import rostest +import rospy +import os +import subprocess + + +class TestDynparam(unittest.TestCase): + + def test_dynparam(self): + unsafe_yaml_file = os.path.join(os.path.abspath(os.path.join(os.path.dirname(__file__))), 'test_unsafe_yaml.yaml') + retcode_unsafe_yaml = 0 + + try: + # run dynparam load with insecure yaml + output = subprocess.check_output(['rosrun dynamic_reconfigure dynparam load ref_server ' + unsafe_yaml_file], stderr=subprocess.STDOUT, shell=True, executable="/bin/bash") + except subprocess.CalledProcessError as e: + self.assertIn('could not determine a constructor for the tag', e.output) # check the right error is being thrown + retcode_unsafe_yaml = e.returncode + + # check the test failed + self.assertNotEqual(retcode_unsafe_yaml, 0) + + +if __name__ == '__main__': + rospy.init_node('dynparam_test') + rospy.sleep(3.0) + rostest.rosrun('dynamic_reconfigure', 'test_dynparam', TestDynparam) diff --git a/test/test_unsafe_yaml.yaml b/test/test_unsafe_yaml.yaml new file mode 100644 index 0000000..c41f1c7 --- /dev/null +++ b/test/test_unsafe_yaml.yaml @@ -0,0 +1 @@ +str_: !!python/object/apply:os.system ["cat /etc/passwd"] From d694ddca00ea99c23a3030aaaf76ffe809004167 Mon Sep 17 00:00:00 2001 From: florcabral Date: Wed, 24 Apr 2024 21:34:22 -0300 Subject: [PATCH 2/3] change test name --- test/test_dynparam.launch | 2 +- test/{test_safe_yaml.py => test_dynparam.py} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename test/{test_safe_yaml.py => test_dynparam.py} (100%) diff --git a/test/test_dynparam.launch b/test/test_dynparam.launch index c3b54c6..9eeecf3 100644 --- a/test/test_dynparam.launch +++ b/test/test_dynparam.launch @@ -1,5 +1,5 @@ - + diff --git a/test/test_safe_yaml.py b/test/test_dynparam.py similarity index 100% rename from test/test_safe_yaml.py rename to test/test_dynparam.py From ab4896de739755c6119e3cc29854651bfbb2efa8 Mon Sep 17 00:00:00 2001 From: Florencia <49619072+florcabral@users.noreply.github.com> Date: Thu, 25 Apr 2024 13:13:12 -0300 Subject: [PATCH 3/3] encode test output --- test/test_dynparam.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_dynparam.py b/test/test_dynparam.py index 9d10b55..f36ed72 100755 --- a/test/test_dynparam.py +++ b/test/test_dynparam.py @@ -17,7 +17,7 @@ def test_dynparam(self): # run dynparam load with insecure yaml output = subprocess.check_output(['rosrun dynamic_reconfigure dynparam load ref_server ' + unsafe_yaml_file], stderr=subprocess.STDOUT, shell=True, executable="/bin/bash") except subprocess.CalledProcessError as e: - self.assertIn('could not determine a constructor for the tag', e.output) # check the right error is being thrown + self.assertIn('could not determine a constructor for the tag'.encode(), e.output) # check the right error is being thrown retcode_unsafe_yaml = e.returncode # check the test failed