Avoid a memory leak when allocation of a pCAL buffer fails

Set the PNG_FREE_PCAL flag immediately after the allocation of the
first pCAL buffer, so that, if any one of the subsequent allocations
fail, all pCAL buffers still get deallocated by png_free_data.

Also reorder the initialization of other PNG_FREE_ flags (without
altering the semantics) to improve the overall consistency inside
the pngset.c module.

Co-authored-by: Philippe Antoine <contact@catenacyber.fr>
Signed-off-by: Cosmin Truta <ctruta@gmail.com>

2 files changed, 13 insertions, 19 deletions

diff --git a/AUTHORS b/AUTHORS
index 9a8e3869e..350a2de5d 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -22,6 +22,7 @@ Authors, for copyright and licensing purposes.
  * Mike Klein
  * Pascal Massimino
  * Paul Schmidt
+ * Philippe Antoine
  * Qiang Zhou
  * Sam Bushell
  * Samuel Williams
diff --git a/pngset.c b/pngset.c
index 8c372cf41..8ce01dff7 100644
--- a/pngset.c
+++ b/pngset.c
@@ -1,7 +1,7 @@
 
 /* pngset.c - storage of image information into info struct
  *
- * Copyright (c) 2018-2022 Cosmin Truta
+ * Copyright (c) 2018-2023 Cosmin Truta
  * Copyright (c) 1998-2018 Glenn Randers-Pehrson
  * Copyright (c) 1996-1997 Andreas Dilger
  * Copyright (c) 1995-1996 Guy Eric Schalnat, Group 42, Inc.
@@ -172,11 +172,10 @@ png_set_eXIf_1(png_const_structrp png_ptr, png_inforp info_ptr,
       return;
    }
 
-   info_ptr->free_me |= PNG_FREE_EXIF;
-
    for (i = 0; i < (int) info_ptr->num_exif; i++)
       info_ptr->exif[i] = eXIf_buf[i];
 
+   info_ptr->free_me |= PNG_FREE_EXIF;
    info_ptr->valid |= PNG_INFO_eXIf;
 }
 #endif /* eXIf */
@@ -237,15 +236,13 @@ png_set_hIST(png_const_structrp png_ptr, png_inforp info_ptr,
    if (info_ptr->hist == NULL)
    {
       png_warning(png_ptr, "Insufficient memory for hIST chunk data");
-
       return;
    }
 
-   info_ptr->free_me |= PNG_FREE_HIST;
-
    for (i = 0; i < info_ptr->num_palette; i++)
       info_ptr->hist[i] = hist[i];
 
+   info_ptr->free_me |= PNG_FREE_HIST;
    info_ptr->valid |= PNG_INFO_hIST;
 }
 #endif
@@ -367,6 +364,8 @@ png_set_pCAL(png_const_structrp png_ptr, png_inforp info_ptr,
 
    memcpy(info_ptr->pcal_purpose, purpose, length);
 
+   info_ptr->free_me |= PNG_FREE_PCAL;
+
    png_debug(3, "storing X0, X1, type, and nparams in info");
    info_ptr->pcal_X0 = X0;
    info_ptr->pcal_X1 = X1;
@@ -383,7 +382,6 @@ png_set_pCAL(png_const_structrp png_ptr, png_inforp info_ptr,
    if (info_ptr->pcal_units == NULL)
    {
       png_warning(png_ptr, "Insufficient memory for pCAL units");
-
       return;
    }
 
@@ -395,7 +393,6 @@ png_set_pCAL(png_const_structrp png_ptr, png_inforp info_ptr,
    if (info_ptr->pcal_params == NULL)
    {
       png_warning(png_ptr, "Insufficient memory for pCAL params");
-
       return;
    }
 
@@ -413,7 +410,6 @@ png_set_pCAL(png_const_structrp png_ptr, png_inforp info_ptr,
       if (info_ptr->pcal_params[i] == NULL)
       {
          png_warning(png_ptr, "Insufficient memory for pCAL parameter");
-
          return;
       }
 
@@ -421,7 +417,6 @@ png_set_pCAL(png_const_structrp png_ptr, png_inforp info_ptr,
    }
 
    info_ptr->valid |= PNG_INFO_pCAL;
-   info_ptr->free_me |= PNG_FREE_PCAL;
 }
 #endif
 
@@ -478,18 +473,17 @@ png_set_sCAL_s(png_const_structrp png_ptr, png_inforp info_ptr,
 
    if (info_ptr->scal_s_height == NULL)
    {
-      png_free (png_ptr, info_ptr->scal_s_width);
+      png_free(png_ptr, info_ptr->scal_s_width);
       info_ptr->scal_s_width = NULL;
 
       png_warning(png_ptr, "Memory allocation failed while processing sCAL");
-
       return;
    }
 
    memcpy(info_ptr->scal_s_height, sheight, lengthh);
 
-   info_ptr->valid |= PNG_INFO_sCAL;
    info_ptr->free_me |= PNG_FREE_SCAL;
+   info_ptr->valid |= PNG_INFO_sCAL;
 }
 
 #  ifdef PNG_FLOATING_POINT_SUPPORTED
@@ -625,11 +619,10 @@ png_set_PLTE(png_structrp png_ptr, png_inforp info_ptr,
    if (num_palette > 0)
       memcpy(png_ptr->palette, palette, (unsigned int)num_palette *
           (sizeof (png_color)));
+
    info_ptr->palette = png_ptr->palette;
    info_ptr->num_palette = png_ptr->num_palette = (png_uint_16)num_palette;
-
    info_ptr->free_me |= PNG_FREE_PLTE;
-
    info_ptr->valid |= PNG_INFO_PLTE;
 }
 
@@ -1020,8 +1013,8 @@ png_set_tRNS(png_structrp png_ptr, png_inforp info_ptr,
               png_malloc(png_ptr, PNG_MAX_PALETTE_LENGTH));
           memcpy(info_ptr->trans_alpha, trans_alpha, (size_t)num_trans);
 
-          info_ptr->valid |= PNG_INFO_tRNS;
           info_ptr->free_me |= PNG_FREE_TRNS;
+          info_ptr->valid |= PNG_INFO_tRNS;
        }
        png_ptr->trans_alpha = info_ptr->trans_alpha;
    }
@@ -1054,8 +1047,8 @@ png_set_tRNS(png_structrp png_ptr, png_inforp info_ptr,
 
    if (num_trans != 0)
    {
-      info_ptr->valid |= PNG_INFO_tRNS;
       info_ptr->free_me |= PNG_FREE_TRNS;
+      info_ptr->valid |= PNG_INFO_tRNS;
    }
 }
 #endif
@@ -1089,11 +1082,11 @@ png_set_sPLT(png_const_structrp png_ptr,
    {
       /* Out of memory or too many chunks */
       png_chunk_report(png_ptr, "too many sPLT chunks", PNG_CHUNK_WRITE_ERROR);
-
       return;
    }
 
    png_free(png_ptr, info_ptr->splt_palettes);
+
    info_ptr->splt_palettes = np;
    info_ptr->free_me |= PNG_FREE_SPLT;
 
@@ -1247,11 +1240,11 @@ png_set_unknown_chunks(png_const_structrp png_ptr,
    {
       png_chunk_report(png_ptr, "too many unknown chunks",
           PNG_CHUNK_WRITE_ERROR);
-
       return;
    }
 
    png_free(png_ptr, info_ptr->unknown_chunks);
+
    info_ptr->unknown_chunks = np; /* safe because it is initialized */
    info_ptr->free_me |= PNG_FREE_UNKN;