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

Jump to bottom

ansible-runner writes artifacts to world rw location by default #738

Closed
cidrblock opened this issue Jun 24, 2021 · 2 comments · Fixed by #742
Closed

ansible-runner writes artifacts to world rw location by default #738

cidrblock opened this issue Jun 24, 2021 · 2 comments · Fixed by #742
Labels
priority:high

Comments

@cidrblock
Copy link
Contributor

related to: #722
@cidrblock cidrblock changed the title ansible-runner writes artifiacts to world rw location by default ansible-runner writes artifacts to world rw location by default Jun 24, 2021
@matburt
Copy link
Member

matburt commented Jun 25, 2021

I should have caught this beforehand, that whole first line probably just needs to be replaced with https://docs.python.org/3/library/tempfile.html#tempfile.mkdtemp

@cidrblock
Copy link
Contributor Author

yeah, and with a prefix for attribution

abadger added a commit to abadger/ansible-runner that referenced this issue Jun 28, 2021
@abadger
When creating a private_data_dir in a world writable location, use mk…
cd6b999 
…dtemp()

The previous code allowed for an easily exploitable race where the
attacker could pre-create a known directory name to gain access to
private files created by ansible-runner.  Using mkdtemp() closes that
hole because mkdtemp() ensures that the directory name returned has
been allocated with restrictive permissions by mkdtemp itself so the
attacker has no opportunity to inject their own directory.

Fixes ansible#738
@abadger abadger mentioned this issue Jun 28, 2021
Merged
abadger added a commit to abadger/ansible-runner that referenced this issue Jun 28, 2021
@abadger
When creating a private_data_dir in a world writable location, use mk…
60b059f 
…dtemp()

The previous code allowed for an easily exploitable race where the
attacker could pre-create a known directory name to gain access to
private files created by ansible-runner.  Using mkdtemp() closes that
hole because mkdtemp() ensures that the directory name returned has
been allocated with restrictive permissions by mkdtemp itself so the
attacker has no opportunity to inject their own directory.

Fixes ansible#738
ansible-zuul bot added a commit that referenced this issue Jun 29, 2021
@ansible-zuul
Merge pull request #742 from abadger/secure-tempfiles
dcdb62d 
Secure tempfiles

Looking through the code, I found two separate race conditions with tempfile handling which can be exploited.  One of those is mentioned in #738. The other one happens because we create a tempdir and then delete it, then later reuse its name. An attacker can pre-create the tempdir in between the deletion and the second creation with the reused name.  This PR attempts to fix both.
Fixes #738

Reviewed-by: Shane McDonald <me@shanemcd.com>
Reviewed-by: Matthew Jones <bsdmatburt@gmail.com>
atbore-phx pushed a commit to atbore-phx/ansible-runner that referenced this issue Jun 29, 2021
@abadger @atbore-phx
When creating a private_data_dir in a world writable location, use mk…
ad79747 
…dtemp()

The previous code allowed for an easily exploitable race where the
attacker could pre-create a known directory name to gain access to
private files created by ansible-runner.  Using mkdtemp() closes that
hole because mkdtemp() ensures that the directory name returned has
been allocated with restrictive permissions by mkdtemp itself so the
attacker has no opportunity to inject their own directory.

Fixes ansible#738
atbore-phx pushed a commit to atbore-phx/ansible-runner that referenced this issue Jun 29, 2021
@abadger @atbore-phx
When creating a private_data_dir in a world writable location, use mk…
77ec511 
…dtemp()

The previous code allowed for an easily exploitable race where the
attacker could pre-create a known directory name to gain access to
private files created by ansible-runner.  Using mkdtemp() closes that
hole because mkdtemp() ensures that the directory name returned has
been allocated with restrictive permissions by mkdtemp itself so the
attacker has no opportunity to inject their own directory.

Fixes ansible#738
shanemcd pushed a commit to shanemcd/ansible-runner that referenced this issue Jul 2, 2021
@abadger @shanemcd
When creating a private_data_dir in a world writable location, use mk…
11eb1f7 
…dtemp()

The previous code allowed for an easily exploitable race where the
attacker could pre-create a known directory name to gain access to
private files created by ansible-runner.  Using mkdtemp() closes that
hole because mkdtemp() ensures that the directory name returned has
been allocated with restrictive permissions by mkdtemp itself so the
attacker has no opportunity to inject their own directory.

Fixes ansible#738
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
priority:high
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Secure tempfiles abadger/ansible-runner
3 participants
@matburt @shanemcd @cidrblock