Bug 106137 - Crash on PDF with wrong attachment
Summary: Crash on PDF with wrong attachment
Status: RESOLVED MOVED
Alias: None
Product: poppler
Classification: Unclassified
Component: general (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: poppler-bugs
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-04-19 15:34 UTC by Marek Kasik
Modified: 2018-08-20 22:13 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
reproducer - null object instead of embedded file stream (38.37 KB, application/pdf)
2018-04-19 15:34 UTC, Marek Kasik
Details
reproducer - missing reference to a embedded file stream (102.12 KB, application/pdf)
2018-04-19 15:35 UTC, Marek Kasik
Details
Patch fixing this issue (3.25 KB, patch)
2018-04-19 15:37 UTC, Marek Kasik
Details | Splinter Review
Patch fixing this issue - modified (2.75 KB, patch)
2018-04-20 09:51 UTC, Marek Kasik
Details | Splinter Review

Description Marek Kasik 2018-04-19 15:34:58 UTC
Created attachment 138926 [details]
reproducer - null object instead of embedded file stream

Hi,

poppler crashes on a file with embedded file which has null object instead of embedded file stream or when even reference to a stream is missing.

Originally reported here:
https://bugzilla.redhat.com/show_bug.cgi?id=1569334
https://bugzilla.gnome.org/show_bug.cgi?id=795368

Related to:
https://bugs.freedesktop.org/show_bug.cgi?id=10386
Comment 1 Marek Kasik 2018-04-19 15:35:32 UTC
Created attachment 138927 [details]
reproducer - missing reference to a embedded file stream
Comment 2 Marek Kasik 2018-04-19 15:37:01 UTC
Created attachment 138928 [details] [review]
Patch fixing this issue
Comment 3 Albert Astals Cid 2018-04-19 20:11:45 UTC
Why are you adding the FileSpec.cc code? the EmbFile constructor already checks for the object being an stream and EmbFile has an isOk you should be using. 

I'd prefer not to change that since your change would actually most probably break the qt5 frontend code.
Comment 4 Marek Kasik 2018-04-20 09:51:45 UTC
Created attachment 138946 [details] [review]
Patch fixing this issue - modified

(In reply to Albert Astals Cid from comment #3)
> Why are you adding the FileSpec.cc code? the EmbFile constructor already
> checks for the object being an stream and EmbFile has an isOk you should be
> using. 

It is for the case when the referenced object is null object and not a stream. I've modified the patch so that the object is tested for being a stream in glib frontend and does not modify FileSpec.cc at all.

Or we can alternatively fetch the object in constructor of FileSpec and check for being stream so that isOk() covers also this case.

> I'd prefer not to change that since your change would actually most probably
> break the qt5 frontend code.
Comment 5 Albert Astals Cid 2018-04-22 17:57:16 UTC
Looking at 

EmbFile *FileSpec::getEmbeddedFile()

it should never return null if you've checked for isOk before, maybe that's what it's missing? but i guess it's similar to what you're doing now.

I'll let the glib/ experts comment :)
Comment 6 GitLab Migration User 2018-08-20 22:13:20 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/poppler/poppler/issues/236.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.