CVE-2009-3287: Improper Input Validation (CWE-20)
value: lib/thin/connection.rb in Thin web server before 1.2.4 relies on the X-Forwarded-For header to determine the IP address of the client, which allows remote attackers to spoof the IP address and hide activities via a modified X-Forwarded-For header.
CWE description: The product receives input or data, but it does
        not validate or incorrectly validates that the input has the
        properties that are required to process the data safely and
        correctly.
Published: 2009-09-22T10:30Z
Repository: https://github.com/macournoyer/thin
Language: Ruby
Files changed: CHANGELOG, lib/thin/connection.rb, lib/thin/request.rb, lib/thin/version.rb, spec/connection_spec.rb
Commit date: 2009-08-12 17:28:38 -0400
Commit message: Fix Remote address spoofing vulnerability in Connection#remote_address [Alexey Borzenkov]

Diff: diff --git a/CHANGELOG b/CHANGELOG
index 595de22..4cc2b5a 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,6 +1,7 @@
 == 1.2.3
+ * Fix Remote address spoofing vulnerability in Connection#remote_address [Alexey Borzenkov]
  * Fix uninitialized constant ActionController::Dispatcher error with Rails 1.2.3 [Chris Anderton] [#103 state:resolved]
 
 == 1.2.2 I Find Your Lack of Sauce Disturbing release
  * Fix force kill under 1.9 [Alexey Chebotar]
  * Fix regression when --only option is used w/ --socket.
diff --git a/lib/thin/connection.rb b/lib/thin/connection.rb
index 1da7d1e..7ead989 100644
--- a/lib/thin/connection.rb
+++ b/lib/thin/connection.rb
@@ -178,11 +178,11 @@ module Thin
       @threaded || (@app.respond_to?(:deferred?) && @app.deferred?(@request.env))
     end
 
     # IP Address of the remote client.
     def remote_address
-      @request.forwarded_for || socket_address
+      socket_address
     rescue Exception
       log_error
       nil
     end
 
diff --git a/lib/thin/request.rb b/lib/thin/request.rb
index 60adae7..0add8a6 100644
--- a/lib/thin/request.rb
+++ b/lib/thin/request.rb
@@ -19,11 +19,10 @@ module Thin
     SERVER_NAME       = 'SERVER_NAME'.freeze
     LOCALHOST         = 'localhost'.freeze
     HTTP_VERSION      = 'HTTP_VERSION'.freeze
     HTTP_1_0          = 'HTTP/1.0'.freeze
     REMOTE_ADDR       = 'REMOTE_ADDR'.freeze
-    FORWARDED_FOR     = 'HTTP_X_FORWARDED_FOR'.freeze
     CONTENT_LENGTH    = 'CONTENT_LENGTH'.freeze
     CONNECTION        = 'HTTP_CONNECTION'.freeze
     KEEP_ALIVE_REGEXP = /\bkeep-alive\b/i.freeze
     CLOSE_REGEXP      = /\bclose\b/i.freeze
     
@@ -121,14 +120,10 @@ module Thin
 
     def remote_address=(address)
       @env[REMOTE_ADDR] = address
     end
 
-    def forwarded_for
-      @env[FORWARDED_FOR]
-    end
-
     def threaded=(value)
       @env[RACK_MULTITHREAD] = value
     end
     
     def async_callback=(callback)
diff --git a/lib/thin/version.rb b/lib/thin/version.rb
index bcab7e6..c6242d2 100644
--- a/lib/thin/version.rb
+++ b/lib/thin/version.rb
@@ -4,15 +4,15 @@ module Thin
   class PlatformNotSupported < RuntimeError; end
   
   module VERSION #:nodoc:
     MAJOR    = 1
     MINOR    = 2
-    TINY     = 2
+    TINY     = 3
     
     STRING   = [MAJOR, MINOR, TINY].join('.')
     
-    CODENAME = "I Find Your Lack of Sauce Disturbing".freeze
+    CODENAME = "Astroboy".freeze
     
     RACK     = [1, 0].freeze # Rack protocol version
   end
   
   NAME    = 'thin'.freeze
diff --git a/spec/connection_spec.rb b/spec/connection_spec.rb
index 151e783..1167211 100644
--- a/spec/connection_spec.rb
+++ b/spec/connection_spec.rb
@@ -38,13 +38,14 @@ describe Connection do
   it "should rescue Timeout error in process" do
     @connection.app.should_receive(:call).and_raise(Timeout::Error.new("timeout error not rescued"))
     @connection.process
   end
   
-  it "should return HTTP_X_FORWARDED_FOR as remote_address" do
+  it "should not return HTTP_X_FORWARDED_FOR as remote_address" do
     @connection.request.env['HTTP_X_FORWARDED_FOR'] = '1.2.3.4'
-    @connection.remote_address.should == '1.2.3.4'
+    @connection.stub!(:socket_address).and_return("127.0.0.1")
+    @connection.remote_address.should == "127.0.0.1"
   end
   
   it "should return nil on error retreiving remote_address" do
     @connection.stub!(:get_peername).and_raise(RuntimeError)
     @connection.remote_address.should be_nil

Vulnerable code:       @request.forwarded_for || socket_address
    FORWARDED_FOR     = 'HTTP_X_FORWARDED_FOR'.freeze
    def forwarded_for
      @env[FORWARDED_FOR]
    end

    TINY     = 2
    CODENAME = "I Find Your Lack of Sauce Disturbing".freeze
  it "should return HTTP_X_FORWARDED_FOR as remote_address" do
    @connection.remote_address.should == '1.2.3.4'
Fixed code:  * Fix Remote address spoofing vulnerability in Connection#remote_address [Alexey Borzenkov]
      socket_address
    TINY     = 3
    CODENAME = "Astroboy".freeze
  it "should not return HTTP_X_FORWARDED_FOR as remote_address" do
    @connection.stub!(:socket_address).and_return("127.0.0.1")
    @connection.remote_address.should == "127.0.0.1"