Skip to content

Fix #59935: pillar_roots.write on pillar subdirs #59944

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

Merged
merged 1 commit into from
Oct 20, 2021

Conversation

niflostancu
Copy link
Contributor

@niflostancu niflostancu commented Apr 1, 2021

What does this PR do?

Fixes the pillar_roots.write's call to salt.utils.verify.clean_path to account for subdirectory writes.

What issues does this PR fix or reference?

Fixes: #59935

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

Commits signed with GPG?

No

@niflostancu niflostancu requested a review from a team as a code owner April 1, 2021 09:55
@niflostancu niflostancu requested review from twangboy and removed request for a team April 1, 2021 09:55
twangboy
twangboy previously approved these changes Apr 1, 2021
@twangboy twangboy added the Silicon v3004.0 Release code name label Apr 1, 2021
Copy link
Contributor

@Ch3LL Ch3LL left a comment

Choose a reason for hiding this comment

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

This will require a changelog and would you please add a test similar to this: aafc5ed#diff-e3e8844bfd715f8094e0c7e1d4294e6d638132fd46e14786ff2ab948e7b48511R34 but testing with a subdir. For example: sub/../root_dir/.

@Ch3LL Ch3LL added Phosphorus v3005.0 Release code name and version and removed Silicon v3004.0 Release code name labels Sep 22, 2021
@Ch3LL
Copy link
Contributor

Ch3LL commented Sep 30, 2021

@niflostancu bump, did you see my change requests?

@niflostancu
Copy link
Contributor Author

Oh god, sorry, I completely forgot about this (I've been busy with other projects) :(
I will fix them tomorrow ;)

@niflostancu
Copy link
Contributor Author

Hello, I added the supplementary testcase (testing for subdir/../../ that traverses outside the pillar root) + changelog.

My fix simply calls clean_path with subdir=True argument for the pillar_roots to be able to write inside a subdirectory.
I see that it has its own unit tests (tests/unit/utils/test_verify.py) which can be further augmented with path traversal checks (which would benefit any modules using it).

@Ch3LL Ch3LL requested a review from dwoz October 8, 2021 15:25
@Ch3LL Ch3LL merged commit da38195 into saltstack:master Oct 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Phosphorus v3005.0 Release code name and version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] v3002.6 After CVE-2021-25282 fix, pillar_roots.write cannot write to subdirs.
4 participants