diff --git a/ldclient/impl/integrations/files/file_data_source.py b/ldclient/impl/integrations/files/file_data_source.py index 785a3851..9f9f3eaf 100644 --- a/ldclient/impl/integrations/files/file_data_source.py +++ b/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): diff --git a/testing/test_file_data_source.py b/testing/test_file_data_source.py index 78ab5359..7b13cf9b 100644 --- a/testing/test_file_data_source.py +++ b/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