Significant vulnerability in libgeotiff

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

Significant vulnerability in libgeotiff

Frank Warmerdam
Folks,

Duncan Chaundy at PCI has brought an issue to my attention where
improperly produced geotiff files (with geotags that should be short
stored as double for instance) can easily result in memory corruption
in applications since GTIFKeyGet() does not provide any opportunity
to check a key value type before loading it into the application provided
buffer.

In his case he is encountering this problem with some Vexcel generated
files. But we can imagine this might arise anytime someone uses the
improper types for geokeys.

I have created a bug report for this issue:

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

In it I write:

It would appear that GTIFKeyGet() has no protection against applications
passing in a buffer of a type different than the geokey value stored in
the file.  Furthermore, there is nothing in libgeotiff that enforces the
use of the types expected in the specification.

I can see a few approaches.

 1) GTIFKeyInfo() can be used to find the type of a field in a given geotiff
    file, and this could be used to avoid fetching it when it is the wrong type.

 2) We could implement a new version of GTIFKeyGet() that includes the type
    of the passed in field for validation.

 3) We could add validation into libgeotiff such that geokeys of the wrong
    type are discarded on reading (or possibly coerced to the correct type).

I don't like option (1) because it puts a great deal of extra effort onto the
applications and is unlikely to be done.  Option (2) would not be
disruptive, but only applications that go to the effort to use the new
function would be safe.  Of course, I would update the geo_normalize.c
to do this.   Option (3) should benefit any application upgrading to a
new libgeotiff without any actual code changes to the application,
though it would potentially be bad just throwing away some geokeys.

Any thoughts from the community on how this should be approached?

Best regards,
--
---------------------------------------+--------------------------------------
I set the clouds in motion - turn up   | Frank Warmerdam, [hidden email]
light and sound - activate the windows | http://pobox.com/~warmerdam
and watch the world go round - Rush    | Geospatial Programmer for Rent

_______________________________________________
Geotiff mailing list
[hidden email]
http://lists.maptools.org/mailman/listinfo/geotiff
Reply | Threaded
Open this post in threaded view
|

Re: Significant vulnerability in libgeotiff

Chris 'Xenon' Hanson
Frank Warmerdam wrote:
>  3) We could add validation into libgeotiff such that geokeys of the wrong
>     type are discarded on reading (or possibly coerced to the correct type).
> Option (3) should benefit any application upgrading to a
> new libgeotiff without any actual code changes to the application,
> though it would potentially be bad just throwing away some geokeys.
> Any thoughts from the community on how this should be approached?

   Option 3 seems like the wisest course. For those who are worried about a performance
impact, the safety-checking code could perhaps be compiled out by a #define, which
defaulted to off. In this way, naive users would get the new error checking and people who
(thought) they knew what they were doing would have to deliberately take steps to get the
old behaviour.

--
      Chris 'Xenon' Hanson | Xenon @ 3D Nature | http://www.3DNature.com/
  "I set the wheels in motion, turn up all the machines, activate the programs,
   and run behind the scenes. I set the clouds in motion, turn up light and sound,
   activate the window, and watch the world go 'round." -Prime Mover, Rush.
_______________________________________________
Geotiff mailing list
[hidden email]
http://lists.maptools.org/mailman/listinfo/geotiff