From 86c184ff348add8bb4dbb2d9f5df3054c92d0130 Mon Sep 17 00:00:00 2001 From: Michael R Sweet Date: Mon, 21 Jan 2019 16:03:08 -0500 Subject: [PATCH] Clean out some more _cupsStr cruft that might potentially cause an unaligned memory access (Issue #5474) Don't directly use the string pool in the CGI programs or scheduler. --- cgi-bin/admin.c | 12 ++++++------ cgi-bin/var.c | 42 ++++++++++++++++++++---------------------- cups/string.c | 24 ++++++++++++------------ scheduler/ipp.c | 5 ++--- 4 files changed, 40 insertions(+), 43 deletions(-) diff --git a/cgi-bin/admin.c b/cgi-bin/admin.c index 0111d2dd6..f8c7e157b 100644 --- a/cgi-bin/admin.c +++ b/cgi-bin/admin.c @@ -562,7 +562,7 @@ do_am_class(http_t *http, /* I - HTTP connection */ attr = ippAddStrings(request, IPP_TAG_PRINTER, IPP_TAG_URI, "member-uris", num_printers, NULL, NULL); for (i = 0; i < num_printers; i ++) - attr->values[i].string.text = _cupsStrAlloc(cgiGetArray("MEMBER_URIS", i)); + ippSetString(request, &attr, i, cgiGetArray("MEMBER_URIS", i)); } /* @@ -2123,7 +2123,7 @@ do_list_printers(http_t *http) /* I - HTTP connection */ attr; attr = ippFindNextAttribute(response, "device-uri", IPP_TAG_URI)) { - cupsArrayAdd(printer_devices, _cupsStrAlloc(attr->values[0].string.text)); + cupsArrayAdd(printer_devices, strdup(attr->values[0].string.text)); } /* @@ -2261,7 +2261,7 @@ do_list_printers(http_t *http) /* I - HTTP connection */ for (printer_device = (char *)cupsArrayFirst(printer_devices); printer_device; printer_device = (char *)cupsArrayNext(printer_devices)) - _cupsStrFree(printer_device); + free(printer_device); cupsArrayDelete(printer_devices); } @@ -2658,7 +2658,7 @@ do_set_allowed_users(http_t *http) /* I - HTTP connection */ * Add the name... */ - attr->values[i].string.text = _cupsStrAlloc(ptr); + ippSetString(request, &attr, i, ptr); /* * Advance to the next name... @@ -3467,8 +3467,8 @@ do_set_options(http_t *http, /* I - HTTP connection */ attr = ippAddStrings(request, IPP_TAG_PRINTER, IPP_TAG_NAME, "job-sheets-default", 2, NULL, NULL); - attr->values[0].string.text = _cupsStrAlloc(cgiGetVariable("job_sheets_start")); - attr->values[1].string.text = _cupsStrAlloc(cgiGetVariable("job_sheets_end")); + ippSetString(request, &attr, 0, cgiGetVariable("job_sheets_start")); + ippSetString(request, &attr, 1, cgiGetVariable("job_sheets_end")); if ((var = cgiGetVariable("printer_error_policy")) != NULL) ippAddString(request, IPP_TAG_PRINTER, IPP_TAG_NAME, diff --git a/cgi-bin/var.c b/cgi-bin/var.c index 12f3c8344..21b3c5308 100644 --- a/cgi-bin/var.c +++ b/cgi-bin/var.c @@ -1,8 +1,8 @@ /* * CGI form variable and array functions for CUPS. * - * Copyright 2007-2015 by Apple Inc. - * Copyright 1997-2005 by Easy Software Products. + * Copyright © 2007-2019 by Apple Inc. + * Copyright © 1997-2005 by Easy Software Products. * * Licensed under Apache License v2.0. See the file "LICENSE" for more information. */ @@ -29,10 +29,10 @@ typedef struct /**** Form variable structure ****/ { - const char *name; /* Name of variable */ + char *name; /* Name of variable */ int nvalues, /* Number of values */ avalues; /* Number of values allocated */ - const char **values; /* Value(s) of variable */ + char **values; /* Value(s) of variable */ } _cgi_var_t; @@ -135,10 +135,10 @@ cgiClearVariables(void) for (v = form_vars, i = form_count; i > 0; v ++, i --) { - _cupsStrFree(v->name); + free(v->name); for (j = 0; j < v->nvalues; j ++) if (v->values[j]) - _cupsStrFree(v->values[j]); + free(v->values[j]); } form_count = 0; @@ -164,7 +164,7 @@ cgiGetArray(const char *name, /* I - Name of array variable */ if (element < 0 || element >= var->nvalues) return (NULL); - return (_cupsStrRetain(var->values[element])); + return (strdup(var->values[element])); } @@ -222,7 +222,7 @@ cgiGetVariable(const char *name) /* I - Name of variable */ var = cgi_find_variable(name); - return ((var == NULL) ? NULL : _cupsStrRetain(var->values[var->nvalues - 1])); + return ((var == NULL) ? NULL : strdup(var->values[var->nvalues - 1])); } @@ -370,10 +370,9 @@ cgiSetArray(const char *name, /* I - Name of variable */ { if (element >= var->avalues) { - const char **temp; /* Temporary pointer */ + char **temp; /* Temporary pointer */ - temp = (const char **)realloc((void *)(var->values), - sizeof(char *) * (size_t)(element + 16)); + temp = (char **)realloc((void *)(var->values), sizeof(char *) * (size_t)(element + 16)); if (!temp) return; @@ -389,9 +388,9 @@ cgiSetArray(const char *name, /* I - Name of variable */ var->nvalues = element + 1; } else if (var->values[element]) - _cupsStrFree((char *)var->values[element]); + free((char *)var->values[element]); - var->values[element] = _cupsStrAlloc(value); + var->values[element] = strdup(value); } } @@ -448,10 +447,9 @@ cgiSetSize(const char *name, /* I - Name of variable */ if (size >= var->avalues) { - const char **temp; /* Temporary pointer */ + char **temp; /* Temporary pointer */ - temp = (const char **)realloc((void *)(var->values), - sizeof(char *) * (size_t)(size + 16)); + temp = (char **)realloc((void *)(var->values), sizeof(char *) * (size_t)(size + 16)); if (!temp) return; @@ -468,7 +466,7 @@ cgiSetSize(const char *name, /* I - Name of variable */ { for (i = size; i < var->nvalues; i ++) if (var->values[i]) - _cupsStrFree((void *)(var->values[i])); + free((void *)(var->values[i])); } var->nvalues = size; @@ -503,9 +501,9 @@ cgiSetVariable(const char *name, /* I - Name of variable */ { for (i = 0; i < var->nvalues; i ++) if (var->values[i]) - _cupsStrFree((char *)var->values[i]); + free((char *)var->values[i]); - var->values[0] = _cupsStrAlloc(value); + var->values[0] = strdup(value); var->nvalues = 1; } } @@ -548,10 +546,10 @@ cgi_add_variable(const char *name, /* I - Variable name */ if ((var->values = calloc((size_t)element + 1, sizeof(char *))) == NULL) return; - var->name = _cupsStrAlloc(name); + var->name = strdup(name); var->nvalues = element + 1; var->avalues = element + 1; - var->values[element] = _cupsStrAlloc(value); + var->values[element] = strdup(value); form_count ++; } @@ -583,7 +581,7 @@ cgi_find_variable(const char *name) /* I - Name of variable */ if (form_count < 1 || name == NULL) return (NULL); - key.name = name; + key.name = (char *)name; return ((_cgi_var_t *)bsearch(&key, form_vars, (size_t)form_count, sizeof(_cgi_var_t), (int (*)(const void *, const void *))cgi_compare_variables)); diff --git a/cups/string.c b/cups/string.c index e91ba40d4..54f7bd0cf 100644 --- a/cups/string.c +++ b/cups/string.c @@ -1,10 +1,11 @@ /* * String functions for CUPS. * - * Copyright 2007-2014 by Apple Inc. - * Copyright 1997-2007 by Easy Software Products. + * Copyright © 2007-2019 by Apple Inc. + * Copyright © 1997-2007 by Easy Software Products. * - * Licensed under Apache License v2.0. See the file "LICENSE" for more information. + * Licensed under Apache License v2.0. See the file "LICENSE" for more + * information. */ /* @@ -311,15 +312,6 @@ _cupsStrFree(const char *s) /* I - String to free */ key = (_cups_sp_item_t *)(s - offsetof(_cups_sp_item_t, str)); -#ifdef DEBUG_GUARDS - if (key->guard != _CUPS_STR_GUARD) - { - DEBUG_printf(("5_cupsStrFree: Freeing string %p(%s), guard=%08x, " - "ref_count=%d", key, key->str, key->guard, key->ref_count)); - abort(); - } -#endif /* DEBUG_GUARDS */ - if ((item = (_cups_sp_item_t *)cupsArrayFind(stringpool, key)) != NULL && item == key) { @@ -327,6 +319,14 @@ _cupsStrFree(const char *s) /* I - String to free */ * Found it, dereference... */ +#ifdef DEBUG_GUARDS + if (key->guard != _CUPS_STR_GUARD) + { + DEBUG_printf(("5_cupsStrFree: Freeing string %p(%s), guard=%08x, ref_count=%d", key, key->str, key->guard, key->ref_count)); + abort(); + } +#endif /* DEBUG_GUARDS */ + item->ref_count --; if (!item->ref_count) diff --git a/scheduler/ipp.c b/scheduler/ipp.c index 819499a6f..2e3d33a51 100644 --- a/scheduler/ipp.c +++ b/scheduler/ipp.c @@ -2601,8 +2601,7 @@ add_printer(cupsd_client_t *con, /* I - Client connection */ if (!strcmp(attr->values[i].string.text, "none")) continue; - printer->reasons[printer->num_reasons] = - _cupsStrRetain(attr->values[i].string.text); + printer->reasons[printer->num_reasons] = _cupsStrAlloc(attr->values[i].string.text); printer->num_reasons ++; if (!strcmp(attr->values[i].string.text, "paused") && @@ -4892,7 +4891,7 @@ copy_printer_attrs( if ((p2_uri = ippFindAttribute(p2->attrs, "printer-uri-supported", IPP_TAG_URI)) != NULL) { - member_uris->values[i].string.text = _cupsStrRetain(p2_uri->values[0].string.text); + member_uris->values[i].string.text = _cupsStrAlloc(p2_uri->values[0].string.text); } else { -- 2.39.2