Hi Brian,
> > I think this is a crafted file.
$ hexdump potrace_testcase
0000000 4d42 ffff ffff 00e2 0000 0001 0000 006c
0000010 0000 0029 0000 0001 4000 0001 0020 0003
0000020 0000 1703 00ff ff00 0000 80ff f200 0000
0000030 ff00 0005 0000 0000 0000 0000 7900 0001
0000040 fff2 0000 1200 000b 1200 000b 0000 0000
0000050 0000 0000 0080 ff00 0000 00ff ff1d ff00
0000060 0000 081d 01ff 0000 0000 0000 05f1 0000
0000070 0100 0000 0000 0000 0000 0080 0100 0000
0000080 0000 0000 0000 0000 4100 0000 0000 0000
0000090 1800 0000 0000 0000 0000 0000 0001
the four bytes (0x2-0x5) defining the size[0] have been set to the maximal
value and the four bytes defining the height (0x16-0x1A) (1073741825=0x40000001)
suppose a number of lines much higher than the actual number of lines.
So yes, this looks like a malicious, buggy bmp :)
> > By the way, where did you find the reproducer ? I can't find it
> > anywhere.
>
> It was sent on the oss-security list as an attachment, but the HTML
> archive strips attachments.
>
> http://www.openwall.com/lists/oss-security/2016/10/10/1
Thanks !
> So I had to find the original email. I have attached the file to this
> email. Let me know if you get it.
I've had a look at it, here's what I got:
The malloc call is realised at src/bitmap.h:75
81 bm->dy = dy;
82 bm->map = (potrace_word *) malloc(size);
83 if (!bm->map) {
in the bm_new function.
The backtrace indicates that bm_new was called by src/bitmap_io.c:537:
536 /* allocate bitmap */
537 bm = bm_new(bmpinfo.w, bmpinfo.h);
538 if (!bm) {
539 goto std_error;
540 }
in the bm_readbody_bmp function.
In our case bmpinfo.w and bmpinfo.h are declared at line 461/462
457 if (bmpinfo.InfoSize == 40 || bmpinfo.InfoSize == 64
458 || bmpinfo.InfoSize == 108 || bmpinfo.InfoSize == 124) {
459 /* Windows or new OS/2 format */
460 bmpinfo.ctbits = 32; /* sample size in color table */
461 TRY(bmp_readint(f, 4, &bmpinfo.w));
462 TRY(bmp_readint(f, 4, &bmpinfo.h));
So, bmpinfo.x = 41 and bmpinfo.y = 1073741825.
This means that bm_new tries to allocate
size = dy*h*BM_WORDSIZE = 8589934600 bytes (according to my logs) = 8G
of memory, which will obviously fail.
So, the issue looks like an OOM problem: The program tries to allocate a
large amount of memory and malloc fails to do so. This can lead to
buffer under/overflows in some situations[1] and that's why it is
commonly considered as security issue.
Please correct me if I'm wrong.
Whether CVE-2016-8686 is really exploitable in practice or not is another
question.
So, what did upstream do to fix this problem ?
Well, on my system, asan still reports a memory allocation failure for
1.14, so it looks like the problem was not 100% fixed.
What upstream did is to 'truncate the image size when bitmap data ends
prematurely'.
So, he added a 'realheight' variable in the bm_readbody_bmp function that
counts the number of lines that really contain informations and adjusts
the bitmap's height and malloc-ed memory after loading the file.
Concerning the changes to bm_new, they look like pure refactoring.
So, I'm not 100% sure that the upstream fix is sufficient.
Would it help to add a check after the malloc call, that would make sure
that malloc-ed memory is really available ?
Something like:
bm->map = (potrace_word *) malloc(size);
if (!bm->map || malloc_usable_size(bm->map) != size) {
free(bm);
return NULL;
}
Could anyone try to reproduce the memory allocation failure on 1.14 ?
This is maybe something that we should report ?
I feel like fixing this issue is going to be very time consuming and I'm
not sure whether it's worth taking so much time for this (previously
no-dsa triaged) issue. :)
Cheers,
Hugo
[0] https://en.wikipedia.org/wiki/BMP_file_format#Bitmap_file_header
[1] https://cansecwest.com/core05/memory_vulns_delalleau.pdf
--
Hugo Lefeuvre (hle) | www.owl.eu.com
4096/ ACB7 B67F 197F 9B32 1533 431C AC90 AC3E C524 065E
Attachment:
signature.asc
Description: PGP signature