From 17b413faee2367520b949ca98f9d084c546dcebd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Georges=20Dup=C3=A9ron?= Date: Mon, 27 May 2019 22:00:09 +0200 Subject: [PATCH] Security aspects for shell scripts (well, an attempt at that) --- scripts/build_docker_image.sh | 5 +- scripts/install_ligo_with_dependencies.sh | 5 +- scripts/install_native_dependencies.sh | 5 +- scripts/installer.sh | 116 +++++++++++++++++----- scripts/ligo.sh | 7 +- scripts/setup_ligo_opam_repository.sh | 5 +- 6 files changed, 112 insertions(+), 31 deletions(-) diff --git a/scripts/build_docker_image.sh b/scripts/build_docker_image.sh index eb2bdb611..8a84fc2f6 100755 --- a/scripts/build_docker_image.sh +++ b/scripts/build_docker_image.sh @@ -1 +1,4 @@ -docker build -t ligolang/ligo -f docker/Dockerfile . \ No newline at end of file +#!/bin/bash +set -euET -o pipefail + +docker build -t ligolang/ligo -f docker/Dockerfile . diff --git a/scripts/install_ligo_with_dependencies.sh b/scripts/install_ligo_with_dependencies.sh index 9ad969f3f..0fbbc166b 100755 --- a/scripts/install_ligo_with_dependencies.sh +++ b/scripts/install_ligo_with_dependencies.sh @@ -1 +1,4 @@ -cd src && opam install . --yes \ No newline at end of file +#!/bin/bash +set -euET -o pipefail + +cd src && opam install . --yes diff --git a/scripts/install_native_dependencies.sh b/scripts/install_native_dependencies.sh index 04d4ce17f..0797f0300 100755 --- a/scripts/install_native_dependencies.sh +++ b/scripts/install_native_dependencies.sh @@ -1,7 +1,10 @@ +#!/bin/bash +set -euET -o pipefail + apt-get -y install \ libev-dev \ perl \ pkg-config \ libgmp-dev \ libhidapi-dev \ - m4 \ No newline at end of file + m4 diff --git a/scripts/installer.sh b/scripts/installer.sh index 7486f5a0f..7da6107c8 100755 --- a/scripts/installer.sh +++ b/scripts/installer.sh @@ -1,31 +1,95 @@ #!/bin/bash +set -euET -o pipefail + # You can run this installer like this: # curl https://gitlab.com/ligolang/ligo/blob/master/scripts/installer.sh | bash -# Make sure the marigold/ligo image is published at docker hub first -set -euET -o pipefail -version=$1 -printf "\nInstalling LIGO ($version)\n\n" +# Make sure the marigold/ligo image is published at docker hub first -if [ $version = "next" ] - then - # Install the ligo.sh from master - wget https://gitlab.com/ligolang/ligo/raw/dev/scripts/ligo.sh - else - # Install the ligo.sh from master - wget https://gitlab.com/ligolang/ligo/raw/master/scripts/ligo.sh +if test $# -ne 1; then + printf 'Usage: installer.sh VERSION'\\n + printf ' where VERSION can be "next" or a version number'\\n + exit 1 +else + version=$1 + printf \\n'Installing LIGO ($version)'\\n\\n + + if [ $version = "next" ] + then + # Install the ligo.sh from master + url=https://gitlab.com/ligolang/ligo/raw/dev/scripts/ligo.sh + else + # Install the ligo.sh from master + url=https://gitlab.com/ligolang/ligo/raw/master/scripts/ligo.sh + fi + + # Pull the docker image used by ligo.sh + docker pull "ligolang/ligo:$version" + + # Install ligo.sh + # Rationale behind this part of the script: + # * mv is one of the few commands which is atomic + # * therefore we will create a file with the desired contents, and if that works, atomically mv it. + # If something goes wrong it will attempt to remove the temporary file + # (if removing the temporary file fails it's not a big deal due to the fairly explicit file name, + # the fact that it is hidden, and its small size) + # * most utilities (e.g. touch) don't explicitly state that they support umask in their man page + # * therefore we try to set the mode for the temporary file with an umask + do a chmod just to be sure + # * this leaves open a race condition where: + # 0) umask isn't applied by touch (e.g. the file already exists) + # 1) for some reason touch creates an executable file (e.g. the file already exists) + # 2) a user grabs the file while it is executable, and triggers its execution (the process is created but execution of the script doesn't start yet) + # 3) chmod makes it non-executable + # 4) the file is partially written + # 5) the execution actually starts, and executes a prefix of the desired command, and that prefix is usable for adverse effects + # To mitigate this, we wrap the command in the script with + # if true; then the_command; fi + # That way, the shell will raise an error due to a missing "fi" if the script executed while it is partially written + # * This still leaves open the same race condition where a propper prefix of #!/bin/sh\nif can be used to adverse effect, but there's not much we can do about this. + # * after the file is completely written, we make it executable + # * we then check for the cases where `mv` misbehaves + # * we then atomically move it to (hopefully) its destination + # * the main risks here are if /usr/local/bin/ is writable by hostile users on the same machine (then there are bigger problems than what is our concern) + # or if root itself tries to create a race condition (then there are bigger problems than what is our concern) + + # It's hard to place comments inside a sequence of commands, so here are the comments for the following code: + # wget download to stdout + # | sudo become root (sudo) for the rest of the commands + # ( subshell (to clean up temporary file if anything goes wrong) + # remove temporary file in case it already exists + # && create temporary file with (hopefully) the right permissions + # && fix permisisons in case the creation didn't take umask into account + # && redirect the output of the wget download to the temporary file + # ) || clean up temporary file if any command in the previous block failed + + wget "$url" -O - \ + | sudo sh -c ' \ + ( \ + rm -f /usr/local/bin/.temp.ligo.before-atomic-move \ + && (umask 0600 > /dev/null 2>&1; UMASK=0600 touch /usr/local/bin/.temp.ligo.before-atomic-move) \ + && chmod 0600 /usr/local/bin/.temp.ligo.before-atomic-move \ + && cat > /usr/local/bin/.temp.ligo.before-atomic-move \ + ) || rm /usr/local/bin/.temp.ligo.before-atomic-move' + + # sudo become root (sudo) for the rest of the commands + # ( subshell (to clean up temporary file if anything goes wrong) + # && check that the download seems complete (one can't rely on sigpipe & failures to correctly stop the sudo session in case the download fails) + # && overwite LIGO version in the executable + # && now that the temporary file is complete, make it executable + # && if check for some corner cases: destination exists and is a directory + # elif check for some corner cases: destination exists and is symbolic link + # else atomically (hopefully) move temporary file to its destination + # ) || clean up temporary file if any command in the previous block failed + + sudo sh -c ' \ + ( \ + && grep "END OF DOWNLOADED FILE" /usr/local/bin/.temp.ligo.before-atomic-move \ + && sed -i '' "s/latest/$version/g" ligo.sh \ + && chmod 0755 /usr/local/bin/.temp.ligo.before-atomic-move \ + && if test -d /usr/local/bin/ligo; then printf "/usr/local/bin/ligo already exists and is a directory, cancelling installation"'\\\\'n; rm /usr/local/bin/.temp.ligo.before-atomic-move; \ + elif test -L /usr/local/bin/ligo; then printf "/usr/local/bin/ligo already exists and is a symbolic link, cancelling installation"'\\\\'n; rm /usr/local/bin/.temp.ligo.before-atomic-move; \ + else mv -i /usr/local/bin/.temp.ligo.before-atomic-move /usr/local/bin/ligo; fi \ + ) || rm /usr/local/bin/.temp.ligo.before-atomic-move' + + # Installation finished, try running 'ligo' from your CLI + printf \\n'Installation successful, try to run '\''ligo --help'\'' now.'\\n fi - - -# Overwrite LIGO version in the executable -sed -i '' "s/latest/$version/g" ligo.sh - -# Copy the exucutable to the appropriate directory -sudo cp ligo.sh /usr/local/bin/ligo -sudo chmod +x /usr/local/bin/ligo -rm ligo.sh - -# Pull the docker image used by ligo.sh -docker pull "ligolang/ligo:$version" - -# Installation finished, try running 'ligo' from your CLI -printf "\nInstallation successful, try to run 'ligo --help' now.\n" \ No newline at end of file diff --git a/scripts/ligo.sh b/scripts/ligo.sh index 8ccadad8e..c68ed3c34 100755 --- a/scripts/ligo.sh +++ b/scripts/ligo.sh @@ -1,2 +1,7 @@ #!/bin/bash -docker run -it -v "$PWD":"$PWD" -w "$PWD" ligolang/ligo:latest "$@" \ No newline at end of file +if true; then + set -euET -o pipefail + docker run -it -v "$PWD":"$PWD" -w "$PWD" ligolang/ligo:latest "$@" +fi +# Do not remove the next line. It is used as an approximate witness that the download of this file was complete. This string should not appear anywhere else in the file. +# END OF DOWNLOADED FILE diff --git a/scripts/setup_ligo_opam_repository.sh b/scripts/setup_ligo_opam_repository.sh index e07eac487..6051930d5 100755 --- a/scripts/setup_ligo_opam_repository.sh +++ b/scripts/setup_ligo_opam_repository.sh @@ -1,3 +1,6 @@ +#!/bin/bash +set -euET -o pipefail + vendors/opam-repository-tools/rewrite-local-opam-repository.sh opam repo add ligo-opam-repository ./vendors/ligo-opam-repository-local-generated -opam update ligo-opam-repository \ No newline at end of file +opam update ligo-opam-repository