From 2686c1ff79d95577513584b4ddc0bb149e1f6749 Mon Sep 17 00:00:00 2001 From: Rose Date: Sat, 14 Sep 2024 16:28:48 -0400 Subject: [PATCH] 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. --- cgi-bin/admin.c | 16 ++++++++-------- cgi-bin/cgi.h | 1 + cgi-bin/classes.c | 2 +- cgi-bin/ipp-var.c | 2 +- cgi-bin/libcupscgi.exp | 1 + cgi-bin/printers.c | 2 +- cgi-bin/var.c | 21 ++++++++++++++++++++- 7 files changed, 33 insertions(+), 12 deletions(-) diff --git a/cgi-bin/admin.c b/cgi-bin/admin.c index 0032fbd5aa..9181b570ce 100644 --- 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 index 0f97727789..1f208378fc 100644 --- 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 index eb83a920d6..4e38b15281 100644 --- 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 index 0f00a75118..8bc877717a 100644 --- 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 index 3a26229d79..fa646d9eb2 100644 --- 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 index b20c0a9ff2..f3b99cdb9e 100644 --- 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 index fdc6fa7b57..7297cb8202 100644 --- 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".