[libpng17] 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.
This commit is contained in:
Glenn Randers-Pehrson 2015-06-10 06:59:18 -05:00
parent 9307eef199
commit 24382d838c
4 changed files with 58 additions and 24 deletions

View File

@ -1,5 +1,5 @@
Libpng 1.7.0beta64 - June 7, 2015 Libpng 1.7.0beta64 - 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.
@ -835,12 +835,17 @@ Version 1.7.0beta63 [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.7.0beta64 [June 7, 2015] Version 1.7.0beta64 [June 10, 2015]
Removed non-working progressive reader 'skip' function. This Removed non-working progressive reader 'skip' function. This
function has apparently never been used. It was implemented function has apparently never been used. It was implemented
to support back-door modification of png_struct in libpng-1.4.x to support back-door modification of png_struct in libpng-1.4.x
but (because it does nothing and cannot do anything) was apparently but (because it does nothing and cannot do anything) was apparently
never tested (John Bowler). never tested (John Bowler).
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

@ -5130,12 +5130,17 @@ Version 1.7.0beta63 [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.7.0beta64 [June 7, 2015] Version 1.7.0beta64 [June 10, 2015]
Removed non-working progressive reader 'skip' function. This Removed non-working progressive reader 'skip' function. This
function has apparently never been used. It was implemented function has apparently never been used. It was implemented
to support back-door modification of png_struct in libpng-1.4.x to support back-door modification of png_struct in libpng-1.4.x
but (because it does nothing and cannot do anything) was apparently but (because it does nothing and cannot do anything) was apparently
never tested (John Bowler). never tested (John Bowler).
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,8 +3245,22 @@ 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)
{ {

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)