* lib/resolv.rb: randomize source port and transaction id.

CVE-2008-1447.

* lib/resolv-replace.rb (UDPSocket#bind): don't resolv host if host is
  "".


git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby_1_8@18424 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

ChangeLog

@@ -1,3 +1,11 @@
+Fri Aug  8 10:53:52 2008  Tanaka Akira  <akr@fsij.org>
+
+	* lib/resolv.rb: randomize source port and transaction id.
+	  CVE-2008-1447.
+
+	* lib/resolv-replace.rb (UDPSocket#bind): don't resolv host if host is
+	  "".
+
 Thu Aug  7 20:55:49 2008  Shugo Maeda  <shugo@ruby-lang.org>
 
 	* lib/net/ftp.rb (login): raise FTPReplyError if passwd or acct

lib/resolv-replace.rb

@@ -23,7 +23,8 @@ end
 class UDPSocket
   alias original_resolv_bind bind
   def bind(host, port)
-    original_resolv_bind(IPSocket.getaddress(host), port)
+    host = IPSocket.getaddress(host) if host != ""
+    original_resolv_bind(host, port)
   end
 
   alias original_resolv_connect connect

lib/resolv.rb

@@ -3,6 +3,11 @@ require 'fcntl'
 require 'timeout'
 require 'thread'
 
+begin
+  require 'securerandom'
+rescue LoadError
+end
+
 # Resolv is a thread-aware DNS resolver library written in Ruby.  Resolv can
 # handle multiple DNS requests concurrently without blocking.  The ruby
 # interpreter.
@@ -285,7 +290,7 @@ class Resolv
     UDPSize = 512
 
     ##
-    # Group of DNS resolver threads
+    # Group of DNS resolver threads (obsolete)
 
     DNSThreadGroup = ThreadGroup.new
 
@@ -330,13 +335,6 @@ class Resolv
       @mutex.synchronize {
         unless @initialized
           @config.lazy_initialize
-
-          if nameserver = @config.single?
-            @requester = Requester::ConnectedUDP.new(nameserver)
-          else
-            @requester = Requester::UnconnectedUDP.new
-          end
-
           @initialized = true
         end
       }
@@ -349,8 +347,6 @@ class Resolv
     def close
       @mutex.synchronize {
         if @initialized
-          @requester.close if @requester
-          @requester = nil
           @initialized = false
         end
       }
@@ -479,7 +475,7 @@ class Resolv
   
     def each_resource(name, typeclass, &proc)
       lazy_initialize
-      q = Queue.new
+      requester = make_requester
       senders = {}
       begin
         @config.resolv(name) {|candidate, tout, nameserver|
@@ -488,11 +484,9 @@ class Resolv
           msg.add_question(candidate, typeclass)
           unless sender = senders[[candidate, nameserver]]
             sender = senders[[candidate, nameserver]] =
-              @requester.sender(msg, candidate, q, nameserver)
+              requester.sender(msg, candidate, nameserver)
           end
-          sender.send
+          reply, reply_name = requester.request(sender, tout)
-          reply = reply_name = nil
-          timeout(tout, ResolvTimeout) { reply, reply_name = q.pop }
           case reply.rcode
           when RCode::NoError
             extract_resources(reply, reply_name, typeclass, &proc)
@@ -504,7 +498,15 @@ class Resolv
           end
         }
       ensure
-        @requester.delete(q)
+        requester.close
+      end
+    end
+
+    def make_requester # :nodoc:
+      if nameserver = @config.single?
+        Requester::ConnectedUDP.new(nameserver)
+      else
+        Requester::UnconnectedUDP.new
       end
     end
 
@@ -539,45 +541,106 @@ class Resolv
       }
     end
 
+    if defined? SecureRandom
+      def self.random(arg) # :nodoc:
+        begin
+          SecureRandom.random_number(arg)
+        rescue NotImplementedError
+          rand(arg)
+        end
+      end
+    else
+      def self.random(arg) # :nodoc:
+        rand(arg)
+      end
+    end
+
+
+    def self.rangerand(range) # :nodoc:
+      base = range.begin
+      len = range.end - range.begin
+      if !range.exclude_end?
+        len += 1
+      end
+      base + random(len)
+    end
+
+    RequestID = {}
+    RequestIDMutex = Mutex.new
+
+    def self.allocate_request_id(host, port) # :nodoc:
+      id = nil
+      RequestIDMutex.synchronize {
+        h = (RequestID[[host, port]] ||= {})
+        begin
+          id = rangerand(0x0000..0xffff)
+        end while h[id] 
+        h[id] = true
+      }
+      id
+    end
+
+    def self.free_request_id(host, port, id) # :nodoc:
+      RequestIDMutex.synchronize {
+        key = [host, port]
+        if h = RequestID[key]
+          h.delete id
+          if h.empty?
+            RequestID.delete key
+          end
+        end
+      }
+    end
+
+    def self.bind_random_port(udpsock) # :nodoc:
+      begin
+        port = rangerand(1024..65535)
+        udpsock.bind("", port)
+      rescue Errno::EADDRINUSE
+        retry
+      end
+    end
+
     class Requester # :nodoc:
       def initialize
         @senders = {}
+        @sock = nil
+      end
+
+      def request(sender, tout)
+        timelimit = Time.now + tout
+        sender.send
+        while (now = Time.now) < timelimit
+          timeout = timelimit - now
+          if !IO.select([@sock], nil, nil, timeout)
+            raise ResolvTimeout
+          end
+          reply, from = recv_reply
+          begin
+            msg = Message.decode(reply)
+          rescue DecodeError
+            next # broken DNS message ignored
+          end
+          if s = @senders[[from,msg.id]]
+            break
+          else
+            # unexpected DNS message ignored
+          end
+        end
+        return msg, s.data
       end
 
       def close
-        thread, sock, @thread, @sock = @thread, @sock
+        sock = @sock
-        begin
+        @sock = nil
-          if thread
+        sock.close if sock
-            thread.kill
-            thread.join
-          end
-        ensure
-          sock.close if sock
-        end
-      end
-
-      def delete(arg)
-        case arg
-        when Sender
-          @senders.delete_if {|k, s| s == arg }
-        when Queue
-          @senders.delete_if {|k, s| s.queue == arg }
-        else
-          raise ArgumentError.new("neither Sender or Queue: #{arg}")
-        end
       end
 
       class Sender # :nodoc:
-        def initialize(msg, data, sock, queue)
+        def initialize(msg, data, sock)
           @msg = msg
           @data = data
           @sock = sock
-          @queue = queue
-        end
-        attr_reader :queue
-
-        def recv(msg)
-          @queue.push([msg, @data])
         end
       end
 
@@ -585,45 +648,38 @@ class Resolv
         def initialize
           super()
           @sock = UDPSocket.new
-          @sock.fcntl(Fcntl::F_SETFD, 1) if defined? Fcntl::F_SETFD
+          @sock.fcntl(Fcntl::F_SETFD, Fcntl::FD_CLOEXEC) if defined? Fcntl::F_SETFD
-          @id = {}
+          DNS.bind_random_port(@sock)
-          @id.default = -1
-          @thread = Thread.new {
-            DNSThreadGroup.add Thread.current
-            loop {
-              reply, from = @sock.recvfrom(UDPSize)
-              msg = begin
-                Message.decode(reply)
-              rescue DecodeError
-                STDERR.print("DNS message decoding error: #{reply.inspect}\n")
-                next
-              end
-              if s = @senders[[[from[3],from[1]],msg.id]]
-                s.recv msg
-              else
-                #STDERR.print("non-handled DNS message: #{msg.inspect} from #{from.inspect}\n")
-              end
-            }
-          }
         end
 
-        def sender(msg, data, queue, host, port=Port)
+        def recv_reply
+          reply, from = @sock.recvfrom(UDPSize)
+          return reply, [from[3],from[1]]
+        end
+
+        def sender(msg, data, host, port=Port)
           service = [host, port]
-          id = Thread.exclusive {
+          id = DNS.allocate_request_id(host, port)
-            @id[service] = (@id[service] + 1) & 0xffff
-          }
           request = msg.encode
           request[0,2] = [id].pack('n')
           return @senders[[service, id]] =
-            Sender.new(request, data, @sock, host, port, queue)
+            Sender.new(request, data, @sock, host, port)
+        end
+
+        def close
+          super
+          @senders.each_key {|service, id|
+            DNS.free_request_id(service[0], service[1], id)
+          }
         end
 
         class Sender < Requester::Sender # :nodoc:
-          def initialize(msg, data, sock, host, port, queue)
+          def initialize(msg, data, sock, host, port)
-            super(msg, data, sock, queue)
+            super(msg, data, sock)
             @host = host
             @port = port
           end
+          attr_reader :data
 
           def send
             @sock.send(@msg, 0, @host, @port)
@@ -637,42 +693,38 @@ class Resolv
           @host = host
           @port = port
           @sock = UDPSocket.new(host.index(':') ? Socket::AF_INET6 : Socket::AF_INET)
+          DNS.bind_random_port(@sock)
           @sock.connect(host, port)
-          @sock.fcntl(Fcntl::F_SETFD, 1) if defined? Fcntl::F_SETFD
+          @sock.fcntl(Fcntl::F_SETFD, Fcntl::FD_CLOEXEC) if defined? Fcntl::F_SETFD
-          @id = -1
-          @thread = Thread.new {
-            DNSThreadGroup.add Thread.current
-            loop {
-              reply = @sock.recv(UDPSize)
-              msg = begin
-                Message.decode(reply)
-              rescue DecodeError
-                STDERR.print("DNS message decoding error: #{reply.inspect}")
-                next
-              end
-              if s = @senders[msg.id]
-                s.recv msg
-              else
-                #STDERR.print("non-handled DNS message: #{msg.inspect}")
-              end
-            }
-          }
         end
 
-        def sender(msg, data, queue, host=@host, port=@port)
+        def recv_reply
+          reply = @sock.recv(UDPSize)
+          return reply, nil
+        end
+
+        def sender(msg, data, host=@host, port=@port)
           unless host == @host && port == @port
             raise RequestError.new("host/port don't match: #{host}:#{port}")
           end
-          id = Thread.exclusive { @id = (@id + 1) & 0xffff }
+          id = DNS.allocate_request_id(@host, @port)
           request = msg.encode
           request[0,2] = [id].pack('n')
-          return @senders[id] = Sender.new(request, data, @sock, queue)
+          return @senders[[nil,id]] = Sender.new(request, data, @sock)
+        end
+
+        def close
+          super
+          @senders.each_key {|from, id|
+            DNS.free_request_id(@host, @port, id)
+          }
         end
 
         class Sender < Requester::Sender # :nodoc:
           def send
             @sock.send(@msg, 0)
           end
+          attr_reader :data
         end
       end
 
@@ -681,39 +733,25 @@ class Resolv
           super()
           @host = host
           @port = port
-          @sock = TCPSocket.new
+          @sock = TCPSocket.new(@host, @port)
-          @sock.connect(host, port)
+          @sock.fcntl(Fcntl::F_SETFD, Fcntl::FD_CLOEXEC) if defined? Fcntl::F_SETFD
-          @sock.fcntl(Fcntl::F_SETFD, 1) if defined? Fcntl::F_SETFD
-          @id = -1
           @senders = {}
-          @thread = Thread.new {
-            DNSThreadGroup.add Thread.current
-            loop {
-              len = @sock.read(2).unpack('n')
-              reply = @sock.read(len)
-              msg = begin
-                Message.decode(reply)
-              rescue DecodeError
-                STDERR.print("DNS message decoding error: #{reply.inspect}")
-                next
-              end
-              if s = @senders[msg.id]
-                s.push msg
-              else
-                #STDERR.print("non-handled DNS message: #{msg.inspect}")
-              end
-            }
-          }
         end
 
-        def sender(msg, data, queue, host=@host, port=@port)
+        def recv_reply
+          len = @sock.read(2).unpack('n')[0]
+          reply = @sock.read(len)
+          return reply, nil
+        end
+
+        def sender(msg, data, host=@host, port=@port)
           unless host == @host && port == @port
             raise RequestError.new("host/port don't match: #{host}:#{port}")
           end
-          id = Thread.exclusive { @id = (@id + 1) & 0xffff }
+          id = DNS.allocate_request_id(@host, @port)
           request = msg.encode
           request[0,2] = [request.length, id].pack('nn')
-          return @senders[id] = Sender.new(request, data, @sock, queue)
+          return @senders[[nil,id]] = Sender.new(request, data, @sock)
         end
 
         class Sender < Requester::Sender # :nodoc:
@@ -721,6 +759,14 @@ class Resolv
             @sock.print(@msg)
             @sock.flush
           end
+          attr_reader :data
+        end
+
+        def close
+          super
+          @senders.each_key {|from,id|
+            DNS.free_request_id(@host, @port, id)
+          }
         end
       end