Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

security improvements related to integral behavior #9

Closed
wants to merge 2 commits into from
Closed

security improvements related to integral behavior #9

wants to merge 2 commits into from

Conversation

rcseacord
Copy link
Contributor

Trying to make some improvements with respect to integral behavior. So far I have primarily tacked issues regarding logical operations on signed integer values:

png.c line 37
An expression of 'essentially signed' type is being used as the operand of this bitwise operator.

png_ptr->sig_bytes = (png_byte)((num_bytes < 0 ? 0 : num_bytes) & 0xff);

changed num_bytes argument from int to png_size_t here and in png.h

png.c line 483
An expression of 'essentially signed' type is being used as the operand of this bitwise operator.
The operand is constant, 'essentially signed' and negative but will be implicitly converted to an unsigned type in this bitwise operation.
Constant: Negative value implicitly converted to an unsigned type.

  info_ptr->valid &= ~PNG_INFO_tRNS;

Changed the following defines in png.h to be unsigned int literals:

define PNG_INFO_gAMA 0x0001U

define PNG_INFO_sBIT 0x0002U

define PNG_INFO_cHRM 0x0004U

define PNG_INFO_PLTE 0x0008U

define PNG_INFO_tRNS 0x0010U

define PNG_INFO_bKGD 0x0020U

define PNG_INFO_hIST 0x0040U

define PNG_INFO_pHYs 0x0080U

define PNG_INFO_oFFs 0x0100U

define PNG_INFO_tIME 0x0200U

define PNG_INFO_pCAL 0x0400U

define PNG_INFO_sRGB 0x0800U /* GR-P, 0.96a */

define PNG_INFO_iCCP 0x1000U /* ESR, 1.0.6 */

define PNG_INFO_sPLT 0x2000U /* ESR, 1.0.6 */

define PNG_INFO_sCAL 0x4000U /* ESR, 1.0.6 */

define PNG_INFO_IDAT 0x8000U /* ESR, 1.0.6 */


png.c line 483
An expression of 'essentially signed' type is being used as the operand of this bitwise operator.

  mask &= ~PNG_FREE_MUL;

changed PNG_FREE_MUL to unsigned in png.h

png.c lines 875-877
palette[i].red = (png_byte)(v & 0xff);
palette[i].green = (png_byte)(v & 0xff);
palette[i].blue = (png_byte)(v & 0xff);

An expression of 'essentially signed' type is being used as the operand of a bitwise operator.

changed i and v to unsigned.

not fixed yet:

for (i = 0, v = 0; i < num_palette; i++, v += color_inc)

color_inc should be unsigned, but all the assignments would need to be changed'


Robert C. Seacord added 2 commits August 2, 2015 15:30
removed config.h.in~ from the libpng_autotools_files list because it was causing a false positive for missing files.
png_ptr->sig_bytes = (png_byte)((num_bytes < 0 ? 0 : num_bytes) & 0xff);
changed num_bytes argument from int to png_size_t here and in png.h

png.c  line 483

      info_ptr->valid &= ~PNG_INFO_tRNS;

Changed integer literals in png.h to be unsigned int literals to eliminate logical operations on signed values

png.c  line 483

      mask &= ~PNG_FREE_MUL;

Changed PNG_FREE_MUL to unsigned literal in png.h

png.c lines 875-877
      palette[i].red = (png_byte)(v & 0xff);
      palette[i].green = (png_byte)(v & 0xff);
      palette[i].blue = (png_byte)(v & 0xff);

changed i and v to unsigned.
@glennrp
Copy link
Contributor

glennrp commented Aug 15, 2015

Forwarded to png-mng-implement@lists.sf.net for discussion.

@glennrp
Copy link
Contributor

glennrp commented Aug 17, 2015

Note that in example.c, sig_read is an unsigned int. This is inconsistent with the current declaration of png_set_sig_bytes(), which takes a signed int. Changing the prototype for png_set_sig_bytes() would be ABI incompatible, but example.c should be fixed. See discussion on the png-mng-implement list.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants