After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 530852 - Impress backend crashes on most ODP files
Impress backend crashes on most ODP files
Status: RESOLVED FIXED
Product: evince
Classification: Core
Component: backends
git master
Other Linux
: Normal major
: ---
Assigned To: Evince Maintainers
Evince Maintainers
Depends on:
Blocks:
 
 
Reported: 2008-05-01 07:55 UTC by Hans Petter Jansson
Modified: 2008-05-01 08:54 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
evince-bnc379750-impress-parser-crashes.patch (924 bytes, patch)
2008-05-01 07:57 UTC, Hans Petter Jansson
accepted-commit_now Details | Review

Description Hans Petter Jansson 2008-05-01 07:55:22 UTC
There's an array overrun bug in the Impress backend that causes it to crash on most ODP files I and other openSUSE users have tried it with. This is fairly critical, as it causes Nautilus to crash when opening the "Properties..." dialog on these files. I have a patch for this bug.

If you decide to accept the patch, it'd be nice if it were also applied to the 2.22 branch.
Comment 1 Hans Petter Jansson 2008-05-01 07:57:40 UTC
Created attachment 110212 [details] [review]
evince-bnc379750-impress-parser-crashes.patch

Patch that fixes the bug. In addition to the array overrun, there's a free() in there that should probably be iks_free(). I've fixed that as well.
Comment 2 Hans Petter Jansson 2008-05-01 08:10:25 UTC
The openSUSE bug, with a trace, is at: https://bugzilla.novell.com/show_bug.cgi?id=379750 .

Problem in detail:

When in the initial attribute state, C_ATTRIBUTE, the parser checks that (prs->attcur < prs->attmax * 2), and if not, expands the array in order to accept more attributes. At this point, prs->attcur is divisible by two, so there is space for at least two more items in the array.

The next state is C_ATTRIBUTE_1, which can go on to C_VALUE, which can lead to C_VALUE_APOS or C_VALUE_QUOT. The last two possible states (of which we visit one), increment prs->attcur by two after the attribute's data is stored in the array. So far so good.

However, the next state is C_ATTRIBUTE_2, which checks if we're at the end of the tag, and if so, terminates the array with a NULL pointer. This means that at most, the array gets three items added to it (two for the attribute, and one for the terminating NULL). But the check in C_ATTRIBUTE only guarantees that we have at minimum two free slots! Thus, if we have exactly two free slots left in the array AND we find another attribute AND it's the last attribute in the tag, we overflow the array.

Since the array is expanded by 12 elements at a time, this means that any tag whose number of attributes is greater than 0 and divisible by 6 will crash the parser.
Comment 3 Hans Petter Jansson 2008-05-01 08:44:55 UTC
Committed to trunk and gnome-2-22.
Comment 4 Nickolay V. Shmyrev 2008-05-01 08:54:07 UTC
Thanks a lot!