[libpng16] Eliminated the final two Coverity defects (insecure temporary file

handling in contrib/libtests/pngstest.c; possible overflow of
unsigned char in contrib/tools/png-fix-itxt.c). To use the "secure"
file handling, define PNG_USE_MKSTEMP, otherwise "tmpfile()" will
continue to be used.
This commit is contained in:
Glenn Randers-Pehrson 2015-06-10 07:05:18 -05:00
parent 4293254bef
commit f6e7551f06
4 changed files with 58 additions and 24 deletions

View File

@ -1,4 +1,4 @@
Libpng 1.6.18beta08 - June 6, 2015 Libpng 1.6.18beta08 - June 10, 2015
This is not intended to be a public release. It will be replaced This is not intended to be a public release. It will be replaced
within a few weeks by a public version or by another test version. within a few weeks by a public version or by another test version.
@ -87,7 +87,12 @@ Version 1.6.18beta07 [June 6, 2015]
to compile them without the minimum required support enabled to compile them without the minimum required support enabled
(suggested by Flavio Medeiros). (suggested by Flavio Medeiros).
Version 1.6.18beta08 [June 6, 2015] Version 1.6.18beta08 [June 10, 2015]
Eliminated the final two Coverity defects (insecure temporary file
handling in contrib/libtests/pngstest.c; possible overflow of
unsigned char in contrib/tools/png-fix-itxt.c). To use the "secure"
file handling, define PNG_USE_MKSTEMP, otherwise "tmpfile()" will
be used.
Send comments/corrections/commendations to png-mng-implement at lists.sf.net Send comments/corrections/commendations to png-mng-implement at lists.sf.net
(subscription required; visit (subscription required; visit

View File

@ -5272,7 +5272,12 @@ Version 1.6.18beta07 [June 6, 2015]
to compile them without the minimum required support enabled to compile them without the minimum required support enabled
(suggested by Flavio Medeiros). (suggested by Flavio Medeiros).
Version 1.6.18beta08 [June 6, 2015] Version 1.6.18beta08 [June 10, 2015]
Eliminated the final two Coverity defects (insecure temporary file
handling in contrib/libtests/pngstest.c; possible overflow of
unsigned char in contrib/tools/png-fix-itxt.c). To use the "secure"
file handling, define PNG_USE_MKSTEMP, otherwise "tmpfile()" will
be used.
Send comments/corrections/commendations to png-mng-implement at lists.sf.net Send comments/corrections/commendations to png-mng-implement at lists.sf.net
(subscription required; visit (subscription required; visit

View File

@ -3245,9 +3245,23 @@ write_one_file(Image *output, Image *image, int convert_to_8bit)
if (image->opts & USE_STDIO) if (image->opts & USE_STDIO)
{ {
#ifndef PNG_USE_MKSTEMP
FILE *f = tmpfile();
#else
/* Experimental. Coverity says tmpfile() is insecure because it
* generates predictable names.
*/
char tmpfile[] = "pngstest-XXXXXX"; char tmpfile[] = "pngstest-XXXXXX";
FILE *f = fopen(mktemp(tmpfile),"w+"); int filedes;
FILE *f;
umask(0600);
filedes = mkstemp(tmpfile);
if (filedes >= 0)
f = fdopen(filedes,"w+");
else
f = NULL;
#endif
if (f != NULL) if (f != NULL)
{ {
if (png_image_write_to_stdio(&image->image, f, convert_to_8bit, if (png_image_write_to_stdio(&image->image, f, convert_to_8bit,

View File

@ -34,8 +34,9 @@
#define MAX_LENGTH 500000 #define MAX_LENGTH 500000
#define GETBREAK ((unsigned char)(inchar=getchar())); if (inchar == EOF) break #define GETBREAK inchar=getchar(); \
c=(inchar & 0xff);\
if (inchar != (int) c) break
int int
main(void) main(void)
{ {
@ -48,25 +49,25 @@ main(void)
/* Skip 8-byte signature */ /* Skip 8-byte signature */
for (i=8; i; i--) for (i=8; i; i--)
{ {
c=GETBREAK; GETBREAK;
putchar(c); putchar(c);
} }
if (inchar != EOF) if (inchar == (int) c) /* !EOF */
for (;;) for (;;)
{ {
/* Read the length */ /* Read the length */
unsigned long length; /* must be 32 bits! */ unsigned long length; /* must be 32 bits! */
c=GETBREAK; buf[0] = c & 0xff; length = (c & 0xff); length <<= 8; GETBREAK; buf[0] = c; length = c; length <<= 8;
c=GETBREAK; buf[1] = c & 0xff; length += (c & 0xff); length <<= 8; GETBREAK; buf[1] = c; length += c; length <<= 8;
c=GETBREAK; buf[2] = c & 0xff; length += (c & 0xff); length <<= 8; GETBREAK; buf[2] = c; length += c; length <<= 8;
c=GETBREAK; buf[3] = c & 0xff; length += (c & 0xff); GETBREAK; buf[3] = c; length += c;
/* Read the chunkname */ /* Read the chunkname */
c=GETBREAK; buf[4] = c & 0xff; GETBREAK; buf[4] = c;
c=GETBREAK; buf[5] = c & 0xff; GETBREAK; buf[5] = c;
c=GETBREAK; buf[6] = c & 0xff; GETBREAK; buf[6] = c;
c=GETBREAK; buf[7] = c & 0xff; GETBREAK; buf[7] = c;
/* The iTXt chunk type expressed as integers is (105, 84, 88, 116) */ /* The iTXt chunk type expressed as integers is (105, 84, 88, 116) */
@ -81,9 +82,12 @@ for (;;)
/* Copy the data bytes */ /* Copy the data bytes */
for (i=8; i < length + 12; i++) for (i=8; i < length + 12; i++)
{ {
c=GETBREAK; buf[i] = c & 0xff; GETBREAK; buf[i] = c;
} }
if (inchar != (int) c) /* EOF */
break;
/* Calculate the CRC */ /* Calculate the CRC */
crc = crc32(crc, buf+4, (uInt)length+4); crc = crc32(crc, buf+4, (uInt)length+4);
@ -101,13 +105,16 @@ for (;;)
if (length >= MAX_LENGTH-12) if (length >= MAX_LENGTH-12)
break; break;
c=GETBREAK; GETBREAK;
buf[length+11] = c & 0xff; buf[length+11] = c;
/* Update the CRC */ /* Update the CRC */
crc = crc32(crc, buf+7+length, 1); crc = crc32(crc, buf+7+length, 1);
} }
if (inchar != (int) c) /* EOF */
break;
/* Update length bytes */ /* Update length bytes */
buf[0] = (unsigned char)((length >> 24) & 0xff); buf[0] = (unsigned char)((length >> 24) & 0xff);
buf[1] = (unsigned char)((length >> 16) & 0xff); buf[1] = (unsigned char)((length >> 16) & 0xff);
@ -121,6 +128,9 @@ for (;;)
else else
{ {
if (inchar != (int) c) /* EOF */
break;
/* Copy bytes that were already read (length and chunk name) */ /* Copy bytes that were already read (length and chunk name) */
for (i=0; i<8; i++) for (i=0; i<8; i++)
putchar(buf[i]); putchar(buf[i]);
@ -128,11 +138,11 @@ for (;;)
/* Copy data bytes and CRC */ /* Copy data bytes and CRC */
for (i=8; i< length+12; i++) for (i=8; i< length+12; i++)
{ {
c=GETBREAK; GETBREAK;
putchar((c & 0xff)); putchar(c);
} }
if (inchar == EOF) if (inchar != (int) c) /* EOF */
{ {
break; break;
} }
@ -142,7 +152,7 @@ for (;;)
break; break;
} }
if (inchar == EOF) if (inchar != (int) c) /* EOF */
break; break;
if (buf[4] == 73 && buf[5] == 69 && buf[6] == 78 && buf[7] == 68) if (buf[4] == 73 && buf[5] == 69 && buf[6] == 78 && buf[7] == 68)