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

[zou lunkai]

>      Allow for same action_buffer to be referenced by multiple swf_events (ie:
>       triggered by multiple clip events).

Not sure what this means. multiple clip events share the same
action_buffer?  Have you found any swf contains this case?  Or is it
theoretically possible according to the PlaceObject2 tag structure?

--zou


On Dec 11, 2007 11:33 PM, Sandro Santilli <address@hidden> wrote:
> 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);
>
>  };
>
>
>
> _______________________________________________
> Gnash-commit mailing list
> address@hidden
> http://lists.gnu.org/mailman/listinfo/gnash-commit
>