[Gnash-commit] gnash ChangeLog server/swf_event.h server/swf/P...

[Sandro Santilli]

CVSROOT:        /sources/gnash
Module name:    gnash
Changes by:     Sandro Santilli <strk>  07/12/11 15:33:11

Modified files:
        .              : ChangeLog 
        server         : swf_event.h 
        server/swf     : PlaceObject2Tag.cpp PlaceObject2Tag.h 

Log message:
        Allow for same action_buffer to be referenced by multiple swf_events 
(ie:
        triggered by multiple clip events). This fixes security bugs when 
multiple
        events are listed for a PlaceObject action. Note that we have no 
automated
        test for this. There are a lot of testcases which may host this 
additional
        one, but I'm not sure we want to squat or make a new one. Zou ?

CVSWeb URLs:
http://cvs.savannah.gnu.org/viewcvs/gnash/ChangeLog?cvsroot=gnash&r1=1.5132&r2=1.5133
http://cvs.savannah.gnu.org/viewcvs/gnash/server/swf_event.h?cvsroot=gnash&r1=1.8&r2=1.9
http://cvs.savannah.gnu.org/viewcvs/gnash/server/swf/PlaceObject2Tag.cpp?cvsroot=gnash&r1=1.24&r2=1.25
http://cvs.savannah.gnu.org/viewcvs/gnash/server/swf/PlaceObject2Tag.h?cvsroot=gnash&r1=1.14&r2=1.15

Patches:
Index: ChangeLog
===================================================================
RCS file: /sources/gnash/gnash/ChangeLog,v
retrieving revision 1.5132
retrieving revision 1.5133
diff -u -b -r1.5132 -r1.5133
--- ChangeLog   11 Dec 2007 15:13:27 -0000      1.5132
+++ ChangeLog   11 Dec 2007 15:33:10 -0000      1.5133
@@ -1,5 +1,13 @@
 2007-12-11 Sandro Santilli <address@hidden>
 
+       * server/swf_event.h: leave ownership of action_buffer to the 
+         caller as a single action buffer can be triggered by multiple
+         system events.
+       * server/swf/PlaceObject2Tag.{cpp,h}: keep ownership of event
+         action buffers, split PlaceObject2 and PlaceObject3 parser.
+
+2007-12-11 Sandro Santilli <address@hidden>
+
        * server/swf/DisplayListTag.h: streamline included headers and
          forward declarations.
        * server/impl.cpp (create_jpeg_movie): fix log message format.

Index: server/swf_event.h
===================================================================
RCS file: /sources/gnash/gnash/server/swf_event.h,v
retrieving revision 1.8
retrieving revision 1.9
diff -u -b -r1.8 -r1.9
--- server/swf_event.h  30 Oct 2007 18:55:43 -0000      1.8
+++ server/swf_event.h  11 Dec 2007 15:33:10 -0000      1.9
@@ -1,3 +1,4 @@
+// swf_event.h -- clip events (PlaceObject-defined)
 // 
 //   Copyright (C) 2005, 2006, 2007 Free Software Foundation, Inc.
 // 
@@ -42,20 +43,23 @@
 {
 public:
 
-       swf_event(const event_id& ev, std::auto_ptr<action_buffer> buf)
+       swf_event(const event_id& ev, action_buffer& buf)
                :
                m_event(ev),
                m_action_buffer(buf)
        {
        }
 
-       swf_event()
+       swf_event(const swf_event& o)
+               :
+               m_event(o.m_event),
+               m_action_buffer(o.m_action_buffer)
        {
        }
 
        const action_buffer& action()
        {
-               return *m_action_buffer;
+               return m_action_buffer;
        }
 
        event_id& event()
@@ -65,15 +69,18 @@
 
 private:
 
-    // NOTE: DO NOT USE THESE AS VALUE TYPES IN AN
-    // std::vector<>!  They cannot be moved!  The private
-    // operator=(const swf_event&) should help guard
-    // against that.
-
+       /// System event id
     event_id   m_event;
-    std::auto_ptr<action_buffer> m_action_buffer;
-    // DON'T USE THESE
-    swf_event(const swf_event& /*s*/) { abort(); }
+
+       /// Action buffer associated with this event
+       //
+       /// The buffer is externally owned
+       /// (by PlaceObject tag in this design)
+       /// and may be shared between multiple swf_events
+       ///
+       action_buffer& m_action_buffer;
+
+       /// Can't assign to an swf_event
     void       operator=(const swf_event& /*s*/) { abort(); }
 };
 

Index: server/swf/PlaceObject2Tag.cpp
===================================================================
RCS file: /sources/gnash/gnash/server/swf/PlaceObject2Tag.cpp,v
retrieving revision 1.24
retrieving revision 1.25
diff -u -b -r1.24 -r1.25
--- server/swf/PlaceObject2Tag.cpp      4 Dec 2007 11:45:33 -0000       1.24
+++ server/swf/PlaceObject2Tag.cpp      11 Dec 2007 15:33:10 -0000      1.25
@@ -17,7 +17,7 @@
 // Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
 //
 
-/* $Id: PlaceObject2Tag.cpp,v 1.24 2007/12/04 11:45:33 strk Exp $ */
+/* $Id: PlaceObject2Tag.cpp,v 1.25 2007/12/11 15:33:10 strk Exp $ */
 
 #ifdef HAVE_CONFIG_H
 #include "config.h"
@@ -117,7 +117,8 @@
                }
 
                // Read the actions for event(s)
-               std::auto_ptr<action_buffer> action(new action_buffer);
+               action_buffer* action = new action_buffer();
+               _actionBuffers.push_back(action); // take ownership
                action->read(in);
 
                size_t readlen = action->size();
@@ -191,12 +192,15 @@
                        );
                }
 
+               // Aah! same action for multiple events !
                for (int i = 0, mask = 1; i < total_known_events; i++, mask <<= 
1)
                {
                        if (flags & mask)
                        {
-                               std::auto_ptr<swf_event> ev ( new 
swf_event(s_code_bits[i], action) );
-                               //log_action("---- actions for event %s", 
ev->event().get_function_name().c_str());
+                               std::auto_ptr<swf_event> ev ( new 
swf_event(s_code_bits[i], *action) );
+                               IF_VERBOSE_PARSE (
+                               log_parse("---- actions for event %s", 
ev->event().get_function_name().c_str());
+                               );
 
                                if (i == 17)    // has KeyPress event
                                {
@@ -211,7 +215,97 @@
 
 // read SWF::PLACEOBJECT2
 void
-PlaceObject2Tag::readPlaceObject2(stream* in, int movie_version, bool place_2)
+PlaceObject2Tag::readPlaceObject2(stream* in, int movie_version)
+{
+       in->align();
+
+       bool    has_actions = in->read_bit(); 
+       bool    has_clip_bracket = in->read_bit(); 
+       bool    has_name = in->read_bit();
+       bool    has_ratio = in->read_bit();
+       bool    has_cxform = in->read_bit();
+       bool    has_matrix = in->read_bit(); 
+       bool    has_char = in->read_bit(); 
+       bool    flag_move = in->read_bit(); 
+
+       m_depth = in->read_u16()+character::staticDepthOffset;
+
+       if (has_char) m_character_id = in->read_u16();
+
+       if (has_matrix)
+       {
+               m_has_matrix = true;
+               m_matrix.read(in);
+       }
+
+       if (has_cxform)
+       {
+               m_has_cxform = true;
+               m_color_transform.read_rgba(in);
+       }
+
+       if (has_ratio) 
+               m_ratio = in->read_u16();
+       else
+               m_ratio = character::noRatioValue;
+
+       if (has_name) m_name = in->read_string();
+
+       if (has_clip_bracket)
+               m_clip_depth = in->read_u16()+character::staticDepthOffset;
+       else
+               m_clip_depth = character::noClipDepthValue;
+
+       if (has_actions)
+       {
+               readPlaceActions(in, movie_version);
+       }
+
+       if (has_char == true && flag_move == true)
+       {
+               // Remove whatever's at m_depth, and put m_character there.
+               m_place_type = REPLACE;
+       }
+       else if (has_char == false && flag_move == true)
+       {
+               // Moves the object at m_depth to the new location.
+               m_place_type = MOVE;
+       }
+       else if (has_char == true && flag_move == false)
+       {
+               // Put m_character at m_depth.
+               m_place_type = PLACE;
+       }
+        else if (has_char == false && flag_move == false)
+        {
+             m_place_type = REMOVE;
+        }
+
+       IF_VERBOSE_PARSE (
+               log_parse(_("  PLACEOBJECT2: depth = %d (%d)"), m_depth, 
m_depth-character::staticDepthOffset);
+               if ( has_char ) log_parse(_("  char id = %d"), m_character_id);
+               if ( has_matrix )
+               {
+                       log_parse(_("  mat:"));
+                       m_matrix.print();
+               }
+               if ( has_cxform )
+               {
+                       log_parse(_("  cxform:"));
+                       m_color_transform.print();
+               }
+               if ( has_ratio ) log_parse(_("  ratio: %d"), m_ratio);
+               if ( has_name ) log_parse(_("  name = %s"), m_name ? m_name : 
"<null>");
+               if ( has_clip_bracket ) log_parse(_("  clip_depth = %d (%d)"), 
m_clip_depth, m_clip_depth-character::staticDepthOffset);
+               log_parse(_(" m_place_type: %d"), m_place_type);
+       );
+
+       //log_msg("place object at depth %i", m_depth);
+}
+
+// read SWF::PLACEOBJECT4
+void
+PlaceObject2Tag::readPlaceObject3(stream* in, int movie_version)
 {
        in->align();
 
@@ -220,6 +314,7 @@
         bool has_bitmap_caching = false;
         bool has_blend_mode = false;
         bool has_filters = false;
+       std::string className;
 
        bool    has_actions = in->read_bit(); 
        bool    has_clip_bracket = in->read_bit(); 
@@ -230,17 +325,26 @@
        bool    has_char = in->read_bit(); 
        bool    flag_move = in->read_bit(); 
 
-        if (!place_2 && movie_version >= 8)
-        {
-            in->read_uint(5); // Ignore on purpose.
+       in->align();
+        in->read_uint(3); // Ignore on purpose.
+       bool    hasImage = in->read_bit();
+       bool    hasClassName = in->read_bit();
             has_bitmap_caching = in->read_bit(); 
             has_blend_mode = in->read_bit(); 
             has_filters = in->read_bit(); 
-        }
 
        m_depth = in->read_u16()+character::staticDepthOffset;
 
-       if (has_char) m_character_id = in->read_u16();
+       if (has_char)
+       {
+               m_character_id = in->read_u16();
+       }
+
+       if (hasClassName || (hasImage && has_char) )
+       {
+               log_unimpl("PLACEOBJECT3 with associated class name");
+               in->read_string(className);
+       }
 
        if (has_matrix)
        {
@@ -310,7 +414,7 @@
         }
 
        IF_VERBOSE_PARSE (
-               log_parse(_("  PLACEOBJECT2: depth = %d (%d)"), m_depth, 
m_depth-character::staticDepthOffset);
+               log_parse(_("  PLACEOBJECT3: depth = %d (%d)"), m_depth, 
m_depth-character::staticDepthOffset);
                if ( has_char ) log_parse(_("  char id = %d"), m_character_id);
                if ( has_matrix )
                {
@@ -324,6 +428,7 @@
                }
                if ( has_ratio ) log_parse(_("  ratio: %d"), m_ratio);
                if ( has_name ) log_parse(_("  name = %s"), m_name ? m_name : 
"<null>");
+               if ( hasClassName ) log_parse(_("  class name = %s"), 
className.c_str());
                if ( has_clip_bracket ) log_parse(_("  clip_depth = %d (%d)"), 
m_clip_depth, m_clip_depth-character::staticDepthOffset);
                log_parse(_(" m_place_type: %d"), m_place_type);
        );
@@ -341,9 +446,13 @@
        {
                readPlaceObject(in);
        }
+       else if ( tag == SWF::PLACEOBJECT2 )
+       {
+               readPlaceObject2(in, movie_version);
+       }
        else
        {
-               readPlaceObject2(in, movie_version, tag == SWF::PLACEOBJECT3 ? 
false : true);
+               readPlaceObject3(in, movie_version);
        }
 }
 
@@ -393,11 +502,18 @@
 PlaceObject2Tag::~PlaceObject2Tag()
 {
        delete [] m_name;
+
        m_name = NULL;
+
        for(size_t i=0; i<m_event_handlers.size(); ++i)
        {
                delete m_event_handlers[i];
        }
+
+       for(size_t i=0; i<_actionBuffers.size(); ++i)
+       {
+               delete _actionBuffers[i];
+       }
 }
 
 /* public static */

Index: server/swf/PlaceObject2Tag.h
===================================================================
RCS file: /sources/gnash/gnash/server/swf/PlaceObject2Tag.h,v
retrieving revision 1.14
retrieving revision 1.15
diff -u -b -r1.14 -r1.15
--- server/swf/PlaceObject2Tag.h        4 Dec 2007 11:45:33 -0000       1.14
+++ server/swf/PlaceObject2Tag.h        11 Dec 2007 15:33:10 -0000      1.15
@@ -19,7 +19,7 @@
 //
 //
 
-/* $Id: PlaceObject2Tag.h,v 1.14 2007/12/04 11:45:33 strk Exp $ */
+/* $Id: PlaceObject2Tag.h,v 1.15 2007/12/11 15:33:10 strk Exp $ */
 
 #ifndef GNASH_SWF_PLACEOBJECT2TAG_H
 #define GNASH_SWF_PLACEOBJECT2TAG_H
@@ -40,6 +40,7 @@
        class stream;
        class sprite_instance;
        class swf_event;
+       class action_buffer;
        class movie_definition;
 }
 
@@ -61,6 +62,7 @@
 {
 public:
 
+       typedef std::vector<action_buffer*> ActionBuffers;
        typedef std::vector<swf_event*> EventHandlers;
 
        PlaceObject2Tag(const movie_definition& def)
@@ -126,6 +128,8 @@
 
        const movie_definition& _movie_def;
 
+       ActionBuffers _actionBuffers;
+
        EventHandlers m_event_handlers;
 
        // read SWF::PLACEOBJECT 
@@ -134,8 +138,11 @@
        // read placeObject2 actions
        void readPlaceActions(stream* in, int movie_version);
 
-       // read SWF::PLACEOBJECT2 or SWF::PLACEOBJECT3
-       void readPlaceObject2(stream* in, int movie_version, bool place_2);
+       // read SWF::PLACEOBJECT2 
+       void readPlaceObject2(stream* in, int movie_version);
+
+       // read SWF::PLACEOBJECT3
+       void readPlaceObject3(stream* in, int movie_version);
 
 };