Bug 39361

Summary:	Numerical overflow in libopenjpeg JPXStream::doLookChar()
Product:	poppler	Reporter:	Daniel Glöckner <daniel-gl>
Component:	general	Assignee:	poppler-bugs <poppler-bugs>
Status:	RESOLVED FIXED	QA Contact:
Severity:	normal
Priority:	medium
Version:	unspecified
Hardware:	All
OS:	All
Whiteboard:
i915 platform:		i915 features:
Attachments:	Fix and optimization Testcase Fix and optimization

Description Daniel Glöckner 2011-07-18 16:27:10 UTC

There is a line

unsigned char rc = (unsigned char) ((r >> adjust)+((r >> (adjust-1))%2));

in that function which overflows a component's value if (r >> (adjust - 1)) == 511.
It is also wrong to execute this at all if adjust == 0.

According to oprofile a lot of time is spent in this function.
See the attached patch for my try to fix and optimize this a little.

Comment 1 Daniel Glöckner 2011-07-18 16:28:08 UTC

Created attachment 49280 [details] [review]
Fix and optimization

Comment 2 Albert Astals Cid 2011-07-19 02:38:02 UTC

please attach a file that shows the need for the patch.

Comment 3 Daniel Glöckner 2011-07-19 03:56:59 UTC

Created attachment 49294 [details]
Testcase

Comment 4 Albert Astals Cid 2011-07-20 09:20:34 UTC

Unfortunately your patch breaks rendering of page 17 of http://www.investis.com/bby/investors/reports/2008rep/anreview08/anreveview2008.pdf

Please have a look and try to fix it, otherwise your patch can not be applied.

Comment 5 Daniel Glöckner 2011-07-20 15:38:16 UTC

Created attachment 49356 [details] [review]
Fix and optimization

I was under the impression that the code did not support JPEG 2000 images with more than three components because of the logic in JPXStream::getImageParams.
It apparently does when the image dictionary overrides the values returned by that method, so interleaving all components into the first component's buffer is out of question. It's still faster than before.

Comment 6 Albert Astals Cid 2011-07-23 10:49:34 UTC

Commited. Thanks

Comment 7 madbiologist 2011-08-19 21:38:39 UTC

Thanks Albert and Daniel.  Does this fix also apply to bug 33280 ?

Comment 8 Albert Astals Cid 2011-08-20 06:20:46 UTC

No

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.