The security of deflate-compressed streams with uncompressed-length field
Cosmin Truta <[email protected]>
| Newsgroups | gmane.comp.graphics.png.general |
|---|---|
| Message-ID | <CAAoVtZxu+b=TALg1hgUu-HXK_MLsSkxb_5feeTpnwHBLQrYx4A@mail.gmail.com> |
I am starting this discussion here, in the light of the past
experience implementing iCCP, zTXt, iTXt, and the recent discussion
about zXIf.
It has been mentioned that an uncompressed-length field ought to
accompany the deflate-compressed stream. This has not been the
practice so far (in zTXt/iTXt/iCCP). I would like to analyze the
correctness and security implications of doing so vs. not doing so,
specification-wise and implementation-wise.
** Specification-wise **
It is a matter of correctness (not security, just correctness) to "get
it right", without requiring the uncompressed length.
When the uncompressed-length field gets added in, it may or may not
become also a matter of security (besides just correctness), depending
on how naive is the implementation. Why? Because that field can be set
to a deceitful value by a malevolent content creator, and the
implementation should check it anyway. The get-it-right
implementations should still be ok, but naive implementations may fall
prey to either (1) deceitfully-short uncompressed-length fields and
opportunities to overflow the uncompressed-data buffer, or (2)
deceitfully-long uncompressed-length fields and opportunities to pass
in uninitialized data, under the (incorrect) assumption that zlib will
fill in that buffer. (Remember the Heartbleed security bug?)
** Implementation-wise **
I understand that some people found it difficult to implement iCCP
correctly, but I do not share that experience. Neither in C, nor in
any higher-level languages. My favorite example is the following
Python one-liner: "zlib.decompress(foo)". The addition of the extra
field will do nothing but add in an extra level of complication, and
an extra opportunity to get it wrong.
This problem needs to be solved at a higher-level, inside the
PNG-supporting library.
PNG chunks may contain integers, reals (fixed-point or
floating-point), strings, raw (uncompressed) blobs, and
deflate-encoded blobs. The PNG-supporting library should offer
services to encode and decode all of these. Currently, libpng offers
support for all of these except for deflate-encoded blobs.
Specifically, libpng should offer a general-purpose inflation routine, such as:
int png_get_inflate_buffer(
png_struct *png_ptr,
png_byte **out_inflate_buffer_ptr,
size_t *out_inflate_buffer_size_ptr,
const png_byte *in_deflate_buffer,
size_t in_deflate_buffer_size)
Implementations should use this, just as easily as they are using
png_get_uint_32 and friends, for any public and private chunk data
manipulation.
Sincerely,
Cosmin
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot