Apache OpenOffice (AOO) Bugzilla – Issue 124661
crash when loading and re-saving attached ppt file with a single customshape
Last modified: 2017-05-20 10:35:42 UTC
Created attachment 83172 [details] minimized exaple file causing the crash To create the crash: Load attached ppt and save as (to be able to reuse the file) also as ppt -> crash Reason is that the contained single customshape seems to have a wrong definition or is wrongly imported (not sure yet). The formula contains references to object 196 (?196), but has only 150 definition lines. This leads to mem accesses out of bound in ConvertEnhancedCustomShapeEquation. A first change to avoid that already survives the crash, but does not yet identify the root of the problem. Nonetheless, out of bound accesses are always good to avoid.
First is to avoid out of bound access, adding that...
Checking the bounds and not accessing when out of bounds makes the save survive and looks good in ppt. Added an assertion, too. Preparing commit of this 1st change. This is not yet fixed, but without the crash afetr that. Keep open due to identifying the cause of this is needed.
"alg" committed SVN revision 1586681 into trunk: i124661 secure possible out of bound access to stl vector content
Found a related issue: https://issues.apache.org/ooo/show_bug.cgi?id=112309
I found Symphony made a partial fix on custom shape for PPT compatibility issue before 2009. But I cannot get more information from the defect DB or related developers. Keep going.
Hi Steve, thanks for the info. Hopefully you can find something, help always appreciated. For reference, I checked with the minimal file and after re-export to ppt and opening in MS ppt 2003 it looks okay, I can see no obvious deformations/errors.
@Armin: The exported file by AOO cannot show in MSO 2010. I found another root cause. The number of equations (pGuides_complex) in the sample file is 137. This value should not be exceed 128. I think the sample file should be deemed with a bad equations array and should not apply these equations to the shape when importing. I tested on my local machine. The exporting file can be opened by MSO 2010 normally.
Add the submit info here. Revision: 1587496 Author: steve_y Date: Tuesday, April 15, 2014 6:12:14 PM Message: Issue 124661 - crash when loading and re-saving attached ppt file with a single customshape check the equation array element number. If the number is greater than 128, the equation array will not be imported. ---- Modified : /openoffice/trunk/main/filter/source/msfilter/msdffimp.cxx
"steve_y" committed SVN revision 1587496 into trunk: Issue 124661 - crash when loading and re-saving attached ppt file with a sing...
Hi steve, thanks for the fix. But why '128' ? Is there really a fixed limit for PPT CustomShape formula equations? Shouldn't it be compared with the (hopefully known) number of entries in the current formula, at least when that number is less than 128...?
Hi Armin. This limit is from [MS-ODRAW] (Office Drawing Binary File Format Structure Specification). In its chapter 2.3.6.27, pGuides_complex. It said that "pGuides_complex (variable): An IMsoArray record of SG records that specifies a set of values that are used to define the geometry of this shape. This array MUST NOT have more than 128 elements." Maybe 128 equations for one custom shape is enough? And I didn't see MSO 2010 applies the equations to the shape, I think it may ignore the equations in some way when importing the sample file and keep them when saving.
Hi Steve, thanks for the infos. I am surprised (well, not too much) that it was defined to be limited from the start. What I wanted to point at is that comparing to that is okay, but when only 20 entries are used, one referring 21 is already wrong, too (AFAIK). Would it not be better to compare to the minimum of the limit of 128 and the (hopefully known) number of entries in the current formula list in the fix?
Hi Armin. Yes, agree with you.
"steve_y" committed SVN revision 1587823 into trunk: Issue 124661 - crash when loading and re-saving attached ppt file with a sing...
(a) already Reproducible with AOO 4.0.0 (b) Still worked without crash with * server installation of "AOO 3.4.1 – German UI / German locale [AOO341m1(Build:9593) - Rev.1372282]" on German WIN7 Home Premium (64bit)", own separate user profile * sever installation of "OOo 3.3.0 English UI / German locale [OOO330m20 (Build 9567)]" on WIN7 Home Premium (64bit) DE
Hi Steve, sorry for the hassle, I mixed up things. I thought you already found the place where the import of the malicious shape takes place, and there it would be possible to know the number of lines in the formula and to correct against that. Please ignore comment 12 and thanks for still enhancing the current fix!
Hi Armin. You're welcome. I did not find the place in AOO source code. I am very suspicious of the sample file which may contains some incorrect equations and the total number of its equations is also exceeding the limit. So I think it should be simply dropped. (In reply to Armin Le Grand from comment #16) > Hi Steve, sorry for the hassle, I mixed up things. I thought you already > found the place where the import of the malicious shape takes place, and > there it would be possible to know the number of lines in the formula and to > correct against that. Please ignore comment 12 and thanks for still > enhancing the current fix!
To resolved, crash is fixed. Additional thanks to Steve Yin!