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

Add more negative patterns to exclude cases of reading from a file for Yaml bad deserialization #3296

Merged
merged 3 commits into from
Feb 21, 2024

Conversation

navhits
Copy link
Contributor

@navhits navhits commented Jan 31, 2024

For cases where YAML.load(File.read(...)) was used, the rule flagged these as potential issues. However, when the same was written as,

filename = File.read(...)
obj = YAML.load(filename)

It was allowed. Either it is a problem with how the parser works, or the rule needs an additional negative pattern.

Likewise, the same is true for cases like YAML.load($MOD.$METHOD(File.read(...))). Here, the file could be passed to a template renderer like ERB, which, in this case, should be fine.

template = ERB.new(File.read("test.yml"))
obj = YAML::load(template.result)

These 2 FP conditions are covered. Also, File.read takes a file name as a string. It may not be explicitly needed to make the pattern expect a string.

@CLAassistant
Copy link

CLAassistant commented Jan 31, 2024

CLA assistant check
All committers have signed the CLA.

@navhits
Copy link
Contributor Author

navhits commented Feb 7, 2024

@0xDC0DE would you please review this? I don't have the request review option.

Copy link
Contributor

@0xDC0DE 0xDC0DE left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks for contributing, Naveen!

@navhits navhits requested a review from 0xDC0DE February 19, 2024 12:19
@navhits
Copy link
Contributor Author

navhits commented Feb 19, 2024

I missed updating the test file for autofix test case to pass. Added that now.

@0xDC0DE 0xDC0DE merged commit 3abb4d5 into semgrep:develop Feb 21, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants