Re: [Gnash-commit] gnash/libamf README

[strk]

On Wed, Apr 30, 2008 at 01:32:16PM +0000, Rob Savoye wrote:

> The AMF class is used to encode and decode data between Buffers and
> Elements.
> When encoding, all the methods are static, as no data needs
> to be retained between usages of the data.

Question1: Is the non-static 'Buffer* encodeProperty(Element*)'
           an exception ? 

> All decoding is handled by the non static
> AMF::extract*{} methods. These are not static as they must retain the
> current amount of data that has been parsed so subsequent decoding
> starts in the right place.

Question2: I see extract* methods take either a Buffer pointer or a
           Network::byte_t pointer. While the Buffer class provides
           a size() method, there's no way to know the size of memory
           safe-to-read from the byte_t pointer. How can the extract 
           method prevent reading past the allocated (or initialized)
           memory ?

Note that valgrind report access to uninitialized memory in
AMF::extractProperty(Network::byte_t *in) here, while playing
the SharedObject-v#.swf file:

==14604== Conditional jump or move depends on uninitialised value(s)
==14604==    at 0x4A1DA30: amf::AMF::extractProperty(unsigned char*) 
(amf.cpp:866)

I can see how that is possible as the line 866 reads:

    if (type != Element::TYPED_OBJECT_AMF0) {

With 'type' being assigned to on line 864:

    Element::amf0_type_e type = static_cast<Element::amf0_type_e>(c);

And 'c' being assigned to on line 863:

    char c = *(reinterpret_cast<char *>(tmpptr));

And 'tmpptr' being a pointer into the given argument, possibly
incremented w/out boundary checking on line 859:

        tmpptr += length;

and line 841:

    tmpptr += sizeof(boost::uint16_t);

--strk;