mewin/libpng
1
0
Fork 0
mirror of https://git.code.sf.net/p/libpng/code.git synced 2025-07-10 18:04:09 +02:00
Code Issues Projects Releases Wiki Activity

Coverity related changes

Browse Source 
These should fix most of the reported Coverity issues.  The remaining issues
should be the back_b etc assignments, which look like a Coverity bug, and
passing a pointer to a byte to a function that expects a pointer to one or more
bytes, which should (I believe) be fixed in one case and not the other
(next_filter) case; the latter case will probably go away as I am going to
rewrite that piece of code to avoid a spurious buffer allocation.

Signed-off-by: John Bowler <jbowler@acm.org>
This commit is contained in:
John Bowler 2015-09-17 17:58:13 -07:00 committed by Glenn Randers-Pehrson
parent d280c3a6da
commit f8d3e854cb
8 changed files with 107 additions and 82 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
ANNOUNCE
View File

@ -1,5 +1,5 @@
Libpng 1.7.0beta66 - September 17, 2015
Libpng 1.7.0beta66 - September 18, 2015
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.
@ -871,7 +871,7 @@ Version 1.7.0beta65 [September 16, 2015]
(but not yet by the writer) (John Bowler).
Implemented shared transform handling that is used throughout (John Bowler).
Version 1.7.0beta66 [September 17, 2015]
Version 1.7.0beta66 [September 18, 2015]
Enabled the low-bit-depth gray tests that were disabled in prior versions
of libpng because of problems that should have been fixed by the recent
changes to libpng17. Enabling the tests revealed bugs in those changes
@ -881,6 +881,7 @@ Version 1.7.0beta66 [September 17, 2015]
be copied, also the copies are improved to copy the transform args too;
not required at present but it may prevent a bug being introduced in
the future.
Fixed some new Coverity defects that were introduced in 1.7.0beta65.
Send comments/corrections/commendations to png-mng-implement at lists.sf.net
(subscription required; visit

3
CHANGES
View File

@ -5168,7 +5168,7 @@ Version 1.7.0beta65 [September 16, 2015]
(but not yet by the writer) (John Bowler).
Implemented shared transform handling that is used throughout (John Bowler).
Version 1.7.0beta66 [September 17, 2015]
Version 1.7.0beta66 [September 18, 2015]
Enabled the low-bit-depth gray tests that were disabled in prior versions
of libpng because of problems that should have been fixed by the recent
changes to libpng17. Enabling the tests revealed bugs in those changes
@ -5178,6 +5178,7 @@ Version 1.7.0beta66 [September 17, 2015]
be copied, also the copies are improved to copy the transform args too;
not required at present but it may prevent a bug being introduced in
the future.
Fixed some new Coverity defects that were introduced in 1.7.0beta65.
Send comments/corrections/commendations to png-mng-implement at lists.sf.net
(subscription required; visit

55
pngerror.c
View File

@ -1162,6 +1162,34 @@ PNG_FUNCTION(void,png_affirm,(png_const_structrp png_ptr,
# endif /* AFFIRM_ERROR */
}
#if !PNG_RELEASE_BUILD
void /* PRIVATE */
png_handled_affirm(png_const_structrp png_ptr, png_const_charp message,
unsigned int position)
{
# if PNG_RELEASE_BUILD
/* testing in RC: we want to return control to the caller, so do not
* use png_affirm.
*/
char buffer[512];
affirm_text(buffer, message, position);
# ifdef PNG_CONSOLE_IO_SUPPORTED
fprintf(stderr, "%s\n", buffer);
# elif defined PNG_WARNINGS_SUPPORTED
if (png_ptr != NULL && png_ptr->warning_fn != NULL)
png_ptr->warning_fn(png_constcast(png_structrp, png_ptr), buffer);
/* else no way to output the text */
# else
PNG_UNUSED(png_ptr)
# endif
# else
png_affirm(png_ptr, message, position);
# endif
}
#endif /* !RELEASE_BUILD */
#ifdef PNG_RANGE_CHECK_SUPPORTED
/* The character/byte checking APIs. These do their own calls to png_affirm
* because the caller provides the position.
@ -1209,32 +1237,5 @@ png_u16_affirm(png_const_structrp png_ptr, unsigned int position, int b)
png_affirm(png_ptr, param_deb("PNG 16-bit range") position);
}
#endif /* INT_MAX >= 65535 */
void /* PRIVATE */
png_handled_affirm(png_const_structrp png_ptr, png_const_charp message,
unsigned int position)
{
# if PNG_RELEASE_BUILD
/* testing in RC: we want to return control to the caller, so do not
* use png_affirm.
*/
char buffer[512];
affirm_text(buffer, message, position);
# ifdef PNG_CONSOLE_IO_SUPPORTED
fprintf(stderr, "%s\n", buffer);
# elif defined PNG_WARNINGS_SUPPORTED
if (png_ptr != NULL && png_ptr->warning_fn != NULL)
png_ptr->warning_fn(png_constcast(png_structrp, png_ptr), buffer);
/* else no way to output the text */
# else
PNG_UNUSED(png_ptr)
# endif
# else
png_affirm(png_ptr, message, position);
# endif
}
#endif /* RANGE_CHECK */
#endif /* READ || WRITE */

32
pngpriv.h
View File

@ -334,9 +334,11 @@
while (0)
# define png_affirmexp(pp, cond)\
((cond) ? (void)0 : png_affirm(pp, PNG_SRC_LINE))
# define png_handled(pp, m) ((void)0)
# define png_impossiblepp(pp, reason) png_affirm(pp, PNG_SRC_LINE)
# define debug(cond) do {} while (0)
# define debug_handled(cond) do {} while (0)
# if PNG_LIBPNG_BUILD_BASE_TYPE >= PNG_LIBPNG_BUILD_RC
/* Make sure there are no 'UNTESTED' macros in released code: */
# define UNTESTED libpng untested code
@ -349,15 +351,21 @@
while (0)
# define png_affirmexp(pp, cond)\
((cond) ? (void)0 : png_affirm(pp, #cond, PNG_SRC_LINE))
# define png_handled(pp, m) (png_handled_affirm((pp), (m), PNG_SRC_LINE))
# define png_impossiblepp(pp, reason) png_affirm(pp, reason, PNG_SRC_LINE)
# define debug(cond) png_affirmpp(png_ptr, cond)
# define debug_handled(cond)\
do\
if (!(cond)) png_handled(png_ptr, #cond);\
while (0)
# define UNTESTED png_affirm(png_ptr, "untested code", PNG_SRC_LINE);
# define NOT_REACHED png_affirm(png_ptr, "NOT REACHED", PNG_SRC_LINE)
#endif
#define affirm(cond) png_affirmpp(png_ptr, cond)
#define affirmexp(cond) png_affirmexp(png_ptr, cond)
#define handled(m) png_handled(png_ptr, (m))
#define impossible(cond) png_impossiblepp(png_ptr, cond)
#define implies(a, b) debug(!(a) || (b))
@ -770,6 +778,14 @@ extern "C" {
PNG_INTERNAL_FUNCTION(void, png_affirm,(png_const_structrp png_ptr,
param_deb(png_const_charp condition) unsigned int position), PNG_NORETURN);
#if !PNG_RELEASE_BUILD
PNG_INTERNAL_FUNCTION(void, png_handled_affirm,(png_const_structrp png_ptr,
png_const_charp message, unsigned int position), PNG_EMPTY);
/* This is not marked PNG_NORETURN because in PNG_RELEASE_BUILD it will
* disappear and control will pass through it.
*/
#endif /* !RELEASE_BUILD */
/* Character/byte range checking. */
/* GCC complains about assignments of an (int) expression to a (char) even when
* it can readily determine that the value is in range. This makes arithmetic
@ -831,12 +847,6 @@ PNG_INTERNAL_FUNCTION(char, png_char_affirm,(png_const_structrp png_ptr,
PNG_INTERNAL_FUNCTION(png_byte, png_byte_affirm,(png_const_structrp png_ptr,
unsigned int position, int b), PNG_EMPTY);
PNG_INTERNAL_FUNCTION(void, png_handled_affirm,(png_const_structrp png_ptr,
png_const_charp message, unsigned int position), PNG_EMPTY);
/* This is not marked PNG_NORETURN because in PNG_RELEASE_BUILD it will
* disappear and control will pass through it.
*/
#if INT_MAX >= 65535
PNG_INTERNAL_FUNCTION(png_uint_16, png_u16_affirm,(png_const_structrp png_ptr,
unsigned int position, int u), PNG_EMPTY);
@ -854,13 +864,11 @@ PNG_INTERNAL_FUNCTION(png_uint_16, png_u16_affirm,(png_const_structrp png_ptr,
# define png_check_byte(pp, b) (png_byte_affirm((pp), PNG_SRC_LINE, (b)))
# define PNG_BYTE(b) ((png_byte)((b) & 0xFFU))
# define PNG_UINT_16(u) ((png_uint_16)((u) & 0xFFFFU))
# define png_handled(pp, m) (png_handled_affirm((pp), (m), PNG_SRC_LINE))
#elif !(defined PNG_REMOVE_CASTS)
# define png_check_bits(pp, u, bits) (((1U<<(bits))-1U) & (u))
# define png_check_char(pp, c) ((char)(c))
# define png_check_byte(pp, b) ((png_byte)(b))
# define png_check_u16(pp, u) ((png_uint_16)(u))
# define png_handled(pp, m) ((void)0)
# define PNG_BYTE(b) ((png_byte)((b) & 0xFFU))
# define PNG_UINT_16(u) ((png_uint_16)((u) & 0xFFFFU))
#else
@ -877,18 +885,10 @@ PNG_INTERNAL_FUNCTION(png_uint_16, png_u16_affirm,(png_const_structrp png_ptr,
# define png_check_char(pp, c) (c)
# define png_check_byte(pp, b) (b)
# define png_check_u16(pp, u) (u)
# define png_handled(pp, m) ((void)0)
# define PNG_BYTE(b) (b)
# define PNG_UINT_16(u) (u)
#endif /* RANGE_CHECK */
/* Utility macro to mark a handled error condition ; when control reaches this
* there has been an arithmetic overflow but it is being handled. Use the
* png_check_ macros above where control should leave the code for
* safety/security reasons.
*/
#define handled(m) png_handled(png_ptr, (m))
/* Safe calculation of a rowbytes value; does a png_error if the system limits
* are exceeded.
*/

2
pngrtran.c
View File

@ -1506,7 +1506,7 @@ png_gamma_significant(png_const_structrp png_ptr, png_fixed_point gamma_val,
};
/* Handle out of range values in release by doing the gamma correction: */
debug(sbits > 0U && sbits <= 16U);
debug_handled(sbits > 0U && sbits <= 16U);
if (sbits == 0U || sbits > 16U)
return 1;

19
pngrutil.c
View File

@ -3329,8 +3329,11 @@ png_combine_row(png_const_structrp png_ptr, png_bytep dp, int display)
switch (pixel_depth)
{
case 1: spixel_rep &= 1; spixel_rep |= spixel_rep << 1;
/*FALL THROUGH*/
case 2: spixel_rep &= 3; spixel_rep |= spixel_rep << 2;
/*FALL THROUGH*/
case 4: spixel_rep &= 15; spixel_rep |= spixel_rep << 4;
/*FALL THROUGH*/
default:
break;
}
@ -3420,8 +3423,11 @@ png_combine_row(png_const_structrp png_ptr, png_bytep dp, int display)
switch (pixel_depth)
{
case 1: spixel_rep &= 1; spixel_rep |= spixel_rep << 1;
/*FALL THROUGH*/
case 2: spixel_rep &= 3; spixel_rep |= spixel_rep << 2;
/*FALL THROUGH*/
case 4: spixel_rep &= 15; spixel_rep |= spixel_rep << 4;
/*FALL THROUGH*/
default:
break;
}
@ -4023,7 +4029,8 @@ png_read_process_IDAT(png_structrp png_ptr)
* once at the start:
*/
png_ptr->zstream.next_out = png_ptr->row_buffer;
state = need_row_bytes;
/* state = need_row_bytes; [not used below] */
/* FALL THROUGH */
case need_row_bytes:
{
@ -4140,7 +4147,7 @@ png_read_process_IDAT(png_structrp png_ptr)
}
} /* need_row_bytes */
state = need_filter_byte;
state = need_filter_byte; /* as opposed to 'start_of_pass' */
/* FALL THROUGH */
case need_filter_byte: /* for the next row */
@ -4376,11 +4383,11 @@ png_read_finish_IDAT(png_structrp png_ptr)
if (!png_ptr->zstream_ended)
{
int end_of_IDAT = png_ptr->zstream.avail_in == 0;
png_byte b;
png_alloc_size_t cb = png_inflate_IDAT(png_ptr, 2/*finish*/, &b, 1);
png_byte b[1];
png_alloc_size_t cb = png_inflate_IDAT(png_ptr, 2/*finish*/, b, 1);
debug(png_ptr->zstream.avail_out == 1-cb &&
png_ptr->zstream.next_out == cb + &b);
png_ptr->zstream.next_out == cb + b);
/* As above, for safety do this: */
png_ptr->zstream.next_out = NULL;
@ -4426,7 +4433,7 @@ png_read_finish_IDAT(png_structrp png_ptr)
/* In fact we expect this to always succeed, so it is a good idea to
* catch it in pre-release builds:
*/
debug(ret == Z_OK);
debug_handled(ret == Z_OK);
if (ret != Z_OK)
{

29
pngtest.c
View File

@ -852,7 +852,8 @@ test_one_file(PNG_CONST char *inname, PNG_CONST char *outname)
png_bytep row_buf;
png_uint_32 y;
png_uint_32 width, height;
int num_pass = 1, pass;
volatile int num_passes;
int pass;
int bit_depth, color_type;
row_buf = NULL;
@ -1052,18 +1053,17 @@ test_one_file(PNG_CONST char *inname, PNG_CONST char *outname)
{
png_set_IHDR(write_ptr, write_info_ptr, width, height, bit_depth,
color_type, interlace_type, compression_type, filter_type);
/* num_pass will not be set below, set it here if the image is
* interlaced: what happens is that write interlacing is *not* turned
* on an the partial interlaced rows are written directly.
/* num_passes may not be available below if interlace support is not
* provided by libpng for both read and write.
*/
switch (interlace_type)
{
case PNG_INTERLACE_NONE:
num_pass = 1;
num_passes = 1;
break;
case PNG_INTERLACE_ADAM7:
num_pass = 7;
num_passes = 7;
break;
default:
@ -1364,9 +1364,16 @@ test_one_file(PNG_CONST char *inname, PNG_CONST char *outname)
#if defined(PNG_READ_DEINTERLACE_SUPPORTED) &&\
defined(PNG_WRITE_INTERLACING_SUPPORTED)
num_pass = png_set_interlace_handling(read_ptr);
if (png_set_interlace_handling(write_ptr) != num_pass)
png_error(write_ptr, "png_set_interlace_handling: inconsistent num_pass");
/* Both must be defined for libpng to be able to handle the interlace,
* otherwise it gets handled below by simply reading and writing the passes
* directly.
*/
if (png_set_interlace_handling(read_ptr) != num_passes)
png_error(write_ptr,
"png_set_interlace_handling(read): wrong pass count ");
if (png_set_interlace_handling(write_ptr) != num_passes)
png_error(write_ptr,
"png_set_interlace_handling(write): wrong pass count ");
#endif
#ifdef PNGTEST_TIMING
@ -1374,7 +1381,7 @@ test_one_file(PNG_CONST char *inname, PNG_CONST char *outname)
t_misc += (t_stop - t_start);
t_start = t_stop;
#endif
for (pass = 0; pass < num_pass; pass++)
for (pass = 0; pass < num_passes; pass++)
{
pngtest_debug1("Writing row data for pass %d", pass);
for (y = 0; y < height; y++)
@ -2007,4 +2014,4 @@ main(void)
#endif
/* Generate a compiler error if there is an old png.h in the search path. */
typedef png_libpng_version_1_7_0beta65 Your_png_h_is_not_version_1_7_0beta65;
typedef png_libpng_version_1_7_0beta66 Your_png_h_is_not_version_1_7_0beta66;

44
pngtrans.c
View File

@ -1534,8 +1534,8 @@ png_do_byte_ops_down(png_transformp *transform, png_transform_controlp tc)
png_bytep dp = png_voidcast(png_bytep, tc->dp);
png_alloc_size_t dest_rowbytes;
debug(tc->bit_depth == 8 || tc->bit_depth == 16);
debug((tc->format & PNG_FORMAT_FLAG_COLORMAP) == 0);
debug(tc->bit_depth == 8U || tc->bit_depth == 16U);
debug((tc->format & PNG_FORMAT_FLAG_COLORMAP) == 0U);
tc->sp = tc->dp;
tc->format = tr->format;
@ -1552,24 +1552,30 @@ png_do_byte_ops_down(png_transformp *transform, png_transform_controlp tc)
unsigned int size, hwm, i;
png_byte output[32];
/* This catches an empty codes array, which would cause all the input to
* be skipped and, potentially, a garbage output[] to be written (once) to
* *dp.
*/
affirm((codes & 0xFU) >= 4U);
/* Align the writes to a 16-byte multiple from the start of the
* destination buffer:
*/
size = dest_rowbytes & 0xFU;
if (size == 0) size = 16;
i = size+16;
if (size == 0U) size = 16U;
i = size+16U;
sp -= sp_advance; /* Move 1 pixel back */
hwm = 0;
hwm = 0U;
for (;;)
{
unsigned int next_code = code & 0xf;
unsigned int next_code = code & 0xFU;
if (next_code >= 8)
output[--i] = sp[next_code-8];
if (next_code >= 8U)
output[--i] = sp[next_code-8U];
else if (next_code >= 4)
output[--i] = tr->filler[next_code - 4];
else if (next_code >= 4U)
output[--i] = tr->filler[next_code - 4U];
else /* end code */
{
@ -1593,8 +1599,8 @@ png_do_byte_ops_down(png_transformp *transform, png_transform_controlp tc)
dp -= size;
hwm ^= 0x10U; /* == i+16 mod 32 */
memcpy(dp, output + hwm, size);
size = 16;
if (i == 0) i = 32;
size = 16U;
if (i == 0U) i = 32U;
}
}
@ -1602,7 +1608,7 @@ png_do_byte_ops_down(png_transformp *transform, png_transform_controlp tc)
* against 'hwm' before and, because of the alignment, i will always be
* either 16 or 32:
*/
debug((i == 16 || i == 32) & (((i & 0x10U)^0x10U) == hwm));
debug((i == 16U || i == 32U) & (((i & 0x10U)^0x10U) == hwm));
debug(sp+sp_advance == ep);
/* At the end the bytes i..(hwm-1) need to be written, with the proviso
@ -1612,24 +1618,26 @@ png_do_byte_ops_down(png_transformp *transform, png_transform_controlp tc)
* bytes will be written (hwm == 0, i == 32) or 16 bytes need to be
* written.
*/
if (size < 16)
if (size < 16U)
{
debug(i == 16);
debug(i == 16U);
dp -= size;
memcpy(dp, output + i, size);
}
else /* size == 16 */
{
debug(size == 16U);
/* Write i..(hwm-1); 16 or 32 bytes, however if 32 bytes are written
* they are contiguous and i==0.
*
* hwm is 0 or 16, i is 16 or 32, swap 0 and 32:
*/
if (hwm == 0) hwm = 32;
if (i == 32) i = 0;
if (hwm == 0U) hwm = 32U;
if (i == 32U) i = 0U;
affirm(i < hwm);
debug(hwm == i+16 || (i == 0 && hwm == 32));
debug(hwm == i+16U || (i == 0U && hwm == 32U));
hwm -= i;
dp -= hwm;