From e29affe66625bb8493753a5c542dac6a0416e942 Mon Sep 17 00:00:00 2001 From: Todd Zullinger Date: Tue, 27 Sep 2011 20:39:11 -0400 Subject: [PATCH] Apply upstream patch for CVE-2011-3848 --- ...st-directory-traversal-attacks-2.6.x.patch | 138 ++++++++++++++++++ puppet.spec | 8 +- 2 files changed, 145 insertions(+), 1 deletion(-) create mode 100644 0001-Resist-directory-traversal-attacks-2.6.x.patch diff --git a/0001-Resist-directory-traversal-attacks-2.6.x.patch b/0001-Resist-directory-traversal-attacks-2.6.x.patch new file mode 100644 index 0000000..8c5bb49 --- /dev/null +++ b/0001-Resist-directory-traversal-attacks-2.6.x.patch @@ -0,0 +1,138 @@ +From 0a92a70a22b7e85ef60ed9b4d4070433b5ec3220 Mon Sep 17 00:00:00 2001 +From: Daniel Pittman +Date: Sat, 24 Sep 2011 12:44:20 -0700 +Subject: [PATCH] Resist directory traversal attacks through indirections. + +In various versions of Puppet it was possible to cause a directory traversal +attack through the SSLFile indirection base class. This was variously +triggered through the user-supplied key, or the Subject of the certificate, in +the code. + +Now, we detect bad patterns down in the base class for our indirections, and +fail hard on them. This reduces the attack surface with as little disruption +to the overall codebase as possible, making it suitable to deploy as part of +older, stable versions of Puppet. + +In the long term we will also address this higher up the stack, to prevent +these problems from reoccurring, but for now this will suffice. + +Huge thanks to Kristian Erik Hermansen for the +responsible disclosure, and useful analysis, around this defect. + +Signed-off-by: Daniel Pittman +--- + lib/puppet/indirector.rb | 7 +++++++ + lib/puppet/indirector/ssl_file.rb | 6 +++++- + lib/puppet/indirector/yaml.rb | 5 +++++ + spec/unit/indirector/ssl_file_spec.rb | 19 +++++++++++++++++++ + spec/unit/indirector/yaml_spec.rb | 14 ++++++++++++++ + 5 files changed, 50 insertions(+), 1 deletions(-) + +diff --git a/lib/puppet/indirector.rb b/lib/puppet/indirector.rb +index e6472f4..fd6bf30 100644 +--- a/lib/puppet/indirector.rb ++++ b/lib/puppet/indirector.rb +@@ -68,4 +68,11 @@ module Puppet::Indirector + self.class.indirection.save key, self + end + end ++ ++ ++ # Helper definition for indirections that handle filenames. ++ BadNameRegexp = Regexp.union(/^\.\./, ++ %r{[\\/]}, ++ "\0", ++ /(?i)^[a-z]:/) + end +diff --git a/lib/puppet/indirector/ssl_file.rb b/lib/puppet/indirector/ssl_file.rb +index 531180f..4510499 100644 +--- a/lib/puppet/indirector/ssl_file.rb ++++ b/lib/puppet/indirector/ssl_file.rb +@@ -52,8 +52,12 @@ class Puppet::Indirector::SslFile < Puppet::Indirector::Terminus + (collection_directory || file_location) or raise Puppet::DevError, "No file or directory setting provided; terminus #{self.class.name} cannot function" + end + +- # Use a setting to determine our path. + def path(name) ++ if name =~ Puppet::Indirector::BadNameRegexp then ++ Puppet.crit("directory traversal detected in #{self.class}: #{name.inspect}") ++ raise ArgumentError, "invalid key" ++ end ++ + if ca?(name) and ca_location + ca_location + elsif collection_directory +diff --git a/lib/puppet/indirector/yaml.rb b/lib/puppet/indirector/yaml.rb +index 23997e9..4c488da 100644 +--- a/lib/puppet/indirector/yaml.rb ++++ b/lib/puppet/indirector/yaml.rb +@@ -43,6 +43,11 @@ class Puppet::Indirector::Yaml < Puppet::Indirector::Terminus + + # Return the path to a given node's file. + def path(name,ext='.yaml') ++ if name =~ Puppet::Indirector::BadNameRegexp then ++ Puppet.crit("directory traversal detected in #{self.class}: #{name.inspect}") ++ raise ArgumentError, "invalid key" ++ end ++ + base = Puppet.run_mode.master? ? Puppet[:yamldir] : Puppet[:clientyamldir] + File.join(base, self.class.indirection_name.to_s, name.to_s + ext) + end +diff --git a/spec/unit/indirector/ssl_file_spec.rb b/spec/unit/indirector/ssl_file_spec.rb +index 37098a7..4760bd7 100755 +--- a/spec/unit/indirector/ssl_file_spec.rb ++++ b/spec/unit/indirector/ssl_file_spec.rb +@@ -87,6 +87,25 @@ describe Puppet::Indirector::SslFile do + it "should set them in the setting directory, with the certificate name plus '.pem', if a directory setting is available" do + @searcher.path(@cert.name).should == @certpath + end ++ ++ ['../foo', '..\\foo', './../foo', '.\\..\\foo', ++ '/foo', '//foo', '\\foo', '\\\\goo', ++ "test\0/../bar", "test\0\\..\\bar", ++ "..\\/bar", "/tmp/bar", "/tmp\\bar", "tmp\\bar", ++ " / bar", " /../ bar", " \\..\\ bar", ++ "c:\\foo", "c:/foo", "\\\\?\\UNC\\bar", "\\\\foo\\bar", ++ "\\\\?\\c:\\foo", "//?/UNC/bar", "//foo/bar", ++ "//?/c:/foo", ++ ].each do |input| ++ it "should resist directory traversal attacks (#{input.inspect})" do ++ expect { @searcher.path(input) }.to raise_error ++ end ++ end ++ ++ # REVISIT: Should probably test MS-DOS reserved names here, too, since ++ # they would represent a vulnerability on a Win32 system, should we ever ++ # support that path. Don't forget that 'CON.foo' == 'CON' ++ # --daniel 2011-09-24 + end + + describe "when finding certificates on disk" do +diff --git a/spec/unit/indirector/yaml_spec.rb b/spec/unit/indirector/yaml_spec.rb +index 86c13c5..c8fadf7 100755 +--- a/spec/unit/indirector/yaml_spec.rb ++++ b/spec/unit/indirector/yaml_spec.rb +@@ -63,6 +63,20 @@ describe Puppet::Indirector::Yaml, " when choosing file location" do + it "should use the object's name to determine the file name" do + @store.path(:me).should =~ %r{me.yaml$} + end ++ ++ ['../foo', '..\\foo', './../foo', '.\\..\\foo', ++ '/foo', '//foo', '\\foo', '\\\\goo', ++ "test\0/../bar", "test\0\\..\\bar", ++ "..\\/bar", "/tmp/bar", "/tmp\\bar", "tmp\\bar", ++ " / bar", " /../ bar", " \\..\\ bar", ++ "c:\\foo", "c:/foo", "\\\\?\\UNC\\bar", "\\\\foo\\bar", ++ "\\\\?\\c:\\foo", "//?/UNC/bar", "//foo/bar", ++ "//?/c:/foo", ++ ].each do |input| ++ it "should resist directory traversal attacks (#{input.inspect})" do ++ expect { @store.path(input) }.to raise_error ++ end ++ end + end + + describe Puppet::Indirector::Yaml, " when storing objects as YAML" do +-- +1.7.4.4 + diff --git a/puppet.spec b/puppet.spec index 9302ae9..4e18bf9 100644 --- a/puppet.spec +++ b/puppet.spec @@ -6,7 +6,7 @@ Name: puppet Version: 2.6.6 -Release: 1%{?dist} +Release: 2%{?dist} Summary: A network tool for managing many disparate systems License: GPLv2 URL: http://puppetlabs.com @@ -18,6 +18,8 @@ Patch0: 0001-5428-More-fully-stub-Puppet-Resource-Reference-for-u.patch Patch1: 0001-4922-Don-t-truncate-remotely-sourced-files-on-404.patch # http://projects.puppetlabs.com/issues/5073 Patch2: 0001-5073-Download-plugins-even-if-you-re-filtering-on-ta.patch +# http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2011-3848 +Patch3: 0001-Resist-directory-traversal-attacks-2.6.x.patch Group: System Environment/Base @@ -75,6 +77,7 @@ The server can also function as a certificate authority and file server. %patch0 -p1 %patch1 -p1 %patch2 -p1 +%patch3 -p1 patch -s -p1 < conf/redhat/rundir-perms.patch @@ -262,6 +265,9 @@ fi rm -rf %{buildroot} %changelog +* Tue Sep 27 2011 Todd Zullinger - 2.6.6-2 +- Apply upstream patch for CVE-2011-3848 + * Wed Mar 16 2011 Todd Zullinger - 2.6.6-1 - Update to 2.6.6 - Ensure %%pre exits cleanly