Create new function cgiGetVariablePtr that avoids allocation

The problem, cgiGetVariable calls strdup on success, which leaks memory all over the place. This is going to be a lot of work to fix, but based on how CoreFoundation fixed this issue, I think just returning a pointer should work. Undefined behavior happens if the user uses the returned pointer to write.
OpenPrinting · Sep 14, 2024 · 2686c1f · 2686c1f
1 parent 7e1a313
commit 2686c1f
Show file tree

Hide file tree

Showing 7 changed files with 33 additions and 12 deletions.
diff --git a/cgi-bin/admin.c b/cgi-bin/admin.c
@@ -90,7 +90,7 @@ main(void)
   * See if we have form data...
   */
 
-  if (!cgiInitialize() || !cgiGetVariable("OP"))
+  if (!cgiInitialize() || !cgiGetVariablePtr("OP"))
   {
    /*
     * Nope, send the administration menu...
@@ -728,10 +728,10 @@ do_am_printer(http_t *http,		/* I - HTTP connection */
       else
         cupsCopyString(make, "Generic", sizeof(make));
 
-      if (!cgiGetVariable("CURRENT_MAKE"))
+      if (!cgiGetVariablePtr("CURRENT_MAKE"))
         cgiSetVariable("CURRENT_MAKE", make);
 
-      if (!cgiGetVariable("CURRENT_MAKE_AND_MODEL"))
+      if (!cgiGetVariablePtr("CURRENT_MAKE_AND_MODEL"))
         cgiSetVariable("CURRENT_MAKE_AND_MODEL", uriptr);
 
       if (!modify)
@@ -848,7 +848,7 @@ do_am_printer(http_t *http,		/* I - HTTP connection */
     cgiCopyTemplateLang("choose-uri.tmpl");
     cgiEndHTML();
   }
-  else if (!strncmp(var, "serial:", 7) && !cgiGetVariable("BAUDRATE"))
+  else if (!strncmp(var, "serial:", 7) && !cgiGetVariablePtr("BAUDRATE"))
   {
    /*
     * Need baud rate, parity, etc.
@@ -925,11 +925,11 @@ do_am_printer(http_t *http,		/* I - HTTP connection */
     return;
   }
   else if (!file &&
-           (!cgiGetVariable("PPD_NAME") || cgiGetVariable("SELECT_MAKE")))
+           (!cgiGetVariablePtr("PPD_NAME") || cgiGetVariable("SELECT_MAKE")))
   {
     int ipp_everywhere = !strncmp(var, "ipp://", 6) || !strncmp(var, "ipps://", 7) || (!strncmp(var, "dnssd://", 8) && (strstr(var, "_ipp._tcp") || strstr(var, "_ipps._tcp")));
 
-    if (modify && !cgiGetVariable("SELECT_MAKE"))
+    if (modify && !cgiGetVariablePtr("SELECT_MAKE"))
     {
      /*
       * Get the PPD file...
@@ -1010,7 +1010,7 @@ do_am_printer(http_t *http,		/* I - HTTP connection */
 
     if ((var = cgiGetVariable("PPD_MAKE")) == NULL)
       var = cgiGetVariable("CURRENT_MAKE");
-    if (var && !cgiGetVariable("SELECT_MAKE"))
+    if (var && !cgiGetVariablePtr("SELECT_MAKE"))
     {
       const char *make_model;		/* Make and model */
 
@@ -1072,7 +1072,7 @@ do_am_printer(http_t *http,		/* I - HTTP connection */
 	*/
 
         cgiStartHTML(title);
-	if (!cgiGetVariable("PPD_MAKE"))
+	if (!cgiGetVariablePtr("PPD_MAKE"))
 	  cgiSetVariable("PPD_MAKE", cgiGetVariable("CURRENT_MAKE"));
         if (ipp_everywhere)
 	  cgiSetVariable("SHOW_IPP_EVERYWHERE", "1");

diff --git a/cgi-bin/cgi.h b/cgi-bin/cgi.h
@@ -78,6 +78,7 @@ extern int		cgiGetSize(const char *name);
 extern char		*cgiGetTemplateDir(void);
 extern const char	*cgiGetTextfield(const char *name);
 extern char		*cgiGetVariable(const char *name);
+extern char		*cgiGetVariablePtr(const char *name);
 extern int		cgiInitialize(void);
 extern int		cgiIsPOST(void);
 extern void		cgiMoveJobs(http_t *http, const char *dest, int job_id);

diff --git a/cgi-bin/classes.c b/cgi-bin/classes.c
@@ -336,7 +336,7 @@ show_all_classes(http_t     *http,	/* I - Connection to server */
     */
 
     if ((var = cgiGetTextfield("QUERY")) != NULL &&
-        !cgiGetVariable("CLEAR"))
+        !cgiGetVariablePtr("CLEAR"))
       search = cgiCompileSearch(var);
     else
       search = NULL;

diff --git a/cgi-bin/ipp-var.c b/cgi-bin/ipp-var.c
@@ -1413,7 +1413,7 @@ cgiShowJobs(http_t     *http,		/* I - Connection to server */
     */
 
     if ((query = cgiGetVariable("QUERY")) != NULL &&
-        !cgiGetVariable("CLEAR"))
+        !cgiGetVariablePtr("CLEAR"))
       search = cgiCompileSearch(query);
     else
     {

diff --git a/cgi-bin/libcupscgi.exp b/cgi-bin/libcupscgi.exp
@@ -16,6 +16,7 @@ _cgiGetIPPObjects
 _cgiGetSize
 _cgiGetTemplateDir
 _cgiGetVariable
+_cgiGetVariablePtr
 _cgiInitialize
 _cgiIsPOST
 _cgiMoveJobs

diff --git a/cgi-bin/printers.c b/cgi-bin/printers.c
@@ -353,7 +353,7 @@ show_all_printers(http_t     *http,	/* I - Connection to server */
     */
 
     if ((var = cgiGetTextfield("QUERY")) != NULL &&
-        !cgiGetVariable("CLEAR"))
+        !cgiGetVariablePtr("CLEAR"))
       search = cgiCompileSearch(var);
     else
       search = NULL;

diff --git a/cgi-bin/var.c b/cgi-bin/var.c
@@ -296,7 +296,8 @@ cgiGetTextfield(const char *name)	/* I - Name of form field */
  * 'cgiGetVariable()' - Get a CGI variable from the database.
  *
  * Returns NULL if the variable doesn't exist.  If the variable is an
- * array of values, returns the last element.
+ * array of values, returns the last element. This value is duplicated,
+ * so be sure to free it.
  */
 
 char *					/* O - Value of variable */
@@ -310,6 +311,24 @@ cgiGetVariable(const char *name)	/* I - Name of variable */
   return ((var == NULL) ? NULL : strdup(var->values[var->nvalues - 1]));
 }
 
+/*
+ * 'cgiGetVariablePtr()' - Get a CGI variable from the database.
+ *
+ * Obtains a pointer to the C-string buffer containing the variable.
+ * Useful for checking a variable exists.
+ */
+
+char *					/* O - Value of variable */
+cgiGetVariablePtr(const char *name)	/* I - Name of variable */
+{
+  const _cgi_var_t	*var;		/* Returned variable */
+
+
+  var = cgi_find_variable(name);
+
+  return ((var == NULL) ? NULL : var->values[var->nvalues - 1]);
+}
+
 
 /*
  * 'cgiInitialize()' - Initialize the CGI variable "database".