ito-suite / node-ito-transcoder

The transcoder is a shell application with node bindings for file transcoding.
GNU Affero General Public License v3.0
2 stars 1 forks source link

libpng hardening #3

Open denjello opened 9 years ago

denjello commented 9 years ago

libpng has a VERY old issue first reported in 2002, where ancillary data outside of the bounds of the image as described in the header can lead to buffer overflow and is a common exploit on windows machines, but can also affect any device displaying pngs without a modern version of libpng. Furthermore, because of its awesome compression it is possible (and rather trivial) to expand a small file to occupy huge amounts of memory. Considering the fact that we will someday be delivering this module as a stand-alone Node Webkit application, we would be wise to stick to bleeding edge versions of libpng where possible.

Here is a case in point:

libpng-1.6.17 and 1.5.22 released

"libpng-1.6.17 and 1.5.22 have been released. They "harden" the library against attacks using very wide images by imposing a default limit of 1 million columns. Users who truly need to process wider images can override this limit."

Most importantly we need to make sure that we are installing at least, and if possible remove it entirely:

Unfortunately, various components of the transcoder use repository versions of libpng (if I am not mistaken because ufraw requires minimum libpng-1.2*).

apt-get install \
 libpng-dev pngcrush libpng12-0 libpng12-dev \
 libpng++-dev libpng3 libpnglite-dev pngtools

Later we are building libpng-1.6.17 for pngquant - although we are changing the makefile of pngquant to link libpng16 instead of libpng15. I propose refactoring the build process such that all image based libs are installed / made before actual applications (like graphicsmagick etc.) are built.

While we are at it, it would be wise to prevent the system from parsing out of bounds data when reading PNG files - but that might mean PATCHING EVERY LIBRARY THAT USES libpng. :-1:

  #define PNG_NO_READ_iCCP
  #define PNG_NO_READ_TEXT /* disables tEXt, zTXt, and iTXt */

At the very least we should look over the source of the following:

Additionally, it might be a nice touch to simply trim ancillary data out of the png. This should be relatively trivial to write and more trivial to use in the pipeline.

dasantonym commented 9 years ago

the last sentence gives me hope. checking and patching all those huge packages and then maintaining them sounds like wontfix to me.