bugGnash - The GNU Flash player - Bugs: bug #40315, tummy_trouble.swf segfaults

 
 

bug #40315: tummy_trouble.swf segfaults

Submitter:  Sandro Santilli <strk>
Submitted:  Sun 20 Oct 2013 03:08:20 PM UTC
   
 
Category:  render-agg Severity:  6 - Security
Release:  None Status:  Fixed
Privacy:  Public Assigned to:  bjacques
Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Fri 18 Apr 2014 04:31:13 PM UTC, comment #11: 

So, after adding -fsanitize=address to the CXXFLAGS I can confirm that the DrawingApiTest.swf can crash gnash until fixed by your commit f84af61ed7e9.

Good work!

Sandro Santilli <strk>
Group Member
Fri 18 Apr 2014 04:04:49 PM UTC, comment #10: 

I cant' reproduce the tummy_trouble.swf crash anymore with master-21950-b4018a1

But I'm not sure if the testcase you added is really guarding against that crash because I can't get a crash playing DrawingTestApi.swf with master-21911-0df2a46, which instead crashes with tummy_trouble.swf

Sandro Santilli <strk>
Group Member
Fri 18 Apr 2014 12:27:18 AM UTC, comment #9: 

Fix attempt: f84af61ed7e9.

Bastiaan Jacques <bjacques>
Group Member
Thu 17 Apr 2014 11:06:38 PM UTC, comment #8: 

The source of the problem is in DynamicShape::finalize(). The current (temporary) subshape is copied into the shape, and then the subshape is cleared. However, _currpath, _currline and _currfill are not reset. Then, when later the DynamicShape is reused to draw a line without adding a new line style, _currline refers to a nonexistent line style.

Bastiaan Jacques <bjacques>
Group Member
Thu 16 Jan 2014 01:40:51 PM UTC, comment #7: 

The problem was introduced this summer (June) when adding subshape support, and was not present ion 0.8.10.  See also bug #39989 for a related problem introduced by the subshape support.

Petter Reinholdtsen <pere>
Group Member
Fri 10 Jan 2014 08:00:28 AM UTC, comment #6: 

I tested the tummy_trouble.swf flash with the current HEAD
(1eaf85f95a8aabb0275447df9500357081bd333a) using valgrind, and got
this crash after playing and entering and exiting rooms.

pere@minerva:/scratch/pere/src/gnash$ ./gui/gnash tummy_trouble.swf
==25928== Memcheck, a memory error detector
==25928== Copyright (C) 2002-2011, and GNU GPL'd, by Julian Seward et al.
==25928== Using Valgrind-3.7.0 and LibVEX; rerun with -h for copyright info
==25928== Command: /scratch/pere/src/gnash/gui/.libs/lt-gtk-gnash tummy_trouble.swf
==25928==

* (lt-gtk-gnash:25928): WARNING *: Couldn't find pixmap file: GnashG.png
==25928== Invalid read of size 2
==25928==    at 0x4EF6924: gnash::DynamicShape::lineTo(int, int, int) (DynamicShape.cpp:207)
==25928==    by 0x4FC7228: gnash::(anonymous namespace)::movieclip_lineTo(gnash::fn_call const&) (MovieClip_as.cpp:1459)
==25928==    by 0x4FA3EC4: gnash::NativeFunction::call(gnash::fn_call const&) (NativeFunction.h:65)
==25928==    by 0x506F307: gnash::(anonymous namespace)::ActionCallMethod(gnash::ActionExec&) (ASHandlers.cpp:2811)
==25928==    by 0x50651E9: gnash::SWF::SWFHandlers::execute(gnash::SWF::ActionType, gnash::ActionExec&) const (ASHandlers.cpp:432)
==25928==    by 0x5078064: gnash::ActionExec::operator()() (ActionExec.cpp:260)
==25928==    by 0x4F5F609: gnash::Function2::call(gnash::fn_call const&) (Function2.cpp:219)
==25928==    by 0x506F307: gnash::(anonymous namespace)::ActionCallMethod(gnash::ActionExec&) (ASHandlers.cpp:2811)
==25928==    by 0x50651E9: gnash::SWF::SWFHandlers::execute(gnash::SWF::ActionType, gnash::ActionExec&) const (ASHandlers.cpp:432)
==25928==    by 0x5078064: gnash::ActionExec::operator()() (ActionExec.cpp:260)
==25928==    by 0x4F5EE49: gnash::Function::call(gnash::fn_call const&) (Function.cpp:145)
==25928==    by 0x4EDB936: gnash::invoke(gnash::as_value const&, gnash::as_environment const&, gnash::as_object*, gnash::FunctionArgs<gnash::as_value>&, gnash::as_object*, gnash::movie_definition const*) (Global_as.h:185)
==25928==  Address 0x18435814 is not stack'd, malloc'd or (recently) free'd
==25928==
==25928== Invalid read of size 1
==25928==    at 0x548D069: gnash::Renderer_agg<agg::pixfmt_alpha_blend_rgba<agg::blender_rgba_pre<agg::rgba8, agg::order_bgra>, agg::row_accessor<unsigned char>, unsigned int> >::buildPaths_rounded(std::vector<agg::path_base<agg::vertex_block_storage<double, 8u, 256u> >, std::allocator<agg::path_base<agg::vertex_block_storage<double, 8u, 256u> > > >&, std::vector<gnash::Path, std::allocator<gnash::Path> > const&, std::vector<gnash::LineStyle, std::allocator<gnash::LineStyle> > const&) (Renderer_agg.cpp:2208)
==25928==    by 0x54C9687: gnash::Renderer_agg<agg::pixfmt_alpha_blend_rgba<agg::blender_rgba_pre<agg::rgba8, agg::order_bgra>, agg::row_accessor<unsigned char>, unsigned int> >::drawShape(std::vector<gnash::FillStyle, std::allocator<gnash::FillStyle> > const&, std::vector<gnash::LineStyle, std::allocator<gnash::LineStyle> > const&, std::vector<gnash::Path, std::allocator<gnash::Path> > const&, gnash::SWFMatrix const&, gnash::SWFCxForm const&) (Renderer_agg.cpp:1180)
==25928==    by 0x54CFE4A: gnash::Renderer_agg<agg::pixfmt_alpha_blend_rgba<agg::blender_rgba_pre<agg::rgba8, agg::order_bgra>, agg::row_accessor<unsigned char>, unsigned int> >::drawShape(gnash::SWF::ShapeRecord const&, gnash::Transform const&) (Renderer_agg.cpp:1141)
==25928==    by 0x4F2E068: gnash::MovieClip::draw(gnash::Renderer&, gnash::Transform const&) (MovieClip.cpp:1201)
==25928==    by 0x4F2E111: gnash::MovieClip::display(gnash::Renderer&, gnash::Transform const&) (MovieClip.cpp:1216)
==25928==    by 0x4F66662: gnash::DisplayList::display(gnash::Renderer&, gnash::Transform const&) (DisplayList.cpp:593)
==25928==    by 0x4F2E07A: gnash::MovieClip::draw(gnash::Renderer&, gnash::Transform const&) (MovieClip.cpp:1202)
==25928==    by 0x4F2E111: gnash::MovieClip::display(gnash::Renderer&, gnash::Transform const&) (MovieClip.cpp:1216)
==25928==    by 0x4F66662: gnash::DisplayList::display(gnash::Renderer&, gnash::Transform const&) (DisplayList.cpp:593)
==25928==    by 0x4F2E07A: gnash::MovieClip::draw(gnash::Renderer&, gnash::Transform const&) (MovieClip.cpp:1202)
==25928==    by 0x4F2E111: gnash::MovieClip::display(gnash::Renderer&, gnash::Transform const&) (MovieClip.cpp:1216)
==25928==    by 0x4F66662: gnash::DisplayList::display(gnash::Renderer&, gnash::Transform const&) (DisplayList.cpp:593)
==25928==  Address 0x8 is not stack'd, malloc'd or (recently) free'd
==25928==
==25928==
==25928== Process terminating with default action of signal 11 (SIGSEGV)
==25928==  Access not within mapped region at address 0x8
==25928==    at 0x548D069: gnash::Renderer_agg<agg::pixfmt_alpha_blend_rgba<agg::blender_rgba_pre<agg::rgba8, agg::order_bgra>, agg::row_accessor<unsigned char>, unsigned int> >::buildPaths_rounded(std::vector<agg::path_base<agg::vertex_block_storage<double, 8u, 256u> >, std::allocator<agg::path_base<agg::vertex_block_storage<double, 8u, 256u> > > >&, std::vector<gnash::Path, std::allocator<gnash::Path> > const&, std::vector<gnash::LineStyle, std::allocator<gnash::LineStyle> > const&) (Renderer_agg.cpp:2208)
==25928==    by 0x54C9687: gnash::Renderer_agg<agg::pixfmt_alpha_blend_rgba<agg::blender_rgba_pre<agg::rgba8, agg::order_bgra>, agg::row_accessor<unsigned char>, unsigned int> >::drawShape(std::vector<gnash::FillStyle, std::allocator<gnash::FillStyle> > const&, std::vector<gnash::LineStyle, std::allocator<gnash::LineStyle> > const&, std::vector<gnash::Path, std::allocator<gnash::Path> > const&, gnash::SWFMatrix const&, gnash::SWFCxForm const&) (Renderer_agg.cpp:1180)
==25928==    by 0x54CFE4A: gnash::Renderer_agg<agg::pixfmt_alpha_blend_rgba<agg::blender_rgba_pre<agg::rgba8, agg::order_bgra>, agg::row_accessor<unsigned char>, unsigned int> >::drawShape(gnash::SWF::ShapeRecord const&, gnash::Transform const&) (Renderer_agg.cpp:1141)
==25928==    by 0x4F2E068: gnash::MovieClip::draw(gnash::Renderer&, gnash::Transform const&) (MovieClip.cpp:1201)
==25928==    by 0x4F2E111: gnash::MovieClip::display(gnash::Renderer&, gnash::Transform const&) (MovieClip.cpp:1216)
==25928==    by 0x4F66662: gnash::DisplayList::display(gnash::Renderer&, gnash::Transform const&) (DisplayList.cpp:593)
==25928==    by 0x4F2E07A: gnash::MovieClip::draw(gnash::Renderer&, gnash::Transform const&) (MovieClip.cpp:1202)
==25928==    by 0x4F2E111: gnash::MovieClip::display(gnash::Renderer&, gnash::Transform const&) (MovieClip.cpp:1216)
==25928==    by 0x4F66662: gnash::DisplayList::display(gnash::Renderer&, gnash::Transform const&) (DisplayList.cpp:593)
==25928==    by 0x4F2E07A: gnash::MovieClip::draw(gnash::Renderer&, gnash::Transform const&) (MovieClip.cpp:1202)
==25928==    by 0x4F2E111: gnash::MovieClip::display(gnash::Renderer&, gnash::Transform const&) (MovieClip.cpp:1216)
==25928==    by 0x4F66662: gnash::DisplayList::display(gnash::Renderer&, gnash::Transform const&) (DisplayList.cpp:593)
==25928==  If you believe this happened as a result of a stack
==25928==  overflow in your program's main thread (unlikely but
==25928==  possible), you can try to increase the size of the
==25928==  main thread stack using the --main-stacksize= flag.
==25928==  The main thread stack size used in this run was 8388608.
==25928==
==25928== HEAP SUMMARY:
==25928==     in use at exit: 16,224,662 bytes in 109,400 blocks
==25928==   total heap usage: 3,900,169 allocs, 3,790,769 frees, 2,374,121,314 bytes allocated
==25928==
==25928== LEAK SUMMARY:
==25928==    definitely lost: 20,736 bytes in 8 blocks
==25928==    indirectly lost: 11,392 bytes in 354 blocks
==25928==      possibly lost: 3,059,977 bytes in 25,543 blocks
==25928==    still reachable: 13,132,557 bytes in 83,495 blocks
==25928==         suppressed: 0 bytes in 0 blocks
==25928== Rerun with --leak-check=full to see details of leaked memory
==25928==
==25928== For counts of detected and suppressed errors, rerun with: -v
==25928== ERROR SUMMARY: 4 errors from 2 contexts (suppressed: 75 from 10)
Killed

The crash is due to the line_styles vector passed to the
buildPaths_rounded() function being empty when its first element is
accessed.  The first invalid read happen just before the crash, after
playing a bit and moving the player into the next room, back to the
initial room and re-entering the next room again.

Perhaps an assert like this is better than the crash to document the
problem better?

--- a/librender/agg/Renderer_agg.cpp
+++ b/librender/agg/Renderer_agg.cpp
@@ -1265,6 +1265,7 @@ public:
       bool hinting=false, closed=false, hairline=false;
      
       if (this_path.m_line) {
+        assert(line_styles.size() > 0);
         const LineStyle& lstyle = line_styles[this_path.m_line-1];
        
         hinting = lstyle.doPixelHinting();


This change get rid of the invalid read of 2 bytes.  It seem to be
caused by the DynamicShape code assuming all subshapes have a list of
line styles, which is false when re-entering the second room.  The
crash still happen after this change.

--- a/libcore/DynamicShape.cpp
+++ b/libcore/DynamicShape.cpp
@@ -204,7 +204,7 @@ DynamicShape::lineTo(boost::int32_t x, boost::int32_t y, in
     SWFRect bounds = _shape.getBounds();
 
        unsigned thickness = _currline ?
-        _currsubshape.lineStyles().back().getThickness() : 0;
+         (_currsubshape.lineStyles().size() > 0 ? _currsubshape.lineStyles().b
 
        if (_currpath->size() == 1) {
                _currpath->expandBounds(bounds, thickness, swfVersion);

--
Happy hacking
Petter Reinholdtsen

Petter Reinholdtsen <pere>
Group Member
Sun 20 Oct 2013 06:59:48 PM UTC, comment #5: 

The first bad commit could be any of:
22eac818afa08d15c55ae03091ea5a8d576dea20
31ce0628c76253052579065a0c2bf944357452b1
368216f46f76b7efeeafcf84d061857277ab7bcd
73da624b50cf46cba878143d7a2acf6f8b8de981
1fe4c1d27afd9fa97741df52c543382d94bc9be2
We cannot bisect more!

Sandro Santilli <strk>
Group Member
Sun 20 Oct 2013 05:26:48 PM UTC, comment #4: 

# good: [51959a760e5ca622f88373f5476031d75bfb8a3e] Tidy up.
# skip: [368216f46f76b7efeeafcf84d061857277ab7bcd] Update
# bad: [439df46da3c8c037e34e6762f3751f4fc94dff42] cppcheck: remove useless #if which breaks {} pair.

Sandro Santilli <strk>
Group Member
Sun 20 Oct 2013 05:18:19 PM UTC, comment #3: 

# bad: [ee880c6bfc28b71e0a760c6d484acba30524636f] cppcheck:
# good: [51959a760e5ca622f88373f5476031d75bfb8a3e] Tidy up.

Sandro Santilli <strk>
Group Member
Sun 20 Oct 2013 03:41:03 PM UTC, comment #2: 

Just tried 0.8.10 and can confirm there's no bug there.
To reproduce with master: start playing, enter one room and exit again.

Sandro Santilli <strk>
Group Member
Sun 20 Oct 2013 03:09:19 PM UTC, comment #1: 
Sandro Santilli <strk>
Group Member
Sun 20 Oct 2013 03:08:20 PM UTC, original submission:  

Sounds like an infinite recursion. This movie (7db88c39605740feeb9a0890f8669886 tummy_truble.swf) used to work just fine some time ago (0.8.10 ?). Now (master, 0d46c8830264ba86abb9041cff04297e02968516) it segfaults:

Program terminated with signal 11, Segmentation fault.
#0  operator[] (__n=0, this=<optimized out>) at /usr/src/gnash/gnash-head/librender/agg/Renderer_agg.cpp:2207
2207    } // end of namespace gnash
(gdb)
(gdb) bt
#0  operator[] (__n=0, this=<optimized out>) at /usr/src/gnash/gnash-head/librender/agg/Renderer_agg.cpp:2207
#1  gnash::Renderer_agg<agg::pixfmt_alpha_blend_rgba<agg::blender_rgba_pre<agg::rgba8, agg::order_bgra>, agg::row_accessor<unsigned char>, unsigned int> >::buildPaths_rounded (this=this@entry=0x7f3e6b748540, dest=..., paths=..., line_styles=...)
    at /usr/src/gnash/gnash-head/librender/agg/Renderer_agg.cpp:1268
#2  0x00007f3e69b6cdd8 in gnash::Renderer_agg<agg::pixfmt_alpha_blend_rgba<agg::blender_rgba_pre<agg::rgba8, agg::order_bgra>, agg::row_accessor<unsigned char>, unsigned int> >::drawShape (this=this@entry=0x7f3e6b748540, FillStyles=..., line_styles=..., objpaths=..., mat=..., cx=...)
    at /usr/src/gnash/gnash-head/librender/agg/Renderer_agg.cpp:1180
#3  0x00007f3e69b7349b in gnash::Renderer_agg<agg::pixfmt_alpha_blend_rgba<agg::blender_rgba_pre<agg::rgba8, agg::order_bgra>, agg::row_accessor<unsigned char>, unsigned int> >::drawShape (this=0x7f3e6b748540, shape=..., xform=...)
    at /usr/src/gnash/gnash-head/librender/agg/Renderer_agg.cpp:1141
#4  0x00007f3e69eafbc9 in gnash::MovieClip::draw (this=this@entry=0x7f3e6b98a9c0, renderer=..., xform=...)
    at /usr/src/gnash/gnash-head/libcore/MovieClip.cpp:1201
#5  0x00007f3e69eafc72 in gnash::MovieClip::display (this=0x7f3e6b98a9c0, renderer=..., base=...)
    at /usr/src/gnash/gnash-head/libcore/MovieClip.cpp:1216
#6  0x00007f3e69ee82e3 in gnash::DisplayList::display (this=this@entry=0x7f3e6b98a438, renderer=..., base=...)
    at /usr/src/gnash/gnash-head/libcore/DisplayList.cpp:593
#7  0x00007f3e69eafbdb in gnash::MovieClip::draw (this=this@entry=0x7f3e6b98a320, renderer=..., xform=...)
    at /usr/src/gnash/gnash-head/libcore/MovieClip.cpp:1202
#8  0x00007f3e69eafc72 in gnash::MovieClip::display (this=0x7f3e6b98a320, renderer=..., base=...)
    at /usr/src/gnash/gnash-head/libcore/MovieClip.cpp:1216
#9  0x00007f3e69ee82e3 in gnash::DisplayList::display (this=this@entry=0x7f3e6b86dd98, renderer=..., base=...)
    at /usr/src/gnash/gnash-head/libcore/DisplayList.cpp:593
#10 0x00007f3e69eafbdb in gnash::MovieClip::draw (this=this@entry=0x7f3e6b86dc80, renderer=..., xform=...)
    at /usr/src/gnash/gnash-head/libcore/MovieClip.cpp:1202
#11 0x00007f3e69eafc72 in gnash::MovieClip::display (this=0x7f3e6b86dc80, renderer=..., base=...)
    at /usr/src/gnash/gnash-head/libcore/MovieClip.cpp:1216

Sandro Santilli <strk>
Group Member

 

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

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by pere (Posted a comment)
  • -email is unavailable- added by bjacques (Updated the item)
  • -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 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2014-04-18 strk StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2014-04-18 bjacques StatusNone Ready For Test
    2013-10-20 strk Assigned toNone bjacques
    2013-10-20 strk Assigned tobjacques None
    2013-10-20 bjacques Assigned toNone bjacques

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code