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

 
 

bug #24641: Illegal pointer cast (buffered AMF)

Submitted by:  Sandro Santilli <strk>
Submitted on:  Fri 24 Oct 2008 08:31:27 AM UTC  
 
Category: coreSeverity: 6 - Security
Release: NoneStatus: Fixed
Privacy: PublicAssigned to: Sandro Santilli <strk>
Open/Closed: Closed

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission 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>
Project 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>
Project MemberIn charge of this item.
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>
Project MemberIn charge of this item.
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>
Project MemberIn charge of this item.
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>
Project MemberIn charge of this item.
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>
Project MemberIn charge of this item.

 

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

Attach File(s):
   
   
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
  • -unavailable- added by i_am_thor (Posted a comment)
  • -unavailable- added by rsavoye (Posted a comment)
  • -unavailable- added by strk (reporter of the bug)
  • -unavailable- added by strk (author of that code)
  • -unavailable- added by strk (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 7 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Tue 28 Oct 2008 06:20:17 PM UTCstrkStatusReady For Test=>Fixed
      Open/ClosedOpen=>Closed
    Fri 24 Oct 2008 09:22:42 AM UTCstrkStatusNone=>Ready For Test
      Assigned toNone=>strk
    Fri 24 Oct 2008 09:22:31 AM UTCstrkAttached File-=>Added DoubleAlignment.patch, #16726
      Carbon-Copy-=>Added -unavailable-
    Fri 24 Oct 2008 08:36:11 AM UTCstrkCarbon-Copy-=>Added -unavailable-

    Back to the top


    Powered by Savane 3.1-cleanup1