From: Rose <83477269+AtariDreams@users.noreply.github.com>
Date: Fri, 20 Jan 2023 17:47:21 +0000 (-0500)
Subject: Improve state handling in cases of memory allocation failure
X-Git-Tag: v2.4.3~27^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6fe25f7c48ab6ef9becaba5f901c898820b9970b;p=thirdparty%2Fcups.git

Improve state handling in cases of memory allocation failure

Many lines of code assume that malloc will not fail. In cases where it does, sometimes the program does not know, and as a result, memory can leak and more disastrous consequences can happen before the program ultimately finds something is wrong and then calls exit();
---

diff --git a/backend/dnssd.c b/backend/dnssd.c
index f8d582a5c2..f1f4bc1e15 100644
--- a/backend/dnssd.c
+++ b/backend/dnssd.c
@@ -887,7 +887,7 @@ get_device(cups_array_t *devices,	/* I - Device array */
   * Yes, add the device...
   */
 
-  if ((device = calloc(sizeof(cups_device_t), 1)) == NULL)
+  if ((device = calloc(1, sizeof(cups_device_t))) == NULL)
   {
     perror("DEBUG: Out of memory adding a device");
     return (NULL);
diff --git a/cgi-bin/var.c b/cgi-bin/var.c
index 8c8387fa24..ed7a98e85e 100644
--- a/cgi-bin/var.c
+++ b/cgi-bin/var.c
@@ -553,14 +553,27 @@ cgi_add_variable(const char *name,	/* I - Variable name */
     if (!temp_vars)
       return;
 
-    form_vars  = temp_vars;
-    form_alloc += 16;
-  }
+    var = temp_vars + form_count;
 
-  var = form_vars + form_count;
+    if ((var->values = calloc((size_t)element + 1, sizeof(char *))) == NULL)
+    {
+      /* 
+       * Rollback changes
+       */
 
-  if ((var->values = calloc((size_t)element + 1, sizeof(char *))) == NULL)
-    return;
+      if (form_alloc == 0)
+        free(temp_vars);
+      return;
+    }
+    form_vars = temp_vars;
+    form_alloc += 16;
+  }
+  else
+  {
+    var = form_vars + form_count;
+    if ((var->values = calloc((size_t)element + 1, sizeof(char *))) == NULL)
+      return;
+  }
 
   var->name            = strdup(name);
   var->nvalues         = element + 1;
diff --git a/cups/dest.c b/cups/dest.c
index 6d8e08763a..741aae0631 100644
--- a/cups/dest.c
+++ b/cups/dest.c
@@ -293,7 +293,7 @@ cupsAddDest(const char  *name,		/* I  - Destination name */
       * Copy options from parent...
       */
 
-      dest->options = calloc(sizeof(cups_option_t), (size_t)parent->num_options);
+      dest->options = calloc((size_t)parent->num_options, sizeof(cups_option_t));
 
       if (dest->options)
       {
@@ -849,7 +849,7 @@ cupsCopyDest(cups_dest_t *dest,         /* I  - Destination to copy */
   {
     new_dest->is_default = dest->is_default;
 
-    if ((new_dest->options = calloc(sizeof(cups_option_t), (size_t)dest->num_options)) == NULL)
+    if ((new_dest->options = calloc((size_t)dest->num_options, sizeof(cups_option_t))) == NULL)
       return (cupsRemoveDest(dest->name, dest->instance, num_dests, dests));
 
     new_dest->num_options = dest->num_options;
@@ -2828,7 +2828,7 @@ cups_dnssd_get_device(
                   !strcmp(regtype, "_ipps._tcp") ? "IPPS" : "IPP",
                   replyDomain));
 
-    if ((device = calloc(sizeof(_cups_dnssd_device_t), 1)) == NULL)
+    if ((device = calloc(1, sizeof(_cups_dnssd_device_t))) == NULL)
       return (NULL);
 
     device->dest.name = _cupsStrAlloc(name);
diff --git a/cups/globals.c b/cups/globals.c
index 521b336be9..ddeab667c3 100644
--- a/cups/globals.c
+++ b/cups/globals.c
@@ -180,7 +180,7 @@ DllMain(HINSTANCE hinst,		/* I - DLL module handle */
 static _cups_globals_t *		/* O - Pointer to global data */
 cups_globals_alloc(void)
 {
-  _cups_globals_t *cg = malloc(sizeof(_cups_globals_t));
+  _cups_globals_t *cg = calloc(1, sizeof(_cups_globals_t));
 					/* Pointer to global data */
 #ifdef _WIN32
   HKEY		key;			/* Registry key */
@@ -200,7 +200,6 @@ cups_globals_alloc(void)
   * callback values...
   */
 
-  memset(cg, 0, sizeof(_cups_globals_t));
   cg->encryption     = (http_encryption_t)-1;
   cg->password_cb    = (cups_password_cb2_t)_cupsGetPassword;
   cg->trust_first    = -1;
diff --git a/cups/http.c b/cups/http.c
index ef0e26d100..c7e6e3bc37 100644
--- a/cups/http.c
+++ b/cups/http.c
@@ -4000,7 +4000,7 @@ http_create(
   * Allocate memory for the structure...
   */
 
-  if ((http = calloc(sizeof(http_t), 1)) == NULL)
+  if ((http = calloc(1, sizeof(http_t))) == NULL)
   {
     _cupsSetError(IPP_STATUS_ERROR_INTERNAL, strerror(errno), 0);
     httpAddrFreeList(myaddrlist);
diff --git a/cups/ipp.c b/cups/ipp.c
index 4db4e06263..23179d6fab 100644
--- a/cups/ipp.c
+++ b/cups/ipp.c
@@ -6170,8 +6170,8 @@ ipp_add_attr(ipp_t      *ipp,		/* I - IPP message */
   else
     alloc_values = (num_values + IPP_MAX_VALUES - 1) & ~(IPP_MAX_VALUES - 1);
 
-  attr = calloc(sizeof(ipp_attribute_t) +
-                (size_t)(alloc_values - 1) * sizeof(_ipp_value_t), 1);
+  attr = calloc(1, sizeof(ipp_attribute_t) +
+                (size_t)(alloc_values - 1) * sizeof(_ipp_value_t));
 
   if (attr)
   {
diff --git a/cups/language.c b/cups/language.c
index 7e7aed1967..96e828332f 100644
--- a/cups/language.c
+++ b/cups/language.c
@@ -799,7 +799,7 @@ cupsLangGet(const char *language)	/* I - Language or locale */
     * Allocate memory for the language and add it to the cache.
     */
 
-    if ((lang = calloc(sizeof(cups_lang_t), 1)) == NULL)
+    if ((lang = calloc(1, sizeof(cups_lang_t))) == NULL)
     {
       _cupsMutexUnlock(&lang_mutex);
 
diff --git a/cups/ppd-cache.c b/cups/ppd-cache.c
index 2489ee313c..21a5d2113a 100644
--- a/cups/ppd-cache.c
+++ b/cups/ppd-cache.c
@@ -1636,7 +1636,7 @@ _ppdCacheCreateWithPPD(ppd_file_t *ppd)	/* I - PPD file */
 
 	  num_options = pc->num_presets[_PWG_PRINT_COLOR_MODE_COLOR]
 					[pwg_print_quality];
-	  options     = calloc(sizeof(cups_option_t), (size_t)num_options);
+	  options     = calloc((size_t)num_options, sizeof(cups_option_t));
 
 	  if (options)
 	  {
diff --git a/cups/ppd-emit.c b/cups/ppd-emit.c
index 8bffb2bc36..74908d451c 100644
--- a/cups/ppd-emit.c
+++ b/cups/ppd-emit.c
@@ -98,14 +98,14 @@ ppdCollect2(ppd_file_t    *ppd,		/* I - PPD file data */
   */
 
   count = 0;
-  if ((collect = calloc(sizeof(ppd_choice_t *),
-                        (size_t)cupsArrayCount(ppd->marked))) == NULL)
+  if ((collect = calloc((size_t)cupsArrayCount(ppd->marked),
+                        sizeof(ppd_choice_t *))) == NULL)
   {
     *choices = NULL;
     return (0);
   }
 
-  if ((orders = calloc(sizeof(float), (size_t)cupsArrayCount(ppd->marked))) == NULL)
+  if ((orders = calloc((size_t)cupsArrayCount(ppd->marked), sizeof(float))) == NULL)
   {
     *choices = NULL;
     free(collect);
diff --git a/cups/raster-stream.c b/cups/raster-stream.c
index aea71b0338..3260cd7b7a 100644
--- a/cups/raster-stream.c
+++ b/cups/raster-stream.c
@@ -448,7 +448,7 @@ _cupsRasterNew(
 
   _cupsRasterClearError();
 
-  if ((r = calloc(sizeof(cups_raster_t), 1)) == NULL)
+  if ((r = calloc(1, sizeof(cups_raster_t))) == NULL)
   {
     _cupsRasterAddError("Unable to allocate memory for raster stream: %s\n",
                         strerror(errno));
diff --git a/cups/tls-sspi.c b/cups/tls-sspi.c
index f925122f27..fee309298c 100644
--- a/cups/tls-sspi.c
+++ b/cups/tls-sspi.c
@@ -1232,7 +1232,7 @@ _httpTLSWrite(http_t     *http,		/* I - HTTP connection */
 static _http_sspi_t *			/* O  - New SSPI/SSL object */
 http_sspi_alloc(void)
 {
-  return ((_http_sspi_t *)calloc(sizeof(_http_sspi_t), 1));
+  return ((_http_sspi_t *)calloc(1, sizeof(_http_sspi_t)));
 }
 
 
diff --git a/scheduler/cert.c b/scheduler/cert.c
index 470543be4d..8f3829c22c 100644
--- a/scheduler/cert.c
+++ b/scheduler/cert.c
@@ -52,7 +52,7 @@ cupsdAddCert(int        pid,		/* I - Process ID */
   * Allocate memory for the certificate...
   */
 
-  if ((cert = calloc(sizeof(cupsd_cert_t), 1)) == NULL)
+  if ((cert = calloc(1, sizeof(cupsd_cert_t))) == NULL)
     return;
 
  /*
diff --git a/scheduler/cups-driverd.cxx b/scheduler/cups-driverd.cxx
index f487d32ceb..4834fd37c8 100644
--- a/scheduler/cups-driverd.cxx
+++ b/scheduler/cups-driverd.cxx
@@ -2403,7 +2403,12 @@ load_ppds(const char *d,		/* I - Actual directory */
   * Nope, add it to the Inodes array and continue...
   */
 
-  dinfoptr = (struct stat *)malloc(sizeof(struct stat));
+  if ((dinfoptr = (struct stat *)malloc(sizeof(struct stat))) == NULL)
+  {
+    fputs("ERROR: [cups-driverd] Unable to allocate memory for directory info.\n",
+          stderr);
+    exit(1);
+  }
   memcpy(dinfoptr, &dinfo, sizeof(struct stat));
   cupsArrayAdd(Inodes, dinfoptr);
 
@@ -2625,11 +2630,10 @@ load_ppds_dat(char   *filename,		/* I - Filename buffer */
       {
 	if ((ppd = (ppd_info_t *)calloc(1, sizeof(ppd_info_t))) == NULL)
 	{
-	  if (verbose)
-	    fputs("ERROR: [cups-driverd] Unable to allocate memory for PPD!\n",
-		  stderr);
-	  exit(1);
-	}
+    fputs("ERROR: [cups-driverd] Unable to allocate memory for PPD!\n",
+          stderr);
+    exit(1);
+  }
 
 	if (cupsFileRead(fp, (char *)&(ppd->record), sizeof(ppd_rec_t)) > 0)
 	{
diff --git a/scheduler/job.c b/scheduler/job.c
index eada754f37..0659b2b85f 100644
--- a/scheduler/job.c
+++ b/scheduler/job.c
@@ -148,7 +148,7 @@ cupsdAddJob(int        priority,	/* I - Job priority */
   cupsd_job_t	*job;			/* New job record */
 
 
-  if ((job = calloc(sizeof(cupsd_job_t), 1)) == NULL)
+  if ((job = calloc(1, sizeof(cupsd_job_t))) == NULL)
     return (NULL);
 
   job->id              = NextJobId ++;
@@ -4648,7 +4648,7 @@ load_request_root(void)
       * Allocate memory for the job...
       */
 
-      if ((job = calloc(sizeof(cupsd_job_t), 1)) == NULL)
+      if ((job = calloc(1, sizeof(cupsd_job_t))) == NULL)
       {
         cupsdLogMessage(CUPSD_LOG_ERROR, "Ran out of memory for jobs.");
 	cupsDirClose(dir);
diff --git a/tools/ippfind.c b/tools/ippfind.c
index d3c72f1d03..2e4272653d 100644
--- a/tools/ippfind.c
+++ b/tools/ippfind.c
@@ -1931,7 +1931,7 @@ exec_program(ippfind_srv_t *service,	/* I - Service */
     if (strncmp(environ[i], "IPPFIND_", 8))
       myenvc ++;
 
-  if ((myenvp = calloc(sizeof(char *), (size_t)(myenvc + 1))) == NULL)
+  if ((myenvp = calloc((size_t)(myenvc + 1), sizeof(char *))) == NULL)
   {
     _cupsLangPuts(stderr, _("ippfind: Out of memory."));
     exit(IPPFIND_EXIT_MEMORY);
@@ -1956,7 +1956,7 @@ exec_program(ippfind_srv_t *service,	/* I - Service */
   * Allocate and copy command-line arguments...
   */
 
-  if ((myargv = calloc(sizeof(char *), (size_t)(num_args + 1))) == NULL)
+  if ((myargv = calloc((size_t)(num_args + 1), sizeof(char *))) == NULL)
   {
     _cupsLangPuts(stderr, _("ippfind: Out of memory."));
     exit(IPPFIND_EXIT_MEMORY);
@@ -2163,7 +2163,7 @@ get_service(cups_array_t *services,	/* I - Service array */
   * Yes, add the service...
   */
 
-  if ((service = calloc(sizeof(ippfind_srv_t), 1)) == NULL)
+  if ((service = calloc(1, sizeof(ippfind_srv_t))) == NULL)
     return (NULL);
 
   service->name     = strdup(serviceName);
@@ -2499,9 +2499,12 @@ new_expr(ippfind_op_t op,		/* I - Operation */
       if (!strcmp(args[num_args], ";"))
         break;
 
-     temp->num_args = num_args;
-     temp->args     = malloc((size_t)num_args * sizeof(char *));
-     memcpy(temp->args, args, (size_t)num_args * sizeof(char *));
+    temp->num_args = num_args;
+    temp->args     = malloc((size_t)num_args * sizeof(char *));
+    if (temp->args == NULL)
+      return (NULL);
+
+    memcpy(temp->args, args, (size_t)num_args * sizeof(char *));
   }
 
   return (temp);