Merge pull request #122 from launchdarkly/eb/ch58025/yaml-load

use yaml.safe_load() to avoid code execution vulnerability in file data source

Author: eli-darkly

ldclient/impl/integrations/files/file_data_source.py

@@ -80,7 +80,7 @@ def _load_file(self, path, all_data):
 
     def _parse_content(self, content):
         if have_yaml:
-            return yaml.load(content)  # pyyaml correctly parses JSON too
+            return yaml.safe_load(content)  # pyyaml correctly parses JSON too
         return json.loads(content)
 
     def _add_item(self, all_data, kind, item):

testing/test_file_data_source.py

@@ -246,3 +246,28 @@ def test_evaluates_simplified_flag_with_client_as_expected():
         os.remove(path)
         if client is not None:
             client.close()
+
+unsafe_yaml_caused_method_to_be_called = False
+
+def arbitrary_method_called_from_yaml(x):
+    global unsafe_yaml_caused_method_to_be_called
+    unsafe_yaml_caused_method_to_be_called = True
+
+def test_does_not_allow_unsafe_yaml():
+    if not have_yaml:
+        pytest.skip("skipping file source test with YAML because pyyaml isn't available")
+
+    # This extended syntax defined by pyyaml allows arbitrary code execution. We should be using
+    # yaml.safe_load() which does not support such things.
+    unsafe_yaml = '''
+!!python/object/apply:testing.test_file_data_source.arbitrary_method_called_from_yaml ["hi"]
+'''
+    path = make_temp_file(unsafe_yaml)
+    try:
+        factory = Files.new_data_source(paths = path)
+        client = LDClient(config=Config(update_processor_class = factory, send_events = False))
+    finally:
+        os.remove(path)
+        if client is not None:
+            client.close()
+    assert unsafe_yaml_caused_method_to_be_called == False