Need for still supporting truncated StripByteCount/StripOffsets tag ?

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Need for still supporting truncated StripByteCount/StripOffsets tag ?

Even Rouault-2

Hi,

 

currently TIFFFetchStripThing() which is used to read the StripByteCount/StripOffsets tags and

corresponding tags for tiling supports reading files where the declared number of values in the

tag is lower than the expected number of strips/tiles (*). Missing entries are set to 0. Does that make

sense to support those (invalid, since the spec mentions the expected number of values) files ?

One issue with the current behaviour is that it makes it very easy to cause huge allocation of

memory (and the memory is actually pinned since it is memset to zero, so potentially causing

OOM killer to run) for very small (in bytes) files.

 

Digging into history, I see this was added on purpose in libtiff 3.7.0 per

https://github.com/vadz/libtiff/commit/d78ce54306e5cdcd916a1e87a2738cdb210062bd

 

2002-12-17 Frank Warmerdam <[hidden email]>

 

* libtiff/tif_dirread.c: Allow wrong sized arrays in

TIFFFetchStripThing().

 

http://bugzilla.remotesensing.org/show_bug.cgi?id=49

 

The ticket mentionning in this commit message (42) or the one in the above quoted corresponding

changelog entry are unfortunately no longer accessible, so difficult to know what was the actual use case.

 

I'd be in favor of removing that capability, or perhaps limiting this up to a not so big number of

tiles (let's say 1 million, with a warning stating this is an invalid file. And beyond that, error out with an error

message)

 

Even

 

(*) A few weeks ago, I fixed the other case, where the number of values is greater than the

expected number of striles, so as to just allocate the final number and avoid a uncessary

temporary big array.

 

--

Spatialys - Geospatial professional services

http://www.spatialys.com


_______________________________________________
Tiff mailing list: [hidden email]
http://lists.maptools.org/mailman/listinfo/tiff
http://www.remotesensing.org/libtiff/
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Need for still supporting truncated StripByteCount/StripOffsets tag ?

Even Rouault-2

> I'd be in favor of removing that capability, or perhaps limiting this up to

> a not so big number of tiles (let's say 1 million, with a warning stating

> this is an invalid file. And beyond that, error out with an error message)

 

I went on implementing this per

 

2017-07-15 Even Rouault <even.rouault at spatialys.com>

 

* libtiff/tif_read.c: in TIFFFetchStripThing(), only grow the

arrays that hold StripOffsets/StripByteCounts, when they are smaller

than the expected number of striles, up to 1 million striles, and

error out beyond. Can be tweaked by setting the environment variable

LIBTIFF_STRILE_ARRAY_MAX_RESIZE_COUNT.

This partially goes against a change added on 2002-12-17 to accept

those arrays of wrong sizes, but is needed to avoid denial of services.

Fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=2350

Credit to OSS Fuzz

 

/cvs/maptools/cvsroot/libtiff/ChangeLog,v <-- ChangeLog

new revision: 1.1272; previous revision: 1.1271

/cvs/maptools/cvsroot/libtiff/libtiff/tif_dirread.c,v <-- libtiff/tif_dirread.c

new revision: 1.214; previous revision: 1.213

 

Even

 

--

Spatialys - Geospatial professional services

http://www.spatialys.com


_______________________________________________
Tiff mailing list: [hidden email]
http://lists.maptools.org/mailman/listinfo/tiff
http://www.remotesensing.org/libtiff/
Loading...