Bug 2535 - Assertion in TIFFWriteDirectoryTagCheckedRational
: Assertion in TIFFWriteDirectoryTagCheckedRational
Status: RESOLVED FIXED
: libtiff
default
: unspecified
: PC Linux
: P2 normal
: ---
Assigned To:
:
:
:
:
:
  Show dependency treegraph
 
Reported: 2016-02-05 11:24 by
Modified: 2017-01-11 10:51 (History)


Attachments
TIFF-image which fails an assertion (3.21 KB, image/tiff)
2016-02-05 11:24, last5bits
Details


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2016-02-05 11:24:54
Created an attachment (id=643) [details]
TIFF-image which fails an assertion

The tool tiffcrop fails with an assertion on the file attached. The messages
are:

$ tiffcrop -U px -E top -X 60 -Y 60 assert.tiff out.tiff
TIFFReadDirectoryCheckOrder: Warning, Invalid TIFF directory; tags are not
sorted in ascending order.
TIFFReadDirectory: Warning, TIFF directory is missing required
"StripByteCounts" field, calculating from imagelength.
tiffcrop: tif_dirwrite.c:2084: TIFFWriteDirectoryTagCheckedRational: Assertion
`value>=0.0' failed.

The version of libtiff is 4.0.6
------- Comment #1 From 2016-04-18 10:35:28 -------
This assertion is in the following function:

static int
TIFFWriteDirectoryTagCheckedRational(TIFF* tif, uint32* ndir, TIFFDirEntry*
dir, uint16 tag, double value)
{
    uint32 m[2];
    assert(value>=0.0);
    assert(sizeof(uint32)==4);
    if (value<=0.0)
    {
        m[0]=0;
        m[1]=1;
    }
        ...
}

Please note that there is already an if-branch that handles the case of
"value<=0". If the assertion `assert(value>=0.0);` removed, program finishes
successfully.

Moreover, here's a version of this function from an earlier version of libtiff:
static int
TIFFWriteDirectoryTagCheckedRational(TIFF* tif, uint32* ndir, TIFFDirEntry*
dir, uint16 tag, double value)
{
       uint32 m[2];
       assert(value>=0.0);
       if (value==(uint32)value)
       {
               m[0]=value;
               m[1]=1;
       }
       else if (value<1.0)
       {
               m[0]=(uint32)(value*0xFFFFFFFF);
               m[1]=0xFFFFFFFF;
       }
       ...
}

There is no if-branch yet that specifically handles the "value<=0" case. My
guess is that after the branch "if (value<=0.0)" was added, the assertion was
forgotten to be removed.

If you believe that non-positive "value" is suspicious, I suggest to issue a
warning. Assertion is less favorable as it terminates the program
------- Comment #2 From 2016-11-11 15:49:45 -------
*** Bug 2588 has been marked as a duplicate of this bug. ***
------- Comment #3 From 2016-12-03 11:44:33 -------
*** Bug 2612 has been marked as a duplicate of this bug. ***
------- Comment #4 From 2017-01-11 07:52:21 -------
Fixed per

2017-01-11 Even Rouault <even.rouault at spatialys.com>

        * libtiff/tif_dirwrite.c: in TIFFWriteDirectoryTagCheckedRational,
replace
        assertion by runtime check to error out if passed value is strictly
        negative.
        Fixes http://bugzilla.maptools.org/show_bug.cgi?id=2535

        * tools/tiffcrop.c: remove extraneous TIFFClose() in error code path,
that
        caused double free.
        Related to http://bugzilla.maptools.org/show_bug.cgi?id=2535



/cvs/maptools/cvsroot/libtiff/ChangeLog,v  <--  ChangeLog
new revision: 1.1202; previous revision: 1.1201
/cvs/maptools/cvsroot/libtiff/libtiff/tif_dirwrite.c,v  <-- 
libtiff/tif_dirwrite.c
new revision: 1.84; previous revision: 1.83
/cvs/maptools/cvsroot/libtiff/tools/tiffcrop.c,v  <--  tools/tiffcrop.c
new revision: 1.50; previous revision: 1.49
------- Comment #5 From 2017-01-11 10:51:59 -------
*** Bug 2645 has been marked as a duplicate of this bug. ***