Custom tmpdir arg (first try) #8
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Attempt to add a --tmpdir arg. The --shared option can be very useful but also a little dangerous if you don’t trust what’s in the /tmp directory. Giving Arx a custom location for the temp directory should make things a little bit safer. Most useful in this case, $ arx tmpx --shared --tmpdir '$HOME/.cache' my.tar.bz2 This will put things in your HOME directory where you can be fairly sure no one else has snuck in a temporary directory to fool you into using something that is malicious. Using /tmp as a shared directory can be exploited if you know the hash ahead of time. Just something as simple as this, $ tmp=$(mktemp -d tmpx-HASH) $ echo "rm -rf /" > $tmp/run where HASH is the hash of the dat, can create a privilege escalation when Arx runs the /run script.
solidsnack
requested changes
May 3, 2018
model-scripts/tmpx.sh
Outdated
| else | ||
| dir=/tmp/tmpx-"$token" | ||
| dir=$tmpdir/tmpx-"$token" |
There was a problem hiding this comment.
$tmpdir -> "$tmpdir" for shell quoting safety.
model-scripts/tmpx.sh
Outdated
| @@ -39,9 +39,9 @@ opts() { | |||
| if $shared | |||
| then | |||
| rm_=false | |||
| dir=/tmp/tmpx-"$hash" | |||
| dir=$tmpdir/tmpx-"$hash" | |||
There was a problem hiding this comment.
$tmpdir -> "$tmpdir" for shell quoting safety.
System/Posix/ARX/TMPXTools.hs
Outdated
| @@ -22,12 +22,13 @@ import Data.Hashable | |||
| data Template = Template { rm0 :: Bool, {-^ Remove tmp on run success? -} | |||
| rm1 :: Bool, {-^ Remove tmp on run error? -} | |||
| shared :: Bool, {-^ Share directory across runs? -} | |||
| tmpdir :: String, {-^ Location to store tmp files.-} | |||
There was a problem hiding this comment.
Maybe Location for tmp files. would allow a little more space.
System/Posix/ARX/CLI/Options.hs
Outdated
| @@ -98,6 +101,8 @@ qPath = tokCL QualifiedPath | |||
| shared :: ArgsParser Bool | |||
| shared = True <$ arg "--shared" | |||
|
|
|||
| tmpdir :: ArgsParser Path | |||
There was a problem hiding this comment.
␣::␣ArgsParser Path -> ::␣␣ArgsParser Path for consistency with surrounding code.
System/Posix/ARX/CLI/Options.hs
Outdated
| @@ -98,6 +101,8 @@ qPath = tokCL QualifiedPath | |||
| shared :: ArgsParser Bool | |||
| shared = True <$ arg "--shared" | |||
|
|
|||
| tmpdir :: ArgsParser Path | |||
| = arg "--tmpdir" >> QualifiedPath | |||
There was a problem hiding this comment.
=␣arg "--tmpdir" >> QualifiedPath -> =␣␣arg "--tmpdir" >> QualifiedPath for consistency with surrounding code.
Fixes review issues & more.
solidsnack
approved these changes
May 21, 2018
|
I will rebuild and re-release this week. Thanks for your patch. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Attempt to add a --tmpdir arg.
The --shared option can be very useful but also a little dangerous if
you don’t trust what’s in the /tmp directory. Giving Arx a custom
location for the temp directory should make things a little bit safer.
Most useful in this case,
$ arx tmpx --shared --tmpdir '$HOME/.cache' my.tar.bz2
This will put things in your HOME directory where you can be fairly
sure no one else has snuck in a temporary directory to fool you into
using something that is malicious.
Using /tmp as a shared directory can be exploited if you know the hash
ahead of time. Just something as simple as this,
$ tmp=$(mktemp -d tmpx-HASH)
$ echo "rm -rf /" > $tmp/run
where HASH is the hash of the dat,
can create a privilege escalation when Arx runs the /run script.
This is already (hackily) done by patching tmpx.sh in nix-bundle, but I would prefer to get it working as an are.
https://github.com/matthewbauer/nix-bundle/blob/master/default.nix#L8-L9