From 77ade43dec5e6fc5afac7abe4b331a3bc7887e42 Mon Sep 17 00:00:00 2001
From: Luke Kanies <luke@madstop.com>
Date: Fri, 20 Mar 2009 00:25:16 -0500
Subject: Forbidding REST clients to set the node or IP

This is done for security reasons - if a client is
unauthenticated, we don't want them to be able to
just configure their own authentication information.

Signed-off-by: Luke Kanies <luke@madstop.com>
---
 lib/puppet/network/http/handler.rb     |  8 +++++++-
 spec/unit/network/http/mongrel/rest.rb | 10 ++++++++++
 spec/unit/network/http/webrick/rest.rb | 10 ++++++++++
 3 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/lib/puppet/network/http/handler.rb b/lib/puppet/network/http/handler.rb
index db12a8b67..04ba14401 100644
--- a/lib/puppet/network/http/handler.rb
+++ b/lib/puppet/network/http/handler.rb
@@ -160,6 +160,12 @@ module Puppet::Network::HTTP::Handler
     def decode_params(params)
         params.inject({}) do |result, ary|
             param, value = ary
+            param = param.to_sym
+
+            # These shouldn't be allowed to be set by clients
+            # in the query string, for security reasons.
+            next result if param == :node
+            next result if param == :ip
             value = URI.unescape(value)
             if value =~ /^---/
                 value = YAML.load(value)
@@ -169,7 +175,7 @@ module Puppet::Network::HTTP::Handler
                 value = Integer(value) if value =~ /^\d+$/
                 value = value.to_f if value =~ /^\d+\.\d+$/
             end
-            result[param.to_sym] = value
+            result[param] = value
             result
         end
     end
diff --git a/spec/unit/network/http/mongrel/rest.rb b/spec/unit/network/http/mongrel/rest.rb
index 84a7e7f64..1926a6e45 100755
--- a/spec/unit/network/http/mongrel/rest.rb
+++ b/spec/unit/network/http/mongrel/rest.rb
@@ -137,6 +137,16 @@ describe "Puppet::Network::HTTP::MongrelREST" do
                 result[:foo].should == %w{one two}
             end
 
+            it "should not allow the client to set the node via the query string" do
+                @request.stubs(:params).returns('QUERY_STRING' => "node=foo")
+                @handler.params(@request)[:node].should be_nil
+            end
+
+            it "should not allow the client to set the IP address via the query string" do
+                @request.stubs(:params).returns('QUERY_STRING' => "ip=foo")
+                @handler.params(@request)[:ip].should be_nil
+            end
+
             it "should pass the client's ip address to model find" do
                 @request.stubs(:params).returns("REMOTE_ADDR" => "ipaddress")
                 @handler.params(@request)[:ip].should == "ipaddress"
diff --git a/spec/unit/network/http/webrick/rest.rb b/spec/unit/network/http/webrick/rest.rb
index bb0918131..7600cff2c 100755
--- a/spec/unit/network/http/webrick/rest.rb
+++ b/spec/unit/network/http/webrick/rest.rb
@@ -106,6 +106,16 @@ describe Puppet::Network::HTTP::WEBrickREST do
                 result[:foo].should == %w{one two}
             end
 
+            it "should not allow clients to set the node via the request parameters" do
+                @request.stubs(:query).returns("node" => "foo")
+                @handler.params(@request)[:node].should be_nil
+            end
+
+            it "should not allow clients to set the IP via the request parameters" do
+                @request.stubs(:query).returns("ip" => "foo")
+                @handler.params(@request)[:ip].should_not == "foo"
+            end
+
             it "should pass the client's ip address to model find" do
                 @request.stubs(:peeraddr).returns(%w{noidea dunno hostname ipaddress})
                 @handler.params(@request)[:ip].should == "ipaddress"
-- 
cgit