Skip to content

implicit widening conversion to type in pngrutil.c #868

Description

@coder7695

clang-tidy static analyzer discovered about 3 warnings related to implicit widening conversion to type in pngrutil.c png_handle_iCCP function.

clang-tidy pngrutil.c -checks='-,bugprone-implicit-widening-of-multiplication-result,security-,-clang-diagnostic-*' -- -I.
3 warnings generated.
/libpng/pngrutil.c:1404:35: warning: performing an implicit widening conversion to type 'png_alloc_size_t' (aka 'unsigned long') of a multiplication performed in type 'png_uint_32' (aka 'unsigned int') [bugprone-implicit-widening-of-multiplication-result]
1404 | size = 12 * tag_count;
| ^
libpng/pngrutil.c:1404:35: note: make conversion explicit to silence this warning
16 | size = 12 * tag_count;
| ^~~~~~~~~~~~~~
| (png_alloc_size_t)( )
libpng/pngrutil.c:1404:35: note: perform multiplication in a wider type
1404 | size = 12 * tag_count;
| ^~
| (png_alloc_size_t)
libpng/pngrutil.c:1422:40: warning: performing an implicit widening conversion to type 'unsigned long' of a multiplication performed in type 'png_uint_32' (aka 'unsigned int') [bugprone-implicit-widening-of-multiplication-result]
1422 | - 12 * tag_count;
| ^
libpng/pngrutil.c:1422:40: note: make conversion explicit to silence this warning
1422 | - 12 * tag_count;
| ^~~~~~~~~~~~~~
| (unsigned long)( )
libpng/pngrutil.c:1422:40: note: perform multiplication in a wider type
1422 | - 12 * tag_count;
| ^~
| (unsigned long)
libpng/pngrutil.c:1426:38: warning: result of multiplication in type 'png_uint_32' (aka 'unsigned int') is used as a pointer offset after an implicit widening conversion to type 'size_t' [bugprone-implicit-widening-of-multiplication-result]
1426 | profile + (sizeof profile_header) +
| ^
libpng/pngrutil.c:1427:38: note: make conversion explicit to silence this warning
1427 | 12 * tag_count, &size, 1/finish/);
| ^~~~~~~~~~~~~~
| (size_t)( )
libpng/pngrutil.c:1427:38: note: perform multiplication in a wider type
1427 | 12 * tag_count, &size, 1/finish/);
| ^~
| (size_t)

To breakdown the issue, 12 is 32bit signed int and tag_count is 32bit unsigned int(png_uint_32) and size (result) is size_t. The multiplication happens in 32bit space and if size is greater than 32bits the leading bits are dropped and the truncated remainder(wraps around) before assignment.
I have put together PR which adds overflow check and also implicitly converts 12 to size_t before multiplication.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions