Comment 22 for bug 427100

Revision history for this message
Timmmm (tdhutt) wrote :

Here is the code in question:

 switch (dec->colr->data.colr.method) {
 case JP2_COLR_ENUM:
  jas_image_setclrspc(dec->image, jp2_getcs(&dec->colr->data.colr));
  break;
 case JP2_COLR_ICC:
  iccprof = jas_iccprof_createfrombuf(dec->colr->data.colr.iccp,
    dec->colr->data.colr.iccplen);
  assert(iccprof);
  jas_iccprof_gethdr(iccprof, &icchdr);
  jas_eprintf("ICC Profile CS %08x\n", icchdr.colorspc);
  jas_image_setclrspc(dec->image, fromiccpcs(icchdr.colorspc));
  dec->image->cmprof_ = jas_cmprof_createfromiccprof(iccprof);
  assert(dec->image->cmprof_);
  jas_iccprof_destroy(iccprof);
  break;
 }

And in base/jas_icc.c:

jas_iccprof_t *jas_iccprof_createfrombuf(uchar *buf, int len)
{
 jas_stream_t *in;
 jas_iccprof_t *prof;
 if (!(in = jas_stream_memopen(JAS_CAST(char *, buf), len)))
  goto error;
 if (!(prof = jas_iccprof_load(in)))
  goto error;
 jas_stream_close(in);
 return prof;
error:
 return 0;
}

And I'm guessing the problem is in jas_iccprof_load():

jas_iccprof_t *jas_iccprof_load(jas_stream_t *in)
{
 jas_iccprof_t *prof;
 int numtags;
 long curoff;
 long reloff;
 long prevoff;
 jas_iccsig_t type;
 jas_iccattrval_t *attrval;
 jas_iccattrval_t *prevattrval;
 jas_icctagtabent_t *tagtabent;
 jas_iccattrvalinfo_t *attrvalinfo;
 int i;
 int len;

 prof = 0;
 attrval = 0;

 if (!(prof = jas_iccprof_create())) {
  goto error;
 }

 if (jas_iccprof_readhdr(in, &prof->hdr)) {
  jas_eprintf("cannot get header\n");
  goto error;
 }

.. Looks familiar. jas_iccprof_readhdr():

static int jas_iccprof_readhdr(jas_stream_t *in, jas_icchdr_t *hdr)
{
 if (jas_iccgetuint32(in, &hdr->size) ||
   jas_iccgetuint32(in, &hdr->cmmtype) ||
   jas_iccgetuint32(in, &hdr->version) ||
   jas_iccgetuint32(in, &hdr->clas) ||
   jas_iccgetuint32(in, &hdr->colorspc) ||
   jas_iccgetuint32(in, &hdr->refcolorspc) ||
   jas_iccgettime(in, &hdr->ctime) ||
   jas_iccgetuint32(in, &hdr->magic) ||
   jas_iccgetuint32(in, &hdr->platform) ||
   jas_iccgetuint32(in, &hdr->flags) ||
   jas_iccgetuint32(in, &hdr->maker) ||
   jas_iccgetuint32(in, &hdr->model) ||
   jas_iccgetuint64(in, &hdr->attr) ||
   jas_iccgetuint32(in, &hdr->intent) ||
   jas_iccgetxyz(in, &hdr->illum) ||
   jas_iccgetuint32(in, &hdr->creator) ||
   jas_stream_gobble(in, 44) != 44)
  return -1;
 return 0;
}

So I added some debug traces, and the problem is that jas_stream_gobble() returns 0, not 44. Here is the function, complete with silly backwards loop.

int jas_stream_gobble(jas_stream_t *stream, int n)
{
 int m;
 m = n;
 for (m = n; m > 0; --m) {
  if (jas_stream_getc(stream) == EOF) {
   return n - m;
  }
 }
 return n;
}

Since it doesn't seem to use the expected 44 bytes of this header, I tried removing the check. Note that when writing the equivalent icc profile, it does add 44 padding bytes, so someone hasn't read the spec properly... (i.e. either the padding is optional or it isn't and the file is invalid). Anyway, libjasper should never abort(), that is just plain wrong (and dangerous).

Sadly removing the check just shifts us to the error "cannot get tab table". Ok, that's all I have for now. Somebody is going to have to read the spec and see where libjasper is going wrong. And also remove all those ridiculous abort()s.