Skip to content

Conversation

ashwini-orchestral
Copy link
Contributor

It is not safe to call yaml.load with any data received from an untrusted source. It allows instantiation of arbitrary objects. The function yaml.safe_load limits this ability to simple Python objects like integers or lists. For security reasons, yaml.safe_load is used.

@pull-request-size pull-request-size bot added the size/L PR that changes 100-499 lines. Requires some effort to review. label Feb 19, 2021
Copy link
Contributor

@m4dcoder m4dcoder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ashwini-orchestral Looks like you included changes from another PR/commit into this PR. It is hard to tell what changes relate to this PR. Can you clean this up? You should branch from master and then apply the changes.

@@ -77,7 +77,7 @@ def load_spec(module_name, spec_file, allow_duplicate_keys=False):

# 1. Check for duplicate keys
if not allow_duplicate_keys:
yaml.load(spec_string, UniqueKeyLoader)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is only used to load openapi.yaml.j2 file which is controlled by us (developers), which means there should be no chance for "yaml type injection", but I do agree and still think it's better and safer to use yaml.safe_load.

@Kami
Copy link
Member

Kami commented Feb 22, 2021

@ashwini-orchestral As @m4dcoder said, please remove unrelated changes so we can review yaml.safe_load change in isolation.

@Kami
Copy link
Member

Kami commented Feb 22, 2021

Superseded by #5162.

@Kami Kami closed this Feb 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/L PR that changes 100-499 lines. Requires some effort to review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants