From 0cb2a1563dc9b175fbef1972a4e528e9a74e2b1a Mon Sep 17 00:00:00 2001 From: Andrew Gaul Date: Thu, 19 Jun 2014 14:20:26 -0700 Subject: [PATCH] JCLOUDS-612: Securely create temporary directories This commit addresses a potential security issue where an attacker could hijack the ScriptBuilder payload by predicting the temporary directory name. --- compute/src/test/resources/initscript_with_jetty.sh | 8 ++++---- .../org/jclouds/scriptbuilder/domain/Statements.java | 9 +++++---- .../org/jclouds/scriptbuilder/domain/StatementsTest.java | 8 ++++---- .../statements/ruby/InstallRubyGemsTest.java | 6 +++--- .../src/test/resources/test_install_rubygems.sh | 8 ++++---- .../resources/test_install_rubygems_scriptbuilder.sh | 8 ++++---- 6 files changed, 24 insertions(+), 23 deletions(-) diff --git a/compute/src/test/resources/initscript_with_jetty.sh b/compute/src/test/resources/initscript_with_jetty.sh index fca9683265..741b6d2c11 100644 --- a/compute/src/test/resources/initscript_with_jetty.sh +++ b/compute/src/test/resources/initscript_with_jetty.sh @@ -227,11 +227,11 @@ END_OF_JCLOUDS_SCRIPT installOpenJDK || return 1 iptables -I INPUT 1 -p tcp --dport 8080 -j ACCEPT iptables-save - mkdir /tmp/$$ - curl -q -s -S -L --connect-timeout 10 --max-time 600 --retry 20 -X GET http://archive.eclipse.org/jetty/8.1.8.v20121106/dist/jetty-distribution-8.1.8.v20121106.tar.gz |(mkdir -p /tmp/$$ &&cd /tmp/$$ &&tar -xpzf -) + export TAR_TEMP="$(mktemp -d)" + curl -q -s -S -L --connect-timeout 10 --max-time 600 --retry 20 -X GET http://archive.eclipse.org/jetty/8.1.8.v20121106/dist/jetty-distribution-8.1.8.v20121106.tar.gz |(mkdir -p "${TAR_TEMP}" &&cd "${TAR_TEMP}" &&tar -xpzf -) mkdir -p /usr/local/jetty - mv /tmp/$$/*/* /usr/local/jetty - rm -rf /tmp/$$ + mv "${TAR_TEMP}"/*/* /usr/local/jetty + rm -rf "${TAR_TEMP}" chown -R web /usr/local/jetty END_OF_JCLOUDS_SCRIPT diff --git a/scriptbuilder/src/main/java/org/jclouds/scriptbuilder/domain/Statements.java b/scriptbuilder/src/main/java/org/jclouds/scriptbuilder/domain/Statements.java index 365d6a36fa..6d0dcdcf27 100644 --- a/scriptbuilder/src/main/java/org/jclouds/scriptbuilder/domain/Statements.java +++ b/scriptbuilder/src/main/java/org/jclouds/scriptbuilder/domain/Statements.java @@ -189,11 +189,12 @@ public class Statements { */ public static Statement extractTargzAndFlattenIntoDirectory(URI tgz, String dest) { return new StatementList(ImmutableSet. builder() - .add(exec("mkdir /tmp/$$")) - .add(extractTargzIntoDirectory(tgz, "/tmp/$$")) + .add(exec("export TAR_TEMP=\"$(mktemp -d)\"")) + .add(extractTargzIntoDirectory(tgz, "\"${TAR_TEMP}\"")) .add(exec("mkdir -p " + dest)) - .add(exec("mv /tmp/$$/*/* " + dest)) - .add(exec("rm -rf /tmp/$$")).build()); + .add(exec("mv \"${TAR_TEMP}\"/*/* " + dest)) + .add(exec("rm -rf \"${TAR_TEMP}\"")) + .build()); } public static Statement extractTargzIntoDirectory(URI targz, String directory) { diff --git a/scriptbuilder/src/test/java/org/jclouds/scriptbuilder/domain/StatementsTest.java b/scriptbuilder/src/test/java/org/jclouds/scriptbuilder/domain/StatementsTest.java index 1ea5234c73..8bd1143615 100644 --- a/scriptbuilder/src/test/java/org/jclouds/scriptbuilder/domain/StatementsTest.java +++ b/scriptbuilder/src/test/java/org/jclouds/scriptbuilder/domain/StatementsTest.java @@ -54,11 +54,11 @@ public class StatementsTest { "/usr/local/maven"); assertEquals( save.render(OsFamily.UNIX), - "mkdir /tmp/$$\n" + - "curl -q -s -S -L --connect-timeout 10 --max-time 600 --retry 20 -X GET http://www.us.apache.org/dist/maven/binaries/apache-maven-3.0.4-bin.tar.gz |(mkdir -p /tmp/$$ &&cd /tmp/$$ &&tar -xpzf -)\n" + + "export TAR_TEMP=\"$(mktemp -d)\"\n" + + "curl -q -s -S -L --connect-timeout 10 --max-time 600 --retry 20 -X GET http://www.us.apache.org/dist/maven/binaries/apache-maven-3.0.4-bin.tar.gz |(mkdir -p \"${TAR_TEMP}\" &&cd \"${TAR_TEMP}\" &&tar -xpzf -)\n" + "mkdir -p /usr/local/maven\n" + - "mv /tmp/$$/*/* /usr/local/maven\n" + - "rm -rf /tmp/$$\n"); + "mv \"${TAR_TEMP}\"/*/* /usr/local/maven\n" + + "rm -rf \"${TAR_TEMP}\"\n"); } diff --git a/scriptbuilder/src/test/java/org/jclouds/scriptbuilder/statements/ruby/InstallRubyGemsTest.java b/scriptbuilder/src/test/java/org/jclouds/scriptbuilder/statements/ruby/InstallRubyGemsTest.java index 4733ba37ac..8efa2b12df 100644 --- a/scriptbuilder/src/test/java/org/jclouds/scriptbuilder/statements/ruby/InstallRubyGemsTest.java +++ b/scriptbuilder/src/test/java/org/jclouds/scriptbuilder/statements/ruby/InstallRubyGemsTest.java @@ -87,10 +87,10 @@ public class InstallRubyGemsTest { private static String installRubyGems(String version) { String script = "if ! hash gem 2>/dev/null; then\n" + "(\n" - + "mkdir /tmp/$$\n" + + "export TAR_TEMP=\"$(mktemp -d)\"\n" + "curl -q -s -S -L --connect-timeout 10 --max-time 600 --retry 20 -X GET http://production.cf.rubygems.org/rubygems/rubygems-" - + version + ".tgz |(mkdir -p /tmp/$$ &&cd /tmp/$$ &&tar -xpzf -)\n" + "mkdir -p /tmp/rubygems\n" - + "mv /tmp/$$/*/* /tmp/rubygems\n" + "rm -rf /tmp/$$\n" + "cd /tmp/rubygems\n" + + version + ".tgz |(mkdir -p \"${TAR_TEMP}\" &&cd \"${TAR_TEMP}\" &&tar -xpzf -)\n" + "mkdir -p /tmp/rubygems\n" + + "mv \"${TAR_TEMP}\"/*/* /tmp/rubygems\n" + "rm -rf \"${TAR_TEMP}\"\n" + "cd /tmp/rubygems\n" + "ruby setup.rb --no-format-executable\n" // + "rm -fr /tmp/rubygems\n" + // ")\n" + // diff --git a/scriptbuilder/src/test/resources/test_install_rubygems.sh b/scriptbuilder/src/test/resources/test_install_rubygems.sh index c9363d24cc..169250c532 100644 --- a/scriptbuilder/src/test/resources/test_install_rubygems.sh +++ b/scriptbuilder/src/test/resources/test_install_rubygems.sh @@ -1,10 +1,10 @@ if ! hash gem 2>/dev/null; then ( -mkdir /tmp/$$ -curl -q -s -S -L --connect-timeout 10 --max-time 600 --retry 20 -X GET http://production.cf.rubygems.org/rubygems/rubygems-1.8.10.tgz |(mkdir -p /tmp/$$ &&cd /tmp/$$ &&tar -xpzf -) +export TAR_TEMP="$(mktemp -d)" +curl -q -s -S -L --connect-timeout 10 --max-time 600 --retry 20 -X GET http://production.cf.rubygems.org/rubygems/rubygems-1.8.10.tgz |(mkdir -p "${TAR_TEMP}" &&cd "${TAR_TEMP}" &&tar -xpzf -) mkdir -p /tmp/rubygems -mv /tmp/$$/*/* /tmp/rubygems -rm -rf /tmp/$$ +mv "${TAR_TEMP}"/*/* /tmp/rubygems +rm -rf "${TAR_TEMP}" cd /tmp/rubygems ruby setup.rb --no-format-executable rm -fr /tmp/rubygems diff --git a/scriptbuilder/src/test/resources/test_install_rubygems_scriptbuilder.sh b/scriptbuilder/src/test/resources/test_install_rubygems_scriptbuilder.sh index 1c4bb5f8c5..3df852ede0 100644 --- a/scriptbuilder/src/test/resources/test_install_rubygems_scriptbuilder.sh +++ b/scriptbuilder/src/test/resources/test_install_rubygems_scriptbuilder.sh @@ -86,11 +86,11 @@ END_OF_JCLOUDS_SCRIPT trap 'echo $?>$INSTANCE_HOME/rc' 0 1 2 3 15 if ! hash gem 2>/dev/null; then ( - mkdir /tmp/$$ - curl -q -s -S -L --connect-timeout 10 --max-time 600 --retry 20 -X GET http://production.cf.rubygems.org/rubygems/rubygems-1.8.10.tgz |(mkdir -p /tmp/$$ &&cd /tmp/$$ &&tar -xpzf -) + export TAR_TEMP="$(mktemp -d)" + curl -q -s -S -L --connect-timeout 10 --max-time 600 --retry 20 -X GET http://production.cf.rubygems.org/rubygems/rubygems-1.8.10.tgz |(mkdir -p "${TAR_TEMP}" &&cd "${TAR_TEMP}" &&tar -xpzf -) mkdir -p /tmp/rubygems - mv /tmp/$$/*/* /tmp/rubygems - rm -rf /tmp/$$ + mv "${TAR_TEMP}"/*/* /tmp/rubygems + rm -rf "${TAR_TEMP}" cd /tmp/rubygems ruby setup.rb --no-format-executable rm -fr /tmp/rubygems