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.

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

[SECURITY] Change the default to be secure by default #11

Closed
JLLeitschuh opened this issue Jan 5, 2023 · 5 comments · Fixed by #40
Closed

[SECURITY] Change the default to be secure by default #11

JLLeitschuh opened this issue Jan 5, 2023 · 5 comments · Fixed by #40

Comments

@JLLeitschuh
Copy link

Hi!

Security researcher here looking into YAML parsers across the industry. Why is it that this YAML parser is insecure-by-default? Why not make it secure-by-default and then allow end-users to opt-in to the insecure variant similar to how the ruby standard library handles it currently.

@hsbt
Copy link
Member

hsbt commented Jan 5, 2023

?

I didn't understand you say "Why is it that this YAML parser is insecure-by-default?". Please explanation it with Ruby's yaml parser.

@JLLeitschuh
Copy link
Author

The underlying parser for this gem is using the unsafe_load method by default, which has arbitrary deserialization vulnerabilities.

yaml/lib/yaml/store.rb

Lines 67 to 74 in a03d840

def load(content)
table = YAML.unsafe_load(content)
if table == false
{}
else
table
end
end

There is also the line in the README:

Do not use YAML to load untrusted data. Doing so is unsafe and could allow malicious input to execute arbitrary code inside your application.

The YAML standard library also has a safe_load method, why not use that by-default instead of the insecure variant?

@hsbt
Copy link
Member

hsbt commented Jan 9, 2023

It's only used by YAML::Store like PStore usage, not the default YAML parser. And There is no reason why it still use unsafe_load. I welcome to your pull-request.

@JLLeitschuh
Copy link
Author

Pull request contributed

@hsbt
Copy link
Member

hsbt commented Oct 16, 2024

I fixed this at #40 and #41

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants