Heap based OOB READ in hbpldecode.c

[xiang xiaobo]

hbpldecode is used to decode a HBPL stream into human readbal form , and HBPL is Host Based Printer Language , it belongs to the foo2zjs project , whose official page is http://foo2hbpl.rkkda.com/ .
hbpldecode locates in /usr/bin/ directory of ubuntu , a heap based OOB will occur if hbpldecode try to decode a crafted HBPL stream thus resulting in info leak in the heap , which is a security problem .

The problem lies in decode2 function . when the header of crafted file is "PS" , the program will allocate "len" byte heap chunk to mbuf , and then call print_bih(mbuf) . the print_bih will output 20 bytes infomation of mbuf regardless of its actual size , when the "len" read from file is smaller than 20 , it causes an out of bound read issue .

related code snippets :

#decode2:
    if (header[1] == '%' && header[2] == '-') //end of file
      len = 15;
    else
    {
        if (header[1] == 'J' && header[2] == 'P')
        len = 60; // JP doesn't have len
        else
        len = header[3];
        printf("RECTYPE %c%c - size=%d ", header[1], header[2], len);
    }

    curOff += len+4;
    rc = fread(buf, 1, len, fp);
    //....

    if (header[1] == 'P' && header[2] == 'S'){
    //...
        len = getLEdword(&buf[12]);
        mbuf = malloc(len);
        if ( color == 1 ){
            //...
            print_bih(mbuf);
            print_bih(mbuf + offbih[0]);
            print_bih(mbuf + offbih[0] + offbih[1]);
            //...
        }else{
            //...
            print_bih(mbuf);
            //...
        }
    }
# print_bih:
void
print_bih(unsigned char bih[20])
{
    unsigned int xd, yd, l0;

    xd = (bih[4] << 24) | (bih[5] << 16) | (bih[6] << 8) | (bih[7] << 0);
    yd = (bih[8] << 24) | (bih[9] << 16) | (bih[10] << 8) | (bih[11] << 0);
    l0 = (bih[12] << 24) | (bih[13] << 16) | (bih[14] << 8) | (bih[15] << 0);

    printf(" DL = %d, D = %d, P = %d, - = %d, XY = %d x %d\n",
     bih[0], bih[1], bih[2], bih[3], xd, yd);

    printf(" L0 = %d, MX = %d, MY = %d\n",
     l0, bih[16], bih[17]);

    printf(" Order = %d %s%s%s%s%s\n", bih[18],
    bih[18] & JBG_HITOLO ? " HITOLO" : "",
    bih[18] & JBG_SEQ ? " SEQ" : "",
    bih[18] & JBG_ILEAVE ? " ILEAVE" : "",
    bih[18] & JBG_SMID ? " SMID" : "",
    bih[18] & 0xf0 ? " other" : "");

    printf(" Options = %d %s%s%s%s%s%s%s%s\n", bih[19],
    bih[19] & JBG_LRLTWO ? " LRLTWO" : "",
    bih[19] & JBG_VLENGTH ? " VLENGTH" : "",
    bih[19] & JBG_TPDON ? " TPDON" : "",
    bih[19] & JBG_TPBON ? " TPBON" : "",
    bih[19] & JBG_DPON ? " DPON" : "",
    bih[19] & JBG_DPPRIV ? " DPPRIV" : "",
    bih[19] & JBG_DPLAST ? " DPLAST" : "",
    bih[19] & 0x80 ? " other" : "");
    printf(" %u stripes, %d layers, %d planes\n",
    ((yd >> bih[1]) + ((((1UL << bih[1]) - 1) & xd) != 0) + l0 - 1) / l0,
    bih[1] - bih[0], bih[2]);
}

Sanitizer output :
==114006==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200000eff4 at pc 0x000000405552 bp 0x7ffdf9162dc0 sp 0x7ffdf9162db0
READ of size 4 at 0x60200000eff4 thread T0
    #0 0x405551 in print_bih /home/bobb/fuzz-workspace/foo2zjs/hbpldecode.c:183
    #1 0x406937 in decode2 /home/bobb/fuzz-workspace/foo2zjs/hbpldecode.c:431
    #2 0x40f20b in decode /home/bobb/fuzz-workspace/foo2zjs/hbpldecode.c:865
    #3 0x401ea9 in main /home/bobb/fuzz-workspace/foo2zjs/hbpldecode.c:928
    #4 0x7fe49830882f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
    #5 0x402148 in _start (/home/bobb/fuzz-workspace/foo2zjs-workspace/hbpldecode+0x402148)

0x60200000eff4 is located 3 bytes to the right of 1-byte region [0x60200000eff0,0x60200000eff1)
allocated by thread T0 here:
    #0 0x7fe498749602 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x98602)
    #1 0x4067ba in decode2 /home/bobb/fuzz-workspace/foo2zjs/hbpldecode.c:383

SUMMARY: AddressSanitizer: heap-buffer-overflow /home/bobb/fuzz-workspace/foo2zjs/hbpldecode.c:183 print_bih

[Seth Arnold (seth-arnold)]

Nice find. I suspect this wouldn't have actual security consequences but it would be worth fixing, if only so that future fuzz runs don't trip it again. Could you please report this to upstream (probably via http://foo2zjs.rkkda.com/forum/ , I didn't see any other way to report issues to them), and link the topic here?

Thanks

[xiang xiaobo (xiangxiaobo)]

Hi,
I gave an email about these issues to the maintainer of the project. And he
says xxxdecode.c are just test programs, so he refused to give them
patches.
I am a new to fuzzing and reporting bugs, Could you please tell how can I
handle these issues then?

Best regards.
Bobb

2017-01-11 4:27 GMT+08:00 Seth Arnold <email address hidden>:

> Nice find. I suspect this wouldn't have actual security consequences but
> it would be worth fixing, if only so that future fuzz runs don't trip it
> again. Could you please report this to upstream (probably via
> http://foo2zjs.rkkda.com/forum/ , I didn't see any other way to report
> issues to them), and link the topic here?
>
> Thanks
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1653729
>
> Title:
> Heap based OOB READ in hbpldecode.c
>
> Status in foo2zjs package in Ubuntu:
> New
>
> Bug description:
> hbpldecode is used to decode a HBPL stream into human readbal form , and
> HBPL is Host Based Printer Language , it belongs to the foo2zjs project ,
> whose official page is http://foo2hbpl.rkkda.com/ .
> hbpldecode locates in /usr/bin/ directory of ubuntu , a heap based OOB
> will occur if hbpldecode try to decode a crafted HBPL stream thus resulting
> in info leak in the heap , which is a security problem .
>
> The problem lies in decode2 function . when the header of crafted file
> is "PS" , the program will allocate "len" byte heap chunk to mbuf ,
> and then call print_bih(mbuf) . the print_bih will output 20 bytes
> infomation of mbuf regardless of its actual size , when the "len" read
> from file is smaller than 20 , it causes an out of bound read issue .
>
> related code snippets :
>
> #decode2:
> if (header[1] == '%' && header[2] == '-') //end of file
> len = 15;
> else
> {
> if (header[1] == 'J' && header[2] == 'P')
> len = 60; // JP doesn't have len
> else
> len = header[3];
> printf("RECTYPE %c%c - size=%d ", header[1], header[2], len);
> }
>
> curOff += len+4;
> rc = fread(buf, 1, len, fp);
> //....
>
> if (header[1] == 'P' && header[2] == 'S'){
> //...
> len = getLEdword(&buf[12]);
> mbuf = malloc(len);
> if ( color == 1 ){
> //...
> print_bih(mbuf);
> print_bih(mbuf + offbih[0]);
> print_bih(mbuf + offbih[0] + offbih[1]);
> //...
> }else{
> //...
> print_bih(mbuf);
> //...
> }
> }
> # print_bih:
> void
> print_bih(unsigned char bih[20])
> {
> unsigned int xd, yd, l0;
>
> xd = (bih[4] << 24) | (bih[5] << 16) | (bih[6] << 8) | (bih[7] << 0);
> yd = (bih[8] << 24) | (bih[9] << 16) | (bih[10] << 8) | (bih[11] <<
> 0);
> l0 = (bih[12] << 24) | (bih[13] << 16) | (bih[14] << 8) | (bih[15]
> << 0);
>
> printf(" DL = %d, D = %d, P = %d, - = %d, XY = %d x %d\n",
> bih[0], bih[1], bih[2], bih[3], xd, yd);
>
> printf(" L0 = %d, MX = %d, MY = %d\n",...

[Seth Arnold (seth-arnold)]

Hi Bobb,

Oh, that's interesting. I wonder if we should stop packaging these tools, or ask Debian's maintainer to stop packaging them then. Packaged programs have a way of getting used, perhaps beyond the original author's intentions.

I've found fuzzing results to be best accepted by upstreams when run against a recent checkout of their development branch; it's normally best to report issues to upstreams first, since they are in the best position to prepare fixes and determine if older versions may also be affected. If you can test the crashers against released versions, that's often also helpful to report.

When reporting fuzzing-discovered issues, it's important to include the generated test cases.

In this specific case, your analysis was very helpful; I'm sure other upstreams would appreciate this kind of effort in reports. It's all too easy to just dump a few hundred crashing files on someone. (I've done this. Several times. It hasn't been received well.)

Thanks