[Gnash-commit] /srv/bzr/gnash/trunk r9858: Check that data is returned in XMLSocket_as::fillMessageList to prevent

[Benjamin Wolsey]

------------------------------------------------------------
revno: 9858
committer: Benjamin Wolsey <address@hidden>
branch nick: trunk
timestamp: Sun 2008-09-28 22:20:51 +0200
message:
  Check that data is returned in XMLSocket_as::fillMessageList to prevent
  out-of-bounds reads. Don't recycle the return from select() for read(),
  which doesn't help clarity. And make the function void, as we don't
  care about the return.
  
  Fixes an abort in the CSU. Er, I mean the tagesschau.de election monitor
  (which almost works now, by the way).
modified:
  libcore/asobj/XMLSocket_as.cpp

=== modified file 'libcore/asobj/XMLSocket_as.cpp'
--- a/libcore/asobj/XMLSocket_as.cpp    2008-09-16 17:45:34 +0000
+++ b/libcore/asobj/XMLSocket_as.cpp    2008-09-28 20:20:51 +0000
@@ -90,7 +90,7 @@
 
 private:
 
-    bool fillMessageList(MessageList& msgs);
+    void fillMessageList(MessageList& msgs);
 
        /// Return the as_function with given name, converting case if needed
        boost::intrusive_ptr<as_function> getEventHandler(const std::string& 
name);
@@ -148,7 +148,7 @@
 }
 
 
-bool
+void
 XMLSocket_as::fillMessageList(MessageList& msgs)
 {
 
@@ -157,7 +157,7 @@
     if (fd <= 0) {
        log_error(_("%s: fd <= 0, returning false (timer not unregistered "
                "while socket disconnected?"), __FUNCTION__);
-        return false;
+        return;
     }
 
     fd_set                fdset;
@@ -174,45 +174,43 @@
         tval.tv_sec = 0;
         tval.tv_usec = 103;
         
-        int ret = select(fd + 1, &fdset, NULL, NULL, &tval);
+        const int ret = select(fd + 1, &fdset, NULL, NULL, &tval);
         
         // If interupted by a system call, try again
         if (ret == -1 && errno == EINTR) {
-            log_debug(_("The socket for fd #%d was interupted by a system 
call"),
-                    fd);
+            log_debug(_("The socket for fd #%d was interupted by a "
+                        "system call"), fd);
             continue;
         }
         if (ret == -1) {
             log_error(_("%s: The socket for fd #%d never was available"),
                 __FUNCTION__, fd);
-            return false;
-        }
-        if (ret == 0) {
-            //log_debug(_("%s: There is no data in the socket for fd #%d"),
-            //   __FUNCTION__, fd);
-            return false;
-        }
-        if (ret > 0) {
-            //log_debug(_("%s: There is data in the socket for fd #%d"),
-            //    __FUNCTION__, fd);
-        }
-
-        ret = read(_sockfd, buf.get(), bufSize - 1);
-        
-        if (buf[ret - 1] != 0)
+            return;
+        }
+ 
+        // Return if timed out.
+        if (ret == 0) return;
+
+        const size_t bytesRead = read(_sockfd, buf.get(), bufSize - 1);
+
+       // Return if there's no data.
+        if (!bytesRead) return;
+
+        if (buf[bytesRead - 1] != 0)
         {
             // We received a partial message, so bung
             // a null-terminator on the end.
-            buf[ret] = 0;
+            buf[bytesRead] = 0;
         }
 
         char* ptr = buf.get();
-        while (ptr - buf.get() < ret - 1)
+        while (ptr - buf.get() < bytesRead - 1)
         {
-            log_debug ("read: %d, this string ends: %d", ret, ptr + 
std::strlen(ptr) - buf.get());
+            log_debug ("read: %d, this string ends: %d",
+                           bytesRead, ptr + std::strlen(ptr) - buf.get());
             // If the string reaches to the final byte read, it's
             // incomplete. Store it and continue. 
-            if (ptr + std::strlen(ptr) - buf.get() == ret)
+            if (ptr + std::strlen(ptr) - buf.get() == bytesRead)
             {
                 log_debug ("Setting remainder");
                 _remainder += std::string(ptr);
@@ -231,11 +229,8 @@
             ptr += std::strlen(ptr) + 1;
         }
         
-        return true;
-        
     }
     
-    return true;
 }