bugGnash - The GNU Flash player - Bugs: bug #24641, Illegal pointer cast (buffered AMF)

 
 

bug #24641: Illegal pointer cast (buffered AMF)

Submitter:  Sandro Santilli <strk>
Submitted:  Fri 24 Oct 2008 08:31:27 AM UTC
   
 
Category:  core Severity:  6 - Security
Release:  None Status:  Fixed
Privacy:  Public Assigned to:  strk
Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Tue 28 Oct 2008 06:08:30 PM UTC, comment #6: 

I tried out the patch on my target platform and it worked fine.  I didn't run the testsuite on the target, however.

Justin Thor Dick <i_am_thor>
Fri 24 Oct 2008 04:21:10 PM UTC, comment #5: 

More reasons why it's stupid to have multiple implementations of AMF0 reading code.

Rob Savoye <rsavoye>
Group administrator
Fri 24 Oct 2008 10:20:40 AM UTC, comment #4: 

Revision 10070 of trunk contains the correct version of my patch.
Justing, can you try that ?

Unfortunately there are not snapshot for that revision yet,
so you'll need to fetch it from bazaar, supposedly with:

 bzr branch http://bzr.savannah.gnu.org/r/gnash/trunk

Sandro Santilli <strk>
Group Member
Fri 24 Oct 2008 09:28:10 AM UTC, comment #3: 

Oops, forget that patch, I erroneously dropped a b+=8.
Or, add the b+=8 again after the std::copy yourself.

Sandro Santilli <strk>
Group Member
Fri 24 Oct 2008 09:22:31 AM UTC, comment #2: 

Justin, can you try the attached patch please ?
The best test would be a 'make check' after making
sure Ming was found at ./configure time.

In particular, the testsuite/misc-ming.all/SharedObjectTestRunner
is the one that should reproduce your failure with/out the patch and hopefully succeed with the patch.
The SharedObjectTestRunner file won't be built unless Ming
was found at configure time...

(file #16726)

Sandro Santilli <strk>
Group Member
Fri 24 Oct 2008 08:36:11 AM UTC, comment #1: 

Jason.. you may want to look at this, being your code :)

Sandro Santilli <strk>
Group Member
Fri 24 Oct 2008 08:31:27 AM UTC, original submission:  

amf0_read_value, in as_value.cpp does cast a char pointer
to a double one, which isn't safe:

        case amf::Element::NUMBER_AMF0:
        {  
 ...
            double dub = (reinterpret_cast<double>(b)); b += 8;
            amf::swapBytes(&dub, 8);

The cast, on systems with alignmnet constraints may result
in a pointer shift (ie: value of the double* != value of the char*).

A user reports getting a 'bus error' due to that.

What we should do instead is to read 4 bytes, one by one,
into memory specifically allocated for the double.

Sandro Santilli <strk>
Group Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #16726:  DoubleAlignment.patch added by strk (541B - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by i_am_thor (Posted a comment)
  • -email is unavailable- added by rsavoye (Posted a comment)
  • -email is unavailable- added by strk (reporter of the bug)
  • -email is unavailable- added by strk (author of that code)
  • -email is unavailable- added by strk (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2008-10-28 strk StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2008-10-24 strk StatusNone Ready For Test
        Assigned toNone strk
    2008-10-24 strk Attached File- Added DoubleAlignment.patch, #16726
        Carbon-Copy- Added -email is unavailable-
    2008-10-24 strk Carbon-Copy- Added -email is unavailable-

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code