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

Insecure /tmp usage with static path (use mktemp instead or not shared /tmp) #12

Open
emanuelb opened this issue Apr 6, 2021 · 2 comments

Comments

@emanuelb
Copy link

emanuelb commented Apr 6, 2021

Files / Folders are used in /tmp with static/predictable names, which is bad practice and in general as it's vulnerable to malicious usage from other users on the system (it's worth to fix even if it's not problematic in the specific case as it's still bad code)
examples are: /tmp/bwt.tar.gz /tmp/bitcoin.tar.gz

eznode/bwt/install

Lines 12 to 18 in 392290b

# Install bwt
distname=bwt-$BWT_VERSION-$BWT_ARCH
wget -qO /tmp/bwt.tar.gz https://github.com/bwt-dev/bwt/releases/download/v$BWT_VERSION/$distname.tar.gz
echo "$BWT_SHA256 /tmp/bwt.tar.gz" | sha256sum -c -
tar xzf /tmp/bwt.tar.gz -C /tmp
mv /tmp/$distname/bwt /usr/local/bin

wget -qO /tmp/bitcoin.tar.gz https://bitcoincore.org/bin/bitcoin-core-$BITCOIND_VERSION/bitcoin-$BITCOIND_VERSION-$BITCOIND_ARCH.tar.gz --show-progress --progress=bar:force
echo "$BITCOIND_SHA256 /tmp/bitcoin.tar.gz" | sha256sum -c -
tar xzf /tmp/bitcoin.tar.gz -C /tmp
mv /tmp/bitcoin-$BITCOIND_VERSION/bin/bitcoin{d,-cli} /usr/local/bin/

Using mktemp to generate the tmp file is better, also creating a temporary directory with mktemp -d and working with static names in it is ok (or creating temp dir for each usage by mktemp -d --suffix='-some-related-suffix'), also not using shared /tmp/ but another location like /home/user/tmp/ is also a fix.
There more /tmp/ usage instances in this codebase which should be fixed.

@shesek
Copy link
Contributor

shesek commented Apr 14, 2021

What are the disadvantages you see for this, given that this is running in an isolated predictable environment and there's no chance of naming collisions?

@emanuelb
Copy link
Author

  1. it's bad practice + bad code.
  2. code reuse of this pattern may result in worse outcome (such as someone using this code as base for installation on bare linux not in container, etc..)
  3. there's might be small chance of collisions either by coincidence or attack to pass security boundary (other users / processes with different MAC/sandboxes [selinux/apparmor] profiles) for this kind of tmp usage.

it's better to fix it as explained above (mktemp usage).

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

No branches or pull requests

2 participants