gnash-commit
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Gnash-commit] gnash ChangeLog server/stream.cpp server/stream...


From: Benjamin Wolsey
Subject: [Gnash-commit] gnash ChangeLog server/stream.cpp server/stream...
Date: Tue, 06 May 2008 16:15:59 +0000

CVSROOT:        /sources/gnash
Module name:    gnash
Changes by:     Benjamin Wolsey <bwy>   08/05/06 16:15:59

Modified files:
        .              : ChangeLog 
        server         : stream.cpp stream.h 
        server/parser  : action_buffer.cpp 
        server/swf     : tag_loaders.cpp 

Log message:
                * server/swf/tag_loaders.cpp: ensureBytes - doesn't fix any 
known
                  bug, but is good for robustness.
                * server/stream.{h,cpp}: move ensureBytes implementation to
                  stream.cpp to make changing it less painful. Throw parser
                  exception when seeking fails. Stops reading off the end of
                  the stream, which was happening on certain malformed SWFs.
                  Clean up logging. Use ensureBytes for open_tag too.
                * server/parser/action_buffer.cpp: typo.

CVSWeb URLs:
http://cvs.savannah.gnu.org/viewcvs/gnash/ChangeLog?cvsroot=gnash&r1=1.6529&r2=1.6530
http://cvs.savannah.gnu.org/viewcvs/gnash/server/stream.cpp?cvsroot=gnash&r1=1.50&r2=1.51
http://cvs.savannah.gnu.org/viewcvs/gnash/server/stream.h?cvsroot=gnash&r1=1.43&r2=1.44
http://cvs.savannah.gnu.org/viewcvs/gnash/server/parser/action_buffer.cpp?cvsroot=gnash&r1=1.38&r2=1.39
http://cvs.savannah.gnu.org/viewcvs/gnash/server/swf/tag_loaders.cpp?cvsroot=gnash&r1=1.196&r2=1.197

Patches:
Index: ChangeLog
===================================================================
RCS file: /sources/gnash/gnash/ChangeLog,v
retrieving revision 1.6529
retrieving revision 1.6530
diff -u -b -r1.6529 -r1.6530
--- ChangeLog   6 May 2008 15:28:29 -0000       1.6529
+++ ChangeLog   6 May 2008 16:15:54 -0000       1.6530
@@ -1,3 +1,14 @@
+2008-05-06 Benjamin Wolsey <address@hidden>
+
+       * server/swf/tag_loaders.cpp: ensureBytes - doesn't fix any known
+         bug, but is good for robustness.
+       * server/stream.{h,cpp}: move ensureBytes implementation to
+         stream.cpp to make changing it less painful. Throw parser
+         exception when seeking fails. Stops reading off the end of
+         the stream, which was happening on certain malformed SWFs.
+         Clean up logging. Use ensureBytes for open_tag too.
+       * server/parser/action_buffer.cpp: typo.
+
 2008-05-06 Sandro Santilli <address@hidden>
 
        * server/vm/ASHandlers.cpp (ActionFscommand2): fix ensureStack

Index: server/stream.cpp
===================================================================
RCS file: /sources/gnash/gnash/server/stream.cpp,v
retrieving revision 1.50
retrieving revision 1.51
diff -u -b -r1.50 -r1.51
--- server/stream.cpp   15 Mar 2008 08:00:53 -0000      1.50
+++ server/stream.cpp   6 May 2008 16:15:56 -0000       1.51
@@ -45,6 +45,23 @@
 {
 }
 
+void
+stream::ensureBytes(unsigned long needed)
+{
+#ifndef GNASH_TRUST_SWF_INPUT
+
+    if ( _tagBoundsStack.empty() ) return; // not in a tag (should we check 
file length ?)
+
+    unsigned long int left = get_tag_end_position() - get_position();
+    if ( left < needed )
+    {
+           std::stringstream ss;
+           ss << "premature end of tag: need to read " << needed << " bytes, 
but only " << left << " left in this tag";
+           throw ParserException(ss.str());
+    }
+#endif
+}
+
 unsigned stream::read(char *buf, unsigned count)
 {
        align();
@@ -318,7 +335,9 @@
 #else
        using boost::uint32_t;
 
-       unsigned char _buf[2]; read((char*)_buf, 2); // would align
+       unsigned char _buf[2];
+       read((char*)_buf, 2); // would align
+       
        uint32_t result = _buf[0];
                 result |= (_buf[1] << 8);
 
@@ -399,7 +418,9 @@
 
 unsigned long stream::get_position()
 {
-       return m_input->get_position();
+       int pos = m_input->get_position();
+       // TODO: check return value? Could be negative.
+       return static_cast<unsigned long>(pos);
 }
 
 
@@ -451,12 +472,15 @@
 }
 
 
-SWF::tag_type stream::open_tag()
+SWF::tag_type
+stream::open_tag()
 {
        align();
 
        unsigned long tagStart = get_position();
 
+    ensureBytes(2);
+
        int     tagHeader = read_u16();
        int     tagType = tagHeader >> 6;
        int     tagLength = tagHeader & 0x3F;
@@ -464,6 +488,7 @@
                
        if (tagLength == 0x3F)
        {
+           ensureBytes(4);
                tagLength = read_u32();
        }
 
@@ -489,7 +514,7 @@
                std::stringstream ss;
                ss << "Invalid tag end position " << tagEnd << " advertised 
(tag length "
                        << tagLength << ").";
-               throw ParserException(ss.str().c_str());
+               throw ParserException(ss.str());
        }       
 
        if ( ! _tagBoundsStack.empty() )
@@ -499,14 +524,11 @@
                if ( tagEnd > containerTagEnd )
                {
                        unsigned long containerTagStart = 
_tagBoundsStack.back().first;
-                       std::stringstream ss;
-                       ss << "Tag " << tagType << " starting at offset " << 
tagStart
-                          << " is advertised to end at offset " << tagEnd
-                          << " which is after end of previously opened tag 
starting "
-                          << " at offset " << containerTagStart
-                          << " and ending at offset " << containerTagEnd << "."
-                          << " Making it end where container tag ends.";
-                       log_swferror("%s", ss.str().c_str());
+                       log_swferror(_("Tag %d starting at offset %d is 
advertised to end "
+                               "at offset %d, which is after end of previously 
opened "
+                               "tag starting at offset %d and ending at offset 
%d. "
+                               "Making it end where container tag ends."),
+                               tagType, tagStart, tagEnd, containerTagStart, 
containerTagEnd);
 
                        // what to do now ?
                        tagEnd = containerTagEnd;
@@ -527,7 +549,8 @@
 }
 
 
-void   stream::close_tag()
+void
+stream::close_tag()
 {
        assert(_tagBoundsStack.size() > 0);
        unsigned long end_pos = _tagBoundsStack.back().second;
@@ -535,7 +558,9 @@
 
        if ( m_input->set_position(end_pos) == TU_FILE_SEEK_ERROR )
        {
-               log_error("Could not seek to end position");
+        // We'll go on reading right past the end of the stream
+        // if we don't throw an exception.
+        throw ParserException(_("Could not seek to end position"));
        }
 
        m_unused_bits = 0;

Index: server/stream.h
===================================================================
RCS file: /sources/gnash/gnash/server/stream.h,v
retrieving revision 1.43
retrieving revision 1.44
diff -u -b -r1.43 -r1.44
--- server/stream.h     27 Mar 2008 10:50:15 -0000      1.43
+++ server/stream.h     6 May 2008 16:15:57 -0000       1.44
@@ -364,19 +364,7 @@
        ///
        /// NOTE: if GNASH_TRUST_SWF_INPUT is defined this function is a no-op 
        ///
-       void ensureBytes(unsigned long needed)
-       {
-#ifndef GNASH_TRUST_SWF_INPUT
-               if ( _tagBoundsStack.empty() ) return; // not in a tag (should 
we check file length ?)
-               unsigned long int left = get_tag_end_position() - 
get_position();
-               if ( left < needed )
-               {
-                       std::stringstream ss;
-                       ss << "premature end of tag: need to read " << needed 
<< " bytes, but only " << left << " left in this tag";
-                       throw ParserException(ss.str());
-               }
-#endif
-       }
+       void ensureBytes(unsigned long needed);
 
        /// \brief
        /// Ensure the requested number of bits are available for a bitwise read

Index: server/parser/action_buffer.cpp
===================================================================
RCS file: /sources/gnash/gnash/server/parser/action_buffer.cpp,v
retrieving revision 1.38
retrieving revision 1.39
diff -u -b -r1.38 -r1.39
--- server/parser/action_buffer.cpp     27 Mar 2008 13:41:26 -0000      1.38
+++ server/parser/action_buffer.cpp     6 May 2008 16:15:57 -0000       1.39
@@ -99,7 +99,7 @@
        IF_VERBOSE_MALFORMED_SWF(
        if ( m_buffer.back() != SWF::ACTION_END )
        {
-               log_swferror(_("Action buffer starting at offset %lu doesn't 
end witn an END tag"),
+               log_swferror(_("Action buffer starting at offset %lu doesn't 
end with an END tag"),
                        startPos);
        }
        );

Index: server/swf/tag_loaders.cpp
===================================================================
RCS file: /sources/gnash/gnash/server/swf/tag_loaders.cpp,v
retrieving revision 1.196
retrieving revision 1.197
diff -u -b -r1.196 -r1.197
--- server/swf/tag_loaders.cpp  30 Apr 2008 12:21:36 -0000      1.196
+++ server/swf/tag_loaders.cpp  6 May 2008 16:15:58 -0000       1.197
@@ -284,6 +284,7 @@
     assert(tag == SWF::DEFINEBITS); // 6
     assert(in);
 
+    in->ensureBytes(2);
     boost::uint16_t    character_id = in->read_u16();
 
     //
@@ -456,6 +457,7 @@
 {
     assert(tag == SWF::DEFINEBITSJPEG3); // 35
 
+    in->ensureBytes(2);
     boost::uint16_t    character_id = in->read_u16();
 
     IF_VERBOSE_PARSE
@@ -464,6 +466,7 @@
                  character_id, in->get_position());
     );
 
+    in->ensureBytes(4);
     boost::uint32_t    jpeg_size = in->read_u32();
     boost::uint32_t    alpha_position = in->get_position() + jpeg_size;
 
@@ -789,6 +792,7 @@
           || tag == SWF::DEFINESHAPE3
           || tag == SWF::DEFINESHAPE4 || tag == SWF::DEFINESHAPE4_);
 
+    in->ensureBytes(2);
     boost::uint16_t    character_id = in->read_u16();
     IF_VERBOSE_PARSE
     (
@@ -807,6 +811,7 @@
                || tag == SWF::DEFINEMORPHSHAPE2
                || tag == SWF::DEFINEMORPHSHAPE2_); 
 
+    in->ensureBytes(2);
     boost::uint16_t character_id = in->read_u16();
 
     IF_VERBOSE_PARSE
@@ -831,6 +836,7 @@
           || tag == SWF::DEFINEFONT2
           || tag == SWF::DEFINEFONT3 ); // 10 || 48 || 75
 
+    in->ensureBytes(2);
     boost::uint16_t font_id = in->read_u16();
 
     font* f = new font;
@@ -850,6 +856,7 @@
 {
     assert(tag == SWF::DEFINEFONTINFO || tag == SWF::DEFINEFONTINFO2);
 
+    in->ensureBytes(2);
     boost::uint16_t font_id = in->read_u16();
 
     font* f = m->get_font(font_id);
@@ -871,6 +878,7 @@
 {
     assert(tag == SWF::DEFINEFONTNAME);
 
+    in->ensureBytes(2);
     boost::uint16_t font_id = in->read_u16();
 
     font* f = m->get_font(font_id);
@@ -893,6 +901,7 @@
 {
     assert(tag == SWF::DEFINESPRITE); // 39 - DefineSprite
 
+    in->ensureBytes(2);
     int        character_id = in->read_u16();
 
     IF_VERBOSE_PARSE
@@ -935,6 +944,7 @@
 {
     assert(tag == SWF::DEFINEBUTTONSOUND); // 17
 
+    in->ensureBytes(2);
     int        button_character_id = in->read_u16();
     character_def* chdef = m->get_character_def(button_character_id);
     if ( ! chdef )
@@ -966,6 +976,7 @@
     // 7 || 34
     assert(tag == SWF::DEFINEBUTTON || tag == SWF::DEFINEBUTTON2);
 
+    in->ensureBytes(2);
     int        character_id = in->read_u16();
 
     IF_VERBOSE_PARSE
@@ -990,6 +1001,7 @@
 {
     assert(tag == SWF::EXPORTASSETS); // 56
 
+    in->ensureBytes(2);
     int        count = in->read_u16();
 
     IF_VERBOSE_PARSE
@@ -1011,6 +1023,7 @@
     // Read the exports.
     for (int i = 0; i < count; i++)
     {
+    in->ensureBytes(2);
        boost::uint16_t id = in->read_u16();
        std::string symbolName;
        in->read_string(symbolName);
@@ -1065,11 +1078,13 @@
 
     if ( tag == SWF::IMPORTASSETS2 )
     {
+    in->ensureBytes(2);
        import_version = in->read_uint(8);
        unsigned char reserved = in->read_uint(8);
        UNUSED(reserved);
     }
 
+    in->ensureBytes(2);
     int count = in->read_u16();
 
     IF_VERBOSE_PARSE
@@ -1110,6 +1125,7 @@
     // Get the imports.
     for (int i = 0; i < count; i++)
     {
+    in->ensureBytes(2);
        boost::uint16_t id = in->read_u16();
        std::string symbolName;
        in->read_string(symbolName);
@@ -1160,6 +1176,7 @@
 {
     assert(tag == SWF::DEFINEEDITTEXT); // 37
 
+    in->ensureBytes(2);
     boost::uint16_t    character_id = in->read_u16();
 
     edit_text_character_def* ch = new edit_text_character_def(m);
@@ -1178,6 +1195,7 @@
 {
     assert(tag == SWF::DEFINETEXT || tag == SWF::DEFINETEXT2);
 
+    in->ensureBytes(2);
     boost::uint16_t    character_id = in->read_u16();
 
     text_character_def* ch = new text_character_def(m);
@@ -1287,6 +1305,8 @@
            unsigned data_bytes = in->get_tag_end_position() - 
in->get_position();
            unsigned char *data = new unsigned char[data_bytes];
 
+        // data_bytes is already calculated from the end of the tag, which
+        // should be inside the end of the file. TODO: check that this is tha 
case.
            in->read((char*)data, data_bytes);
 
            // Store all the data in a SoundInfo object
@@ -1369,36 +1389,30 @@
 
     if ( playbackSoundRate != streamSoundRate )
     {
-       static bool warned = false;
-       if ( ! warned )
-       {
-               log_unimpl("Different stream/playback sound rate (%d/%d)."
-                       " This seems common in SWF files, so we'll warn only 
once.",
-                       streamSoundRate,playbackSoundRate);
-               warned=true;
-       }
+        LOG_ONCE(
+            log_unimpl(_("Different stream/playback sound rate (%d/%d). "
+                           "This seems common in SWF files, so we'll warn only 
once."),
+                           streamSoundRate,playbackSoundRate)
+        );
     }
+
     if ( playbackSound16bit != streamSound16bit )
     {
-       static bool warned = false;
-       if ( ! warned )
-       {
-               log_unimpl("Different stream/playback sample size (%d/%d)."
-                       " This seems common in SWF files, so we'll warn only 
once.",
-                       streamSound16bit?16:32, playbackSound16bit?16:32 );
-               warned=true;
-       }
+        LOG_ONCE(
+            log_unimpl(_("Different stream/playback sample size (%d/%d). "
+                       "This seems common in SWF files, so we'll warn only 
once."),
+                       streamSound16bit ? 16 : 32,
+                       playbackSound16bit ? 16 : 32 )
+           );
     }
     if ( playbackSoundStereo != streamSoundStereo )
     {
-       static bool warned = false;
-       if ( ! warned )
-       {
-               log_unimpl("Different stream/playback channels (%s/%s)."
-                       " This seems common in SWF files, so we'll warn only 
once.",
-                       streamSoundStereo?"stereo":"mono", 
playbackSoundStereo?"stereo":"mono" );
-               warned=true;
-       }
+        LOG_ONCE(
+            log_unimpl(_("Different stream/playback channels (%s/%s). "
+                       "This seems common in SWF files, so we'll warn only 
once."),
+                           streamSoundStereo ? "stereo" : "mono",
+                           playbackSoundStereo ? "stereo":"mono")
+        );
     }
 
     // checks if this is a new streams header or just one in the row
@@ -1455,6 +1469,8 @@
 define_video_loader(stream* in, tag_type tag, movie_definition* m)
 {
     assert(tag == SWF::DEFINEVIDEOSTREAM); // 60
+    
+    in->ensureBytes(2);
     boost::uint16_t character_id = in->read_u16();
 
     std::auto_ptr<video_stream_definition> chdef ( new 
video_stream_definition(character_id) );
@@ -1469,6 +1485,7 @@
 {
     assert(tag == SWF::VIDEOFRAME); // 61
 
+    in->ensureBytes(2);
     boost::uint16_t character_id = in->read_u16();
     character_def* chdef = m->get_character_def(character_id);
 
@@ -1508,6 +1525,7 @@
 
     file_attrs_flags flags;
 
+    in->ensureBytes(1 + 3);
     flags.reserved1 = in->read_uint(3);
     flags.has_metadata = in->read_bit(); 
     flags.reserved2 = in->read_uint(3);
@@ -1618,6 +1636,7 @@
        {
 
                // Skip the 'flags' until they are actually used.
+               in->ensureBytes(4);
                static_cast<void> (in->read_u32());
                std::string name;
                in->read_string(name);




reply via email to

[Prev in Thread] Current Thread [Next in Thread]