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

GoogleFriendlyConfig.read: Use YAML.safe_load to avoid arbitrary Ruby class construction #295

Merged
merged 1 commit into from
Jun 1, 2018

Conversation

cben
Copy link
Contributor

@cben cben commented May 31, 2018

Same as ManageIQ/kubeclient#334, you have a copy-paste of that code since #88.

I don't think there is any security impact in this project, like most uses of Config.read out there it's a command-line tool, config(s) come from $KUBECONFIG env var, so user that runs this controls the config but can only "attack themself".

P.S. ManageIQ/kubeclient#213 finally landed in kubeclient 3.1, you could in principle drop GoogleFriendlyConfig in favor of that — though it doesn't autodetect auth-provider 'gcp', you'd need to know when to call it.

P.S. you have some unrelated YAML.load uses, consider checking which can be safe_load.

Cheers

@dturn
Copy link
Contributor

dturn commented May 31, 2018

@klautcomputing This looks like a good one for you to review.

Copy link
Contributor

@klautcomputing klautcomputing left a comment

Choose a reason for hiding this comment

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

@cben Thank you ❤️

Copy link
Contributor

@dturn dturn left a comment

Choose a reason for hiding this comment

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

lgtm

@dturn dturn merged commit eed1403 into Shopify:master Jun 1, 2018
@dturn
Copy link
Contributor

dturn commented Jun 1, 2018

@cben Thanks for the contribution!

@KnVerey KnVerey mentioned this pull request Jun 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants