Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Power of two length on broken file leads to infinite loop #130

Closed
mike-hart opened this issue Dec 15, 2016 · 3 comments
Closed

Power of two length on broken file leads to infinite loop #130

mike-hart opened this issue Dec 15, 2016 · 3 comments

Comments

@mike-hart
Copy link

I was using Perl's Image::TestJPG::testJPG (using libjpeg-turbo) and tested a file (attached) which caused the process to hang.

Stepping through the Perl process using gdb shows the issue to occur in the consume_markers function with the given file.

(gdb) bt
#0  0x00007f47effbd496 in fill_input_buffer (cinfo=cinfo@entry=0x7ffea95e4d20) at mydatasrc.c:121
#1  0x00007f47effbd4eb in skip_input_data (cinfo=0x7ffea95e4d20, num_bytes=2) at mydatasrc.c:148
#2  0x00007f47efd8564f in skip_variable (cinfo=0x7ffea95e4d20) at jdmarker.c:871
#3  0x00007f47efd863c5 in read_markers (cinfo=0x7ffea95e4d20) at jdmarker.c:1072
#4  0x00007f47efd8448a in consume_markers (cinfo=0x7ffea95e4d20) at jdinput.c:313
#5  0x00007f47efd7ecf7 in jpeg_consume_input (cinfo=0x7ffea95e4d20) at jdapimin.c:301
#6  0x00007f47efd7efc3 in jpeg_read_header (cinfo=cinfo@entry=0x7ffea95e4d20, require_image=require_image@entry=1) at jdapimin.c:249
#7  0x00007f47effbd07e in XS_Image__TestJPG_testJPG (my_perl=<optimized out>, cv=0x472fab8) at TestJPG.xs:60
#8  0x00007f47fb637ee7 in Perl_pp_entersub () from /usr/lib64/perl5/CORE/libperl.so
#9  0x00007f47fb630776 in Perl_runops_standard () from /usr/lib64/perl5/CORE/libperl.so
#10 0x00007f47fb5ce3f6 in perl_run () from /usr/lib64/perl5/CORE/libperl.so
#11 0x0000000000400d69 in ?? ()
#12 0x00007f47fa24db15 in __libc_start_main () from /lib64/libc.so.6
#13 0x0000000000400da1 in ?? ()

Repeating c (continue), ctrl-c, bt (backtrace) brings up the same kind of result each time. As the Image::TestJPG XS only calls jpeg_read_header once, and not in a loop, and jpeg_read_header contains no loops itself, we eventually find consume_markers as a possible culprit.

Trimming the file from 65536 bytes to 65535, or adding a NUL to bump the file-size to 65537 bytes causes the process to complete, rather than hang. I have repeated the experiment on 32k, 16k files, etc. It doesn't appear to be a multiple-of-8kB or anything like that, just pure powers of two (also tested on 49152 bytes for example which didn't cause the issue).

I only had up to 1.4.2 to test against, so apologies if this has been fixed since, but I didn't see anything in the commit-log when I looked.
17093_5070_img_7

@dcommander
Copy link
Member

Is this reproducible with only libjpeg-turbo? I cannot personally reproduce it with djpeg, and thus I have no way of knowing whether this is our bug or a bug in the Perl code.

@mike-hart
Copy link
Author

The C code used by the Perl library appears simple, but perhaps they have misused the library. I cannot comment on whether that's the case, it's a short file available here: https://st.aticpan.org/source/JHUDGE/Image-TestJPG-1.0/TestJPG.xs

The most minimal Perl code I could use to test the issue is:

use Image::TestJPG;

open my $fh, '<', "17093_5070_IMG_7.jpg";
$/ = undef;
my $file_data = <$fh>;
close $fh;
my $ok = Image::TestJPG::testJPG($file_data, length($file_data));
print(( $ok ? 'OK' : 'Not OK' ) . "\n");

You would need to cpan Image::TestJPG before you can run the above code.

My apologies for making it slightly less easy to duplicate than you might hope. We process millions of files and this is the first we've had this issue with. I don't have sufficient experience with C myself to get a simple test together in a short time period.

If this is a problem with the Perl module then you have my apologies, it did however seem to be circling around within libjpeg-turbo though and not the Perl module (the stack frame for the call into libjpeg was the same each time I interrupted it).

@dcommander
Copy link
Member

The libjpeg-turbo decompressor receives a great deal of scrutiny, particularly from browser developers like Mozilla and Google. If this was truly an issue with the library, then I find it very hard to believe that it would have gone unnoticed. I strongly suspect that this is a bug in the custom source manager their code is using (https://st.aticpan.org/source/JHUDGE/Image-TestJPG-1.0/mydatasrc/mydatasrc.c). They seem to have hacked up the old stdio source manager from libjpeg v6b so that it does in-memory decompression. I don't have time to debug other people's code, but I would suggest that the first course of action should be to replace their custom source manager with the built-in memory source manager available in libjpeg-turbo (this may be as simple as replacing the call to jpeg_my_src() with a similar call to jpeg_mem_src().) If it still fails with that modification, then it will be much easier for me to diagnose. Other people's source/destination manager code is tricky to debug. As it stands, I would need to spend a couple of hours testing their code, and I strongly suspect that what I would find is that this is their bug, not ours.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants