Openwall GNU/*/Linux - a small security-enhanced Linux distro for servers
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 23 Jun 2014 16:17:48 -0700
From: Andrew Gaul <gaul@...che.org>
To: oss-security@...ts.openwall.com, Kurt Seifried <kseifried@...hat.com>
Subject: Re: TMP flaw in rackspace jclouds?

[bcc: jclouds private list]

Since oss-security@...ts.openwall.com is a public list I reported the
issue and submitted a pull request:

https://issues.apache.org/jira/browse/JCLOUDS-612
https://github.com/jclouds/jclouds/pull/420

On Thu, Jun 19, 2014 at 03:55:49PM -0700, Andrew Gaul wrote:
> [bcc: jclouds private list]
> 
> I attached a patch which changes ScriptBuilder to use the "mktemp -d"
> approach that Ignasi suggested.  I verified this against the ec2 live
> tests, specifically testCreateAndRunAService, as well as the
> scriptbuilder unit tests.  I encourage someone more familiar with
> compute to test this since I have limited experience with those
> services.
> 
> The code runs on the target node as Ignasi describes and thus it poses
> no risk to the master jclouds application.  However, users could run
> untrusted software on their target nodes and we should address this in
> the next minor release.  I estimate a low severity to this issue and
> prefer to continue discussion on the public bug tracker.  Does anyone
> have a different understanding of this flaw?
> 
> On Thu, Jun 19, 2014 at 09:32:25AM +0200, Ignasi Barrera wrote:
> > Take into account that the "statement" list will be rendered to a String,
> > composed with other script fragments into a final bash script, uploaded to
> > a node, and executed there locally as a bash script.
> > 
> > That code won't be executed in the machine running jclouds, but as a bash
> > script in the provisioned node, so the name of the temporal directory
> > should better be generated in the script itself. A good approach would be
> > to directly use the "mktemp"command.
> > El 19/06/2014 06:36, "Andrew Gaul" <gaul@...che.org> escribió:
> > 
> > > Kurt, thank you for bringing this flaw to my attention and I will
> > > address it tomorrow.  I do not have a security background; can you
> > > estimate the severity and whether we can continue discussion on the
> > > public bug tracker?  For now I have bcc the Apache jclouds private
> > > mailing list.  Also note that jclouds is an Apache project not a
> > > Rackspace project and the canonical URLs are:
> > >
> > > https://github.com/jclouds/jclouds
> > > https://issues.apache.org/jira/browse/JCLOUDS
> > >
> > > On Wed, Jun 18, 2014 at 08:52:59PM -0600, Kurt Seifried wrote:
> > > > -----BEGIN PGP SIGNED MESSAGE-----
> > > > Hash: SHA1
> > > >
> > > > https://github.com/rackspace/jclouds/
> > > >
> > > > So CC'ing Andrew, he's a consistent contributor, I can't file an issue
> > > > in Github (no link to it) so posting here and CC'ing him.
> > > >
> > > >
> > > https://github.com/rackspace/jclouds/blob/master/scriptbuilder/src/main/java/org/jclouds/scriptbuilder/domain/Statements.java
> > > >
> > > >   public static Statement extractTargzAndFlattenIntoDirectory(URI tgz,
> > > > String dest) {
> > > >       return new StatementList(ImmutableSet.<Statement> builder()
> > > >             .add(exec("mkdir /tmp/$$"))
> > > >             .add(extractTargzIntoDirectory(tgz, "/tmp/$$"))
> > > >             .add(exec("mkdir -p " + dest))
> > > >             .add(exec("mv /tmp/$$/*/* " + dest))
> > > >             .add(exec("rm -rf /tmp/$$")).build());
> > > >    }
> > > >
> > > >
> > > > This is insecure, $$ == PID == predictable
> > > >
> > > > http://kurt.seifried.org/2012/03/14/creating-temporary-files-securely/
> > > >
> > > > use java.io.File.createTempFile() ? some interesting info at
> > > >
> > > http://www.veracode.com/blog/2009/01/how-boring-flaws-become-interesting/
> > > >
> > > > for directories there is a helpful posting at
> > > >
> > > http://stackoverflow.com/questions/617414/create-a-temporary-directory-in-java
> > > >
> > > > Thanks.
> > > >
> > > >
> > > > - --
> > > > Kurt Seifried -- Red Hat -- Product Security -- Cloud
> > > > PGP A90B F995 7350 148F 66BF 7554 160D 4553 5E26 7993
> > > > -----BEGIN PGP SIGNATURE-----
> > > > Version: GnuPG v1
> > > > Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
> > > >
> > > > iQIcBAEBAgAGBQJTolCLAAoJEBYNRVNeJnmTrVYQAJ5glkD/0Ha5+F99Qj9ioNmm
> > > > ZnO4G6TqKctfiqW/X02wMocKLMRV8q5WI/nvs71hCoK5HaVmbtNrV71wE0omHLjB
> > > > smzFz6d8qZaTcOHdvgbSlWEGPjcVnESo0F3K0vgK2L/LtB5mgny6pHDn+c/cqrgt
> > > > Er4n+U3oXlkon/ksW+drWpKOpmGOhn7c4fbE45ci6KnzDbbGpGHF0fZL3lSEfJR0
> > > > 0D/HQzKIAJpI7VvZU8+/d/MHasndgJoAHmUCkTBYU55Vf5eYsm+xWZ1Mt46IyAap
> > > > crMTCHHE1GVUAexYbMxy+lohHbpl+pB/d////LzesJjByRSv87r+1oLhdwank3P9
> > > > Fz1h3sq57JyLFQIcpm4TS7xh3TaByFGCiA5G/mR+CkuS6sZEapSkviu/x7ygmOdG
> > > > cJKM+5CogeE1P1PWsoQ41JcSwfuWAfc5IODvkjLb3MfyoXJRaKcBVdVcdHBUK4BA
> > > > 7xcD9SbDsujxHOJLknFaO22uTtlrDS4yXJaNal6L9P7DCsSSrxG1PmmE+t5qrtYw
> > > > HQoz+RuOMhY/2FWJqOxa7ru99rIQmxxpWgoknUlT+yYJRfoub0kpibyJLBLy2SEx
> > > > xmdqe/i9nHCsGAworK4bEL2vLvsNBiJgdSHlzg7E5POI1tbveE12fIUmSgrgV+zO
> > > > WjPZ/O4oOj0FVWoeyQUN
> > > > =SUf5
> > > > -----END PGP SIGNATURE-----
> > >
> > > --
> > > Andrew Gaul
> > > http://gaul.org/
> > >
> 
> -- 
> Andrew Gaul
> http://gaul.org/

> commit d7321e980b9e66fe95aea340d58ae26baef878d8
> Author: Andrew Gaul <gaul@...che.org>
> Date:   Thu Jun 19 14:20:26 2014 -0700
> 
>     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.
> 
> diff --git a/compute/src/test/resources/initscript_with_jetty.sh b/compute/src/test/resources/initscript_with_jetty.sh
> index fca9683..741b6d2 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 e752e71..aa64d1b 100644
> --- a/scriptbuilder/src/main/java/org/jclouds/scriptbuilder/domain/Statements.java
> +++ b/scriptbuilder/src/main/java/org/jclouds/scriptbuilder/domain/Statements.java
> @@ -187,11 +187,12 @@ public class Statements {
>      */
>     public static Statement extractTargzAndFlattenIntoDirectory(URI tgz, String dest) {
>        return new StatementList(ImmutableSet.<Statement> 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 dbea7ab..72fc452 100644
> --- a/scriptbuilder/src/test/java/org/jclouds/scriptbuilder/domain/StatementsTest.java
> +++ b/scriptbuilder/src/test/java/org/jclouds/scriptbuilder/domain/StatementsTest.java
> @@ -51,11 +51,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 0f17bb2..81128fe 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
> @@ -85,10 +85,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 c9363d2..169250c 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 1c4bb5f..3df852e 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


-- 
Andrew Gaul
http://gaul.org/

Powered by blists - more mailing lists

Your e-mail address:

Please check out the Open Source Software Security Wiki, which is counterpart to this mailing list.

Powered by Openwall GNU/*/Linux - Powered by OpenVZ