[Gnash-commit] gnash ChangeLog libamf/amf.cpp libamf/amf.h lib...

[Sandro Santilli]

CVSROOT:        /sources/gnash
Module name:    gnash
Changes by:     Sandro Santilli <strk>  08/01/31 11:25:57

Modified files:
        .              : ChangeLog 
        libamf         : amf.cpp amf.h sol.cpp sol.h 

Log message:
        cleanups, const-correctness, memory boundary checking.

CVSWeb URLs:
http://cvs.savannah.gnu.org/viewcvs/gnash/ChangeLog?cvsroot=gnash&r1=1.5533&r2=1.5534
http://cvs.savannah.gnu.org/viewcvs/gnash/libamf/amf.cpp?cvsroot=gnash&r1=1.57&r2=1.58
http://cvs.savannah.gnu.org/viewcvs/gnash/libamf/amf.h?cvsroot=gnash&r1=1.29&r2=1.30
http://cvs.savannah.gnu.org/viewcvs/gnash/libamf/sol.cpp?cvsroot=gnash&r1=1.18&r2=1.19
http://cvs.savannah.gnu.org/viewcvs/gnash/libamf/sol.h?cvsroot=gnash&r1=1.8&r2=1.9

Patches:
Index: ChangeLog
===================================================================
RCS file: /sources/gnash/gnash/ChangeLog,v
retrieving revision 1.5533
retrieving revision 1.5534
diff -u -b -r1.5533 -r1.5534
--- ChangeLog   31 Jan 2008 11:08:55 -0000      1.5533
+++ ChangeLog   31 Jan 2008 11:25:56 -0000      1.5534
@@ -1,3 +1,12 @@
+2008-01-31 Sandro Santilli <address@hidden>
+
+       * libamf/amf.{cpp,h}: have encodeVariable take an output parameter
+         to return size of the serialized buffer; perform boundary checking
+         before any memcpy.
+       * libamf/sol.{cpp,h}: drop unneeded write interfaces, const-correct
+         the others; perform boundary checking before memcopies.
+         Turns the memory corruption in bug #22188 to an assertion failure.
+
 2008-01-31 Benjamin Wolsey <address@hidden>
 
        * gui/gtk{sup.h,.cpp}: add another rcfile option (insecureSSL).

Index: libamf/amf.cpp
===================================================================
RCS file: /sources/gnash/gnash/libamf/amf.cpp,v
retrieving revision 1.57
retrieving revision 1.58
diff -u -b -r1.57 -r1.58
--- libamf/amf.cpp      25 Jan 2008 18:07:59 -0000      1.57
+++ libamf/amf.cpp      31 Jan 2008 11:25:57 -0000      1.58
@@ -1073,22 +1073,27 @@
 #endif
 
 uint8_t *
-AMF::encodeVariable(amf::Element *el)
+AMF::encodeVariable(amf::Element *el, size_t& outsize)
 {
     GNASH_REPORT_FUNCTION;
-    int outsize = el->getName().size() + el->getLength() + 5;
-    boost::uint8_t *out = new boost::uint8_t[outsize + 4];
-    memset(out, 0, outsize + 2);
+    outsize = el->getName().size() + el->getLength() + 5; // why +5 here ?
+
+    boost::uint8_t *out = new boost::uint8_t[outsize + 4]; // why +4 here ?
+    boost::uint8_t *end = out + outsize+4; // why +4 ?
+
+    memset(out, 0, outsize + 2); // why +2 here ?
     boost::uint8_t *tmpptr = out;
 
     // Add the length of the string for the name of the variable
     size_t length = el->getName().size();
     boost::uint16_t enclength = length;
     swapBytes(&enclength, 2);
+    assert(tmpptr+2 < end);
     memcpy(tmpptr, &enclength, 2);
 
     // Add the actual name
     tmpptr += sizeof(uint16_t);
+    assert(tmpptr+length < end);
     memcpy(tmpptr, el->getName().c_str(), length);
     tmpptr += length;
     // Add the type of the variable's data
@@ -1098,21 +1103,25 @@
     switch (el->getType()) {
       case Element::BOOLEAN:
          enclength = el->to_bool();
+          assert(tmpptr+2 < end);
          memcpy(tmpptr, &enclength, 2);
          tmpptr += sizeof(uint16_t);
          break;
       case Element::NUMBER:
          if (el->getData()) {
              swapBytes(el->getData(), AMF_NUMBER_SIZE);
+              assert(tmpptr+AMF_NUMBER_SIZE < end);
              memcpy(tmpptr, el->getData(), AMF_NUMBER_SIZE);
          }
          break;
       default:
          enclength = el->getLength();
          swapBytes(&enclength, 2);
+          assert(tmpptr+2 < end);
          memcpy(tmpptr, &enclength, 2);
          tmpptr += sizeof(uint16_t);
          // Now the data for the variable
+          assert(tmpptr+el->getLength() < end);
          memcpy(tmpptr, el->getData(), el->getLength());
     }
     

Index: libamf/amf.h
===================================================================
RCS file: /sources/gnash/gnash/libamf/amf.h,v
retrieving revision 1.29
retrieving revision 1.30
diff -u -b -r1.29 -r1.30
--- libamf/amf.h        21 Jan 2008 22:55:26 -0000      1.29
+++ libamf/amf.h        31 Jan 2008 11:25:57 -0000      1.30
@@ -246,13 +246,16 @@
     boost::uint8_t *encodeElement(amf::Element *el);
 
     /// Encode a variable. 
+    //
+    /// @param el The element to encode, ownership retained by caller
+    ///
+    /// @param size Output parameter: size of the encoded byte array.
     ///
     /// @return a binary AMF packet in big endian format (header,data)
-
-    /// @return a newly allocated byte array.
+    ///         in form of a newly allocated byte array.
     /// to be deleted by caller using delete [] operator, or NULL
     ///
-    boost::uint8_t *encodeVariable(amf::Element *el);
+    boost::uint8_t *encodeVariable(amf::Element *el, size_t& outsize);
 
 #if 0
     /// Encode an element

Index: libamf/sol.cpp
===================================================================
RCS file: /sources/gnash/gnash/libamf/sol.cpp,v
retrieving revision 1.18
retrieving revision 1.19
diff -u -b -r1.18 -r1.19
--- libamf/sol.cpp      31 Jan 2008 10:27:06 -0000      1.18
+++ libamf/sol.cpp      31 Jan 2008 11:25:57 -0000      1.19
@@ -106,7 +106,7 @@
 }
 
 bool
-SOL::formatHeader(vector<unsigned char> & /*data*/)
+SOL::formatHeader(const vector<unsigned char> & /*data*/)
 {
 //    GNASH_REPORT_FUNCTION;
       return false;
@@ -114,13 +114,13 @@
 
 // name is the object name
 bool
-SOL::formatHeader(std::string &name)
+SOL::formatHeader(const std::string &name)
 {
     return formatHeader(name, _filesize);
 }
 
 bool
-SOL::formatHeader(std::string &name, int filesize)
+SOL::formatHeader(const std::string &name, int filesize)
 {
 //    GNASH_REPORT_FUNCTION;
     boost::uint32_t i;
@@ -200,24 +200,7 @@
 // write the data to disk as a .sol file
 
 bool
-SOL::writeFile(string &filespec, const char *name)
-{
-//    GNASH_REPORT_FUNCTION;
-    string str = name;
-    return writeFile(filespec, str);
-}
-
-bool
-SOL::writeFile(const char *filespec, const char *name)
-{
-//    GNASH_REPORT_FUNCTION;
-    string str1 = filespec;
-    string str2 = name;
-    return writeFile(str1, str2);
-}
-
-bool
-SOL::writeFile(string &filespec, string &name)
+SOL::writeFile(const string &filespec, const string &name)
 {
 //    GNASH_REPORT_FUNCTION;
     ofstream ofs(filespec.c_str(), ios::binary);
@@ -227,7 +210,7 @@
     char *ptr;
     int size = 0;
     
-    if (filespec.size() == 0) {
+    if (filespec.empty()) {
        return false;
     }
 
@@ -240,26 +223,35 @@
     boost::scoped_array<char> body ( new char[size + 20] );
     memset(body.get(), 0, size);
     ptr = body.get();
+    char* endPtr = ptr+size+20; // that's the amount we allocated..
 
     for (ita = _amfobjs.begin(); ita != _amfobjs.end(); ita++) {
         amf::Element *el = (*(ita));
-        int outsize = el->getName().size() + el->getLength() + 5;
-        boost::uint8_t *foo = amf_obj.encodeVariable(el); 
+        size_t outsize;
+        boost::uint8_t *foo = amf_obj.encodeVariable(el, outsize); 
+        if ( ! foo )
+        {
+             continue;
+        }
+        assert(outsize);
         switch (el->getType()) {
          case Element::BOOLEAN:
-             outsize = el->getName().size() + 5;
+             //outsize = el->getName().size() + 5;
+              assert(ptr+outsize < endPtr);
              memcpy(ptr, foo, outsize);
              ptr += outsize;
              break;
          case Element::OBJECT:
-             outsize = el->getName().size() + 5;
+             //outsize = el->getName().size() + 5;
+              assert(ptr+outsize < endPtr);
              memcpy(ptr, foo, outsize);
              ptr += outsize;
              *ptr++ = Element::OBJECT_END;
              *ptr++ = 0;       // objects are terminated too!
              break;
          case Element::NUMBER:
-             outsize = el->getName().size() + AMF_NUMBER_SIZE + 2;
+             //outsize = el->getName().size() + AMF_NUMBER_SIZE + 2;
+              assert(ptr+outsize < endPtr);
              memcpy(ptr, foo, outsize);
              ptr += outsize;
              *ptr++ = 0;       // doubles are terminated too!
@@ -267,22 +259,23 @@
              break;
          case Element::STRING:
              if (el->getLength() == 0) {
+                 assert(ptr+outsize+1 < endPtr);
                  memcpy(ptr, foo, outsize+1);
                  ptr += outsize+1;
              } else {          // null terminate the string
+                  assert(ptr+outsize < endPtr);
                  memcpy(ptr, foo, outsize);
                  ptr += outsize;
                  *ptr++ = 0;
              }
              break;
          default:
+              assert(ptr+outsize < endPtr);
              memcpy(ptr, foo, outsize);
              ptr += outsize;
        }
-       if (foo) {
            delete[] foo;
        }
-    }
     
     _filesize = ptr - body.get();
     int len = name.size() + sizeof(uint16_t) + 16;

Index: libamf/sol.h
===================================================================
RCS file: /sources/gnash/gnash/libamf/sol.h,v
retrieving revision 1.8
retrieving revision 1.9
diff -u -b -r1.8 -r1.9
--- libamf/sol.h        31 Jan 2008 10:27:06 -0000      1.8
+++ libamf/sol.h        31 Jan 2008 11:25:57 -0000      1.9
@@ -52,15 +52,13 @@
     bool extractHeader(const std::string &filespec);
 
     // Create the header
-    bool formatHeader(std::vector<boost::uint8_t> &data);
-    bool formatHeader(std::string &name);
-    bool formatHeader(std::string &name, int filesize);
+    bool formatHeader(const std::vector<boost::uint8_t> &data);
+    bool formatHeader(const std::string &name);
+    bool formatHeader(const std::string &name, int filesize);
 
     // write the data to disk as a .sol file
     bool writeFile();
-    bool writeFile(std::string &filespec, std::string &objname);
-    bool writeFile(std::string &filespec, const char *objname);
-    bool writeFile(const char *filespec, const char *objname);
+    bool writeFile(const std::string &filespec, const std::string &objname);
     
     // read the .sol file from disk
     bool readFile(std::string &filespec);