Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix parsing yaml spec files containing + as value #112

Merged
merged 1 commit into from
Jun 17, 2024
Merged

Fix parsing yaml spec files containing + as value #112

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion picofun/spec.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,20 @@ def parse(self, content: str) -> dict:
class YAMLSpecParser(SpecParser):
"""Parses an OpenAPI spec file in YAML format."""

def patch_loader(self, loader: yaml.SafeLoader) -> yaml.SafeLoader:
"""
Patch the loader to allow = as a value.

Handles being call multiple times. It only patches the loader on the first call.

See https://github.com/yaml/pyyaml/issues/89 for more info.

:return: The patched YAML loader.
"""
if "=" in loader.yaml_implicit_resolvers:
loader.yaml_implicit_resolvers.pop("=")
return loader

def parse(self, content: str) -> dict:
"""
Parse the contents of the spec file.
Expand All @@ -108,8 +122,12 @@ def parse(self, content: str) -> dict:
:raises InvalidSpecError: If the spec file is not valid YAML.
:return: The contents of the spec file as a dict.
"""
loader = yaml.SafeLoader
patched_loader = self.patch_loader(loader)
try:
return yaml.safe_load(content)
return yaml.load(
Copy link

Choose a reason for hiding this comment

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

Nullify Code Language: Python 🟡 HIGH Severity CWE-20

Yaml load

Use of unsafe yaml load. Allows instantiation of arbitrary objects. Consider yaml.safe_load().
Read more:
https://cwe.mitre.org/data/definitions/20.html

⚡ Here's how you might fix this potential vulnerability

The vulnerability is due to the use of yaml.load, which can execute arbitrary code if the YAML content is malicious. The fix replaces yaml.load with yaml.safe_load, which safely parses the YAML without executing arbitrary code. This change ensures that the application only parses data structures from the YAML and does not execute any embedded code, mitigating the risk of arbitrary code execution.

Please note that AI auto-fixes are currently experimental

Suggested change
return yaml.load(
return yaml.safe_load(content)

Powered by nullify.ai

Reply with /nullify to interact with me like another developer
(you will need to refresh the page for updates)

content, Loader=patched_loader # noqa: S506 Uses a patched safe loader
)
except yaml.YAMLError as e:
raise picofun.errors.InvalidSpecError() from e

Expand Down
6 changes: 6 additions & 0 deletions tests/test_spec.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,12 @@ def test_yaml_spec_parser_invalid_yaml() -> None:
parser.parse("foo: b: a: r")


def test_yaml_spec_parser_value_equals() -> None:
"""Test the YAMLSpecParser class with a plus symbol as the value."""
parser = picofun.spec.YAMLSpecParser()
assert parser.parse("key: +") == {"key": "+"}


def test_spec_json() -> None:
"""Test the Spec class with JSON."""
spec = picofun.spec.Spec("tests/data/petstore.json")
Expand Down
Loading