| | [ Return to Bugs & Features | Roadmap 2.0 | Post Text | Post File | SVN ⇄ GIT ]
STR #2296
| Application: | FLTK Library |
|---|
| Status: | 5 - New |
|---|
| Priority: | 4 - High, e.g. key functionality not working |
|---|
| Scope: | 3 - Applies to all machines and operating systems |
|---|
| Subsystem: | Unassigned |
|---|
| Summary: | segmentation fault when fetching jpeg images from data |
|---|
| Version: | 2.0-current |
|---|
| Created By: | bebopfreak |
|---|
| Assigned To: | Unassigned |
|---|
| Fix Version: | Unassigned |
|---|
| Update Notification: | |
|---|
Trouble Report Files:
[ Post File ]
Trouble Report Comments:
[ Post Text ]
| |
| #1 | bebopfreak
01:10 Dec 09, 2009 |
| fill_input_buffer in images/fl_jpeg.cxx segfaults (very seldom)
As there is no length check on the data, I get two error types with
valgrind:
- memcpy overlaps (might not be that serious)
- invalid read, which is sort of critical
One should check at least for EOI markers to avoid some error conditions.
(tested with i386 (Ubuntu) and arm (Nokia N800) systems) | |
| |
| #2 | bgbnbigben
00:44 Jul 12, 2010 |
| so i could force the first problem with memcpy overlapping - i cant remember how though. Whilst "it might be not that serious", as far as i'm aware memcpy overlapping has undefined behaviour, so it is a little serious. :P
If you could tell us how you managed to force the invalid read, that'd be good...... | |
| |
| #3 | matt
01:06 Jul 12, 2010 |
| memcpy() does not check for overlapping, however memmove() does. Simply replace the first with the second. | |
| |
| #4 | bgbnbigben
01:38 Jul 12, 2010 |
| yup, that's all the patch does.
It's probably a good idea to keep it here though. It's a simple fix for possibly undefined behavior, so it can't hurt. | |
| |
| #5 | bebopfreak
04:57 Jul 12, 2010 |
| I couldn't write a testprogram that would reproducibly yield the error, but the error occurred reliably after some time (lots of jpegs loaded from the internet and lots of memory management, altogether not very deterministic).
I added a modified version of fl_jpeg.cxx, which tries to compute the length of the jpeg data.
As a result, the errors were gone!
But this patch isn't safe either - the only way would be to know the length. | |
| |
| #6 | bgbnbigben
04:59 Jul 12, 2010 |
| From what I could tell, that patch saves the invalid read.
I think between your patch and mine, we should be about right. :P
Unfortunately, I don't know of a way to load the length of a jpeg file.
I'll post the svn diff of your file and mine together, and upload it again. Hopefully that'll be the last of this bug.
If you run across a file that forces an invalid read, be sure to send it along.
:) | |
| |
| #7 | bgbnbigben
05:02 Jul 12, 2010 |
| Out of curiosity, is there any need to keep the fprintf(stderr, ...) in your code, or can they be changed to xfprintf()s? | |
| |
| #8 | bebopfreak
05:18 Jul 12, 2010 |
| forgot about the fprintfs -- as they didn't fire any more | |
| |
| #9 | bgbnbigben
05:26 Jul 12, 2010 |
| Sure. I've left them in here, but if anyone wants to commit this to svn, feel free to throw out lines 120 to 133, and/or the xfprintf statements.
Is there any reason to keep this STR open now? | |
[ Return to Bugs & Features | Post Text | Post File ]
|
| |
