From bfaa4237440d319ae3e17f97b1d164df592735d4 Mon Sep 17 00:00:00 2001 From: Rose <83477269+AtariDreams@users.noreply.github.com> Date: Thu, 27 Apr 2023 12:14:29 -0400 Subject: [PATCH] Simplify bounds checks We can save a lot of work this way. Also, detect truncation in strlcpy. --- backend/dnssd.c | 30 ++++++++++++++++------------- backend/network.c | 15 ++++++++------- cgi-bin/ipp-var.c | 12 ++++++------ cgi-bin/template.c | 6 +++--- cgi-bin/var.c | 12 ++++++------ cups/debug.c | 12 ++++++------ cups/dest-options.c | 9 ++++++++- cups/dest.c | 17 +++++++++++++---- cups/encode.c | 5 ++--- cups/http-addr.c | 5 +++-- cups/http-addrlist.c | 44 ++++++++++++++++++++++--------------------- cups/options.c | 5 +++-- cups/tls-darwin.c | 29 +++++++++++++++++----------- cups/tls-gnutls.c | 23 ++++++++++++++-------- cups/tls-sspi.c | 13 +++++++++---- scheduler/auth.c | 9 +++++---- scheduler/dirsvc.c | 3 ++- scheduler/log.c | 13 ++++++------- scheduler/printers.c | 19 +++++++------------ tools/ippeveprinter.c | 26 ++++++++++++------------- tools/ippfind.c | 25 +++++++++++++++--------- tools/ipptool.c | 33 +++++++++++++++++++------------- 22 files changed, 209 insertions(+), 156 deletions(-) diff --git a/backend/dnssd.c b/backend/dnssd.c index 14a6772fb7..bd49080533 100644 --- a/backend/dnssd.c +++ b/backend/dnssd.c @@ -1094,12 +1094,13 @@ query_callback( if (!_cups_strncasecmp(key, "usb_", 4)) { + size_t device_id_len = strlen(device_id); /* * Add USB device ID information... */ - ptr = device_id + strlen(device_id); - snprintf(ptr, sizeof(device_id) - (size_t)(ptr - device_id), "%s:%s;", key + 4, value); + ptr = device_id + device_id_len; + snprintf(ptr, sizeof(device_id) - device_id_len, "%s:%s;", key + 4, value); } if (!_cups_strcasecmp(key, "usb_MFG") || !_cups_strcasecmp(key, "usb_MANU") || @@ -1111,14 +1112,14 @@ query_callback( { if (value[0] == '(') { - /* - * Strip parenthesis... - */ - - if ((ptr = value + strlen(value) - 1) > value && *ptr == ')') - *ptr = '\0'; - - strlcpy(model, value + 1, sizeof(model)); + size_t val_len = strlen(value); + /* + * Strip parenthesis... + */ + if (val_len > 1 && value[val_len - 1] == ')') + value[val_len - 1] = '\0'; + + strlcpy(model, value + 1, sizeof(model)); } else strlcpy(model, value, sizeof(model)); @@ -1198,7 +1199,7 @@ query_callback( char *valptr = value + strlen(value); /* Pointer into value */ - if (valptr < (value + sizeof(value) - 1)) + if (valptr < (value + (sizeof(value) - 1))) *valptr++ = ','; ptr += 6; @@ -1213,8 +1214,11 @@ query_callback( *valptr = '\0'; } - ptr = device_id + strlen(device_id); - snprintf(ptr, sizeof(device_id) - (size_t)(ptr - device_id), "CMD:%s;", value + 1); + { + size_t device_len = strlen(device_id); + ptr = device_id + device_len; + snprintf(ptr, sizeof(device_id) - device_len, "CMD:%s;", value + 1); + } } if (device_id[0]) diff --git a/backend/network.c b/backend/network.c index f7ee2fbbe3..4bcd4b1957 100644 --- a/backend/network.c +++ b/backend/network.c @@ -170,6 +170,7 @@ backendNetworkSideCB( const char *snmp_count; /* CUPS_SNMP_COUNT env var */ int count; /* Repetition count */ + datalen = (int)strlen(data); if ((snmp_count = getenv("CUPS_SNMP_COUNT")) != NULL) { if ((count = atoi(snmp_count)) <= 0) @@ -178,16 +179,16 @@ backendNetworkSideCB( else count = 1; - for (dataptr = data + strlen(data) + 1; - count > 0 && dataptr < (data + sizeof(data) - 1); + for (dataptr = data + datalen + 1; + count > 0 && dataptr < (data + (sizeof(data) - 1)); count --, dataptr += strlen(dataptr)) strlcpy(dataptr, snmp_value, sizeof(data) - (size_t)(dataptr - data)); - + + datalen = (int)(dataptr - data); fprintf(stderr, "DEBUG: Returning %s %s\n", data, - data + strlen(data) + 1); + data + datalen); status = CUPS_SC_STATUS_OK; - datalen = (int)(dataptr - data); break; } @@ -210,7 +211,7 @@ backendNetworkSideCB( { if (_cupsSNMPRead(snmp_fd, &packet, 1.0)) { - size_t i; /* Looping var */ + unsigned i; /* Looping var */ if (!_cupsSNMPOIDToString(packet.object_name, data, sizeof(data))) @@ -240,7 +241,7 @@ backendNetworkSideCB( if (packet.object_value.string.num_bytes < (sizeof(data) - (size_t)(dataptr - data))) i = packet.object_value.string.num_bytes; else - i = sizeof(data) - (size_t)(dataptr - data); + i = (unsigned)sizeof(data) - (size_t)(dataptr - data); memcpy(dataptr, packet.object_value.string.bytes, i); diff --git a/cgi-bin/ipp-var.c b/cgi-bin/ipp-var.c index 939675e064..7bd1f4e5a7 100644 --- a/cgi-bin/ipp-var.c +++ b/cgi-bin/ipp-var.c @@ -97,13 +97,13 @@ cgiGetAttributes(ipp_t *request, /* I - IPP request */ break; else if (nameptr > name && ch == '?') break; - else if (nameptr < (name + sizeof(name) - 1)) - { - if (ch == '_') + else if (nameptr < (name + (sizeof(name) - 1))) + { + if (ch == '_') *nameptr++ = '-'; else *nameptr++ = (char)ch; - } + } *nameptr = '\0'; @@ -875,7 +875,7 @@ cgiRewriteURL(const char *uri, /* I - Current URI */ *resptr++ = hexchars[*rawptr & 15]; } } - else if (resptr < (resource + sizeof(resource) - 1)) + else if (resptr < (resource + (sizeof(resource) - 1))) *resptr++ = *rawptr; *resptr = '\0'; @@ -967,7 +967,7 @@ cgiSetIPPObjectVars( else nameptr = name; - for (i = 0; attr->name[i] && nameptr < (name + sizeof(name) - 1); i ++) + for (i = 0; attr->name[i] && nameptr < (name + (sizeof(name) - 1)); i++) if (attr->name[i] == '-') *nameptr++ = '_'; else diff --git a/cgi-bin/template.c b/cgi-bin/template.c index 33d738e9c9..b4672e7505 100644 --- a/cgi-bin/template.c +++ b/cgi-bin/template.c @@ -249,7 +249,7 @@ cgi_copy(FILE *out, /* I - Output file */ uriencode = 1; else if (s > name && ch == '?') break; - else if (s < (name + sizeof(name) - 1)) + else if (s < (name + (sizeof(name) - 1))) *s++ = (char)ch; *s = '\0'; @@ -454,8 +454,8 @@ cgi_copy(FILE *out, /* I - Output file */ for (s = compare; (ch = getc(in)) != EOF;) if (ch == '?') break; - else if (s >= (compare + sizeof(compare) - 1)) - continue; + else if (s >= (compare + (sizeof(compare) - 1))) + continue; else if (ch == '#') { snprintf(s, sizeof(compare) - (size_t)(s - compare), "%d", element + 1); diff --git a/cgi-bin/var.c b/cgi-bin/var.c index ed7a98e85e..fb95734b48 100644 --- a/cgi-bin/var.c +++ b/cgi-bin/var.c @@ -651,7 +651,7 @@ cgi_initialize_cookies(void) */ for (ptr = name; *cookie && *cookie != '=';) - if (ptr < (name + sizeof(name) - 1)) + if (ptr < (name + (sizeof(name) - 1))) { *ptr++ = *cookie++; } @@ -674,7 +674,7 @@ cgi_initialize_cookies(void) if (*cookie == '\"') { for (cookie ++, ptr = value; *cookie && *cookie != '\"';) - if (ptr < (value + sizeof(value) - 1)) + if (ptr < (value + (sizeof(value) - 1))) { *ptr++ = *cookie++; } @@ -692,7 +692,7 @@ cgi_initialize_cookies(void) else { for (ptr = value; *cookie && *cookie != ';';) - if (ptr < (value + sizeof(value) - 1)) + if (ptr < (value + (sizeof(value) - 1))) { *ptr++ = *cookie++; } @@ -1103,7 +1103,7 @@ cgi_initialize_string(const char *data) /* I - Form data string */ break; case '+' : /* Escaped space character */ - if (s < (value + sizeof(value) - 1)) + if (s < (value + (sizeof(value) - 1))) *s++ = ' '; break; @@ -1115,7 +1115,7 @@ cgi_initialize_string(const char *data) /* I - Form data string */ if (!isxdigit(data[1] & 255) || !isxdigit(data[2] & 255)) return (0); - if (s < (value + sizeof(value) - 1)) + if (s < (value + (sizeof(value) - 1))) { data ++; ch = *data - '0'; @@ -1134,7 +1134,7 @@ cgi_initialize_string(const char *data) /* I - Form data string */ break; default : /* Other characters come straight through */ - if (*data >= ' ' && s < (value + sizeof(value) - 1)) + if (*data >= ' ' && s < (value + (sizeof(value) - 1))) *s++ = *data; break; } diff --git a/cups/debug.c b/cups/debug.c index 6b3914ed51..ee6f130cff 100644 --- a/cups/debug.c +++ b/cups/debug.c @@ -407,7 +407,7 @@ _cups_safe_vsnprintf( while (isdigit(*format & 255)) { - if (tptr < (tformat + sizeof(tformat) - 1)) + if (tptr < (tformat + (sizeof(tformat) - 1))) *tptr++ = *format; width = width * 10 + *format++ - '0'; @@ -416,7 +416,7 @@ _cups_safe_vsnprintf( if (*format == '.') { - if (tptr < (tformat + sizeof(tformat) - 1)) + if (tptr < (tformat + (sizeof(tformat) - 1))) *tptr++ = *format; format ++; @@ -439,7 +439,7 @@ _cups_safe_vsnprintf( while (isdigit(*format & 255)) { - if (tptr < (tformat + sizeof(tformat) - 1)) + if (tptr < (tformat + (sizeof(tformat) - 1))) *tptr++ = *format; prec = prec * 10 + *format++ - '0'; @@ -461,8 +461,8 @@ _cups_safe_vsnprintf( } else if (*format == 'h' || *format == 'l' || *format == 'L') { - if (tptr < (tformat + sizeof(tformat) - 1)) - *tptr++ = *format; + if (tptr < (tformat + (sizeof(tformat) - 1))) + *tptr++ = *format; size = *format++; } @@ -472,7 +472,7 @@ _cups_safe_vsnprintf( if (!*format) break; - if (tptr < (tformat + sizeof(tformat) - 1)) + if (tptr < (tformat + (sizeof(tformat) - 1))) *tptr++ = *format; type = *format++; diff --git a/cups/dest-options.c b/cups/dest-options.c index d0d683d7a6..6789bbaa4b 100644 --- a/cups/dest-options.c +++ b/cups/dest-options.c @@ -1933,11 +1933,18 @@ cups_create_defaults( for (attr = ippFirstAttribute(dinfo->attrs); attr; attr = ippNextAttribute(dinfo->attrs)) { + size_t temp; if (!ippGetName(attr) || ippGetGroupTag(attr) != IPP_TAG_PRINTER) continue; strlcpy(name, ippGetName(attr), sizeof(name)); - if ((nameptr = name + strlen(name) - 8) <= name || strcmp(nameptr, "-default")) + + temp = strlen(name); + if (temp <= 8) + continue; + + nameptr = name + (temp - 8); + if (strcmp(nameptr, "-default")) continue; *nameptr = '\0'; diff --git a/cups/dest.c b/cups/dest.c index 741aae0631..0abd272b84 100644 --- a/cups/dest.c +++ b/cups/dest.c @@ -3080,14 +3080,20 @@ cups_dnssd_query_cb( { if (value[0] == '(') { + size_t value_len = strlen(value); + /* * Strip parenthesis... */ - if ((ptr = value + strlen(value) - 1) > value && *ptr == ')') - *ptr = '\0'; - - strlcpy(model, value + 1, sizeof(model)); + if (value_len > 1) + { + if (value[value_len - 1] == ')') + value[value_len - 1] = '\0'; + strlcpy(model, value + 1, sizeof(model)); + } + else + model[0] = '\0'; } else strlcpy(model, value, sizeof(model)); @@ -4425,6 +4431,9 @@ cups_queue_name( const char *ptr; /* Pointer into serviceName */ char *nameptr; /* Pointer into name */ + if (namesize == 0) { + return; + } for (nameptr = name, ptr = serviceName; *ptr && nameptr < (name + namesize - 1); ptr ++) { diff --git a/cups/encode.c b/cups/encode.c index 7f0c1d641e..ea0ed55153 100644 --- a/cups/encode.c +++ b/cups/encode.c @@ -862,10 +862,9 @@ cupsEncodeOptions2( const char * /* O - First out-of-order option or NULL */ _ippCheckOptions(void) { - int i; /* Looping var */ + size_t i; /* Looping var */ - - for (i = 0; i < (int)(sizeof(ipp_options) / sizeof(ipp_options[0]) - 1); i ++) + for (i = 0; i < sizeof(ipp_options) / sizeof(ipp_options[0]) - 1; i++) if (strcmp(ipp_options[i].name, ipp_options[i + 1].name) >= 0) return (ipp_options[i + 1].name); diff --git a/cups/http-addr.c b/cups/http-addr.c index 6aeeb80748..796b0361df 100644 --- a/cups/http-addr.c +++ b/cups/http-addr.c @@ -795,6 +795,7 @@ httpGetHostname(http_t *http, /* I - HTTP connection or NULL */ char *s, /* I - String buffer for name */ int slen) /* I - Size of buffer */ { + size_t temp_len; /* Len of string s (not to be confused with buffer size) */ if (http) { if (!s || slen <= 1) @@ -872,8 +873,8 @@ httpGetHostname(http_t *http, /* I - HTTP connection or NULL */ /* * Make sure .local hostnames end with a period... */ - - if (strlen(s) > 6 && !strcmp(s + strlen(s) - 6, ".local")) + temp_len = strlen(s); + if (temp_len > 6 && !strcmp(s + temp_len - 6, ".local")) strlcat(s, ".", (size_t)slen); } diff --git a/cups/http-addrlist.c b/cups/http-addrlist.c index 4fde80c5f7..01515a7d20 100644 --- a/cups/http-addrlist.c +++ b/cups/http-addrlist.c @@ -59,13 +59,20 @@ httpAddrConnect2( { int val; /* Socket option value */ #ifndef _WIN32 - int i, j, /* Looping vars */ - flags, /* Socket flags */ - result; /* Result from select() or poll() */ + unsigned i, j; /* Looping vars */ + int flags, /* Socket flags */ + result; /* Result from select() or poll() */ #endif /* !_WIN32 */ int remaining; /* Remaining timeout */ + +# ifdef _WIN32 int nfds, /* Number of file descriptors */ fds[100]; /* Socket file descriptors */ +#else + nfds_t nfds; /* Number of file descriptors */ + int fds[100]; /* Socket file descriptors */ +#endif + http_addrlist_t *addrs[100]; /* Addresses */ #ifndef HAVE_POLL int max_fd = -1; /* Highest file descriptor */ @@ -119,14 +126,13 @@ httpAddrConnect2( { while (nfds > 0) { - nfds --; - httpAddrClose(NULL, fds[nfds]); + httpAddrClose(NULL, fds[--nfds]); } return (NULL); } - if (addrlist && nfds < (int)(sizeof(fds) / sizeof(fds[0]))) + if (addrlist && nfds < (sizeof(fds) / sizeof(fds[0]))) { /* * Create the socket... @@ -207,8 +213,7 @@ httpAddrConnect2( while (nfds > 0) { - nfds --; - httpAddrClose(NULL, fds[nfds]); + httpAddrClose(NULL, fds[--nfds]); } return (addrlist); @@ -235,8 +240,7 @@ httpAddrConnect2( max_fd = fds[nfds]; #endif /* !HAVE_POLL */ - addrs[nfds] = addrlist; - nfds ++; + addrs[nfds++] = addrlist; addrlist = addrlist->next; } @@ -269,8 +273,7 @@ httpAddrConnect2( while (nfds > 0) { - nfds --; - httpAddrClose(NULL, fds[nfds]); + httpAddrClose(NULL, fds[--nfds]); } *sock = -1; @@ -285,7 +288,7 @@ httpAddrConnect2( pfds[i].events = POLLIN | POLLOUT; } - result = poll(pfds, (nfds_t)nfds, addrlist ? 100 : remaining > 250 ? 250 : remaining); + result = poll(pfds, nfds, addrlist ? 100 : remaining > 250 ? 250 : remaining); DEBUG_printf(("1httpAddrConnect2: poll() returned %d (%d)", result, errno)); @@ -317,7 +320,7 @@ httpAddrConnect2( for (i = 0; i < nfds; i ++) { # ifdef HAVE_POLL - DEBUG_printf(("pfds[%d].revents=%x\n", i, pfds[i].revents)); + DEBUG_printf(("pfds[%u].revents=%x\n", i, pfds[i].revents)); if (pfds[i].revents && !(pfds[i].revents & (POLLERR | POLLHUP))) # else if (FD_ISSET(fds[i], &input_set) && !FD_ISSET(fds[i], &error_set)) @@ -396,8 +399,7 @@ httpAddrConnect2( while (nfds > 0) { - nfds --; - httpAddrClose(NULL, fds[nfds]); + httpAddrClose(NULL, fds[--nfds]); } #ifdef _WIN32 @@ -503,7 +505,7 @@ httpAddrGetList(const char *hostname, /* I - Hostname, IP address, or NULL for p *temp; /* New address */ char ipv6[64], /* IPv6 address */ *ipv6zone; /* Pointer to zone separator */ - int ipv6len; /* Length of IPv6 address */ + size_t ipv6len; /* Length of IPv6 address */ _cups_globals_t *cg = _cupsGlobals(); /* Global data */ @@ -598,9 +600,9 @@ httpAddrGetList(const char *hostname, /* I - Hostname, IP address, or NULL for p */ strlcpy(ipv6, hostname + 4, sizeof(ipv6)); - if ((ipv6len = (int)strlen(ipv6) - 1) >= 0 && ipv6[ipv6len] == ']') + if ((ipv6len = strlen(ipv6)) && ipv6[ipv6len - 1] == ']') { - ipv6[ipv6len] = '\0'; + ipv6[ipv6len - 1] = '\0'; hostname = ipv6; /* @@ -618,9 +620,9 @@ httpAddrGetList(const char *hostname, /* I - Hostname, IP address, or NULL for p */ strlcpy(ipv6, hostname + 1, sizeof(ipv6)); - if ((ipv6len = (int)strlen(ipv6) - 1) >= 0 && ipv6[ipv6len] == ']') + if ((ipv6len = strlen(ipv6)) && ipv6[ipv6len - 1] == ']') { - ipv6[ipv6len] = '\0'; + ipv6[ipv6len - 1] = '\0'; hostname = ipv6; } } diff --git a/cups/options.c b/cups/options.c index 4fcc281b10..de5403c3f6 100644 --- a/cups/options.c +++ b/cups/options.c @@ -300,13 +300,14 @@ cupsParseOptions( if (*copyarg == '{') { + size_t copyarg_len = strlen(copyarg); /* * Remove surrounding {} so we can parse "{name=value ... name=value}"... */ - if ((ptr = copyarg + strlen(copyarg) - 1) > copyarg && *ptr == '}') + if (copyarg_len > 1 && copyarg[copyarg_len - 1] == '}') { - *ptr = '\0'; + copyarg[copyarg_len - 1] = '\0'; ptr = copyarg + 1; } else diff --git a/cups/tls-darwin.c b/cups/tls-darwin.c index fb5caac071..04033e0235 100644 --- a/cups/tls-darwin.c +++ b/cups/tls-darwin.c @@ -1223,8 +1223,7 @@ _httpTLSSetOptions(int options, /* I - Options */ int /* O - 0 on success, -1 on failure */ _httpTLSStart(http_t *http) /* I - HTTP connection */ { - char hostname[256], /* Hostname */ - *hostptr; /* Pointer into hostname */ + char hostname[256]; /* Hostname */ _cups_globals_t *cg = _cupsGlobals(); /* Pointer to library globals */ OSStatus error; /* Error code */ @@ -1580,13 +1579,16 @@ _httpTLSStart(http_t *http) /* I - HTTP connection */ if (!error && http->mode == _HTTP_MODE_CLIENT) { - /* - * Client: get the hostname to use for TLS... - */ + size_t host_len; + + /* + * Client: get the hostname to use for TLS... + */ if (httpAddrLocalhost(http->hostaddr)) { - strlcpy(hostname, "localhost", sizeof(hostname)); + strcpy(hostname, "localhost"); + host_len = 9; } else { @@ -1594,13 +1596,18 @@ _httpTLSStart(http_t *http) /* I - HTTP connection */ * Otherwise make sure the hostname we have does not end in a trailing dot. */ - strlcpy(hostname, http->hostname, sizeof(hostname)); - if ((hostptr = hostname + strlen(hostname) - 1) >= hostname && - *hostptr == '.') - *hostptr = '\0'; + host_len = strlcpy(hostname, http->hostname, sizeof(hostname)); + if (hostlen) + { + if (host_len > sizeof(hostname) - 1) // Did truncation occur? + hostlen = strlen(hostname); + + if (hostname[host_len - 1] == '.') + hostname[host_len - 1] = '\0'; + } } - error = SSLSetPeerDomainName(http->tls, hostname, strlen(hostname)); + error = SSLSetPeerDomainName(http->tls, hostname, host_len); DEBUG_printf(("4_httpTLSStart: SSLSetPeerDomainName, error=%d", (int)error)); } diff --git a/cups/tls-gnutls.c b/cups/tls-gnutls.c index b1fc298bf9..c894bff3dd 100644 --- a/cups/tls-gnutls.c +++ b/cups/tls-gnutls.c @@ -1261,8 +1261,7 @@ _httpTLSSetOptions(int options, /* I - Options */ int /* O - 0 on success, -1 on failure */ _httpTLSStart(http_t *http) /* I - Connection to server */ { - char hostname[256], /* Hostname */ - *hostptr; /* Pointer into hostname */ + char hostname[256]; /* Hostname */ int status; /* Status of handshake */ gnutls_certificate_credentials_t *credentials; /* TLS credentials */ @@ -1338,13 +1337,16 @@ _httpTLSStart(http_t *http) /* I - Connection to server */ if (http->mode == _HTTP_MODE_CLIENT) { + size_t host_len; + /* * Client: get the hostname to use for TLS... */ if (httpAddrLocalhost(http->hostaddr)) { - strlcpy(hostname, "localhost", sizeof(hostname)); + strcpy(hostname, "localhost"); + host_len = 9; } else { @@ -1352,13 +1354,18 @@ _httpTLSStart(http_t *http) /* I - Connection to server */ * Otherwise make sure the hostname we have does not end in a trailing dot. */ - strlcpy(hostname, http->hostname, sizeof(hostname)); - if ((hostptr = hostname + strlen(hostname) - 1) >= hostname && - *hostptr == '.') - *hostptr = '\0'; + host_len = strlcpy(hostname, http->hostname, sizeof(hostname)); + if (host_len) + { + if (host_len > sizeof(hostname) - 1) // Did truncation occur? + host_len = strlen(hostname); + + if (hostname[host_len - 1] == '.') + hostname[host_len - 1] = '\0'; + } } - status = gnutls_server_name_set(http->tls, GNUTLS_NAME_DNS, hostname, strlen(hostname)); + status = gnutls_server_name_set(http->tls, GNUTLS_NAME_DNS, hostname, host_len); } else { diff --git a/cups/tls-sspi.c b/cups/tls-sspi.c index fee309298c..27d336874b 100644 --- a/cups/tls-sspi.c +++ b/cups/tls-sspi.c @@ -962,10 +962,15 @@ _httpTLSStart(http_t *http) /* I - HTTP connection */ * Otherwise make sure the hostname we have does not end in a trailing dot. */ - strlcpy(hostname, http->hostname, sizeof(hostname)); - if ((hostptr = hostname + strlen(hostname) - 1) >= hostname && - *hostptr == '.') - *hostptr = '\0'; + size_t host_len = strlcpy(hostname, http->hostname, sizeof(hostname)); + if (host_len) + { + if (host_len > sizeof(hostname) - 1) // Did truncation occur? + host_len = strlen(hostname); + + if (hostname[host_len - 1] == '.') + hostname[host_len - 1] = '\0'; + } } return (http_sspi_client(http, hostname)); diff --git a/scheduler/auth.c b/scheduler/auth.c index 7f6c3f6d2f..00d0d1e06e 100644 --- a/scheduler/auth.c +++ b/scheduler/auth.c @@ -1414,8 +1414,9 @@ cupsdFindBest(const char *path, /* I - Resource path */ if ((uriptr = strchr(uri, '?')) != NULL) *uriptr = '\0'; /* Drop trailing query string */ - if ((uriptr = uri + strlen(uri) - 1) > uri && *uriptr == '/') - *uriptr = '\0'; /* Remove trailing '/' */ + bestlen = strlen(uri); + if (bestlen > 1 && uri[bestlen - 1] == '/') + uri[bestlen - 1] = '\0'; /* Remove trailing '/' */ if (!strncmp(uri, "/printers/", 10) || !strncmp(uri, "/classes/", 9)) @@ -1424,7 +1425,7 @@ cupsdFindBest(const char *path, /* I - Resource path */ * Check if the URI has .ppd on the end... */ - uriptr = uri + strlen(uri) - 4; /* len > 4 if we get here... */ + uriptr = uri + bestlen - 4; /* len > 4 if we get here... */ if (!strcmp(uriptr, ".ppd")) *uriptr = '\0'; @@ -1445,7 +1446,7 @@ cupsdFindBest(const char *path, /* I - Resource path */ loc; loc = (cupsd_location_t *)cupsArrayNext(Locations)) { - cupsdLogMessage(CUPSD_LOG_DEBUG2, "cupsdFindBest: Location %s(%d) Limit %x", loc->location ? loc->location : "(null)", (int)loc->length, loc->limit); + cupsdLogMessage(CUPSD_LOG_DEBUG2, "cupsdFindBest: Location %s(%u) Limit %x", loc->location ? loc->location : "(null)", (unsigned)loc->length, loc->limit); if (!strncmp(uri, "/printers/", 10) || !strncmp(uri, "/classes/", 9)) { diff --git a/scheduler/dirsvc.c b/scheduler/dirsvc.c index a438765a5e..9f47bf0cfa 100644 --- a/scheduler/dirsvc.c +++ b/scheduler/dirsvc.c @@ -350,11 +350,12 @@ dnssdBuildTxtRecord( if (strchr(DNSSDHostName, '.')) { + size_t host_len = strlen(DNSSDHostName); /* * Use the provided hostname, but make sure it ends with a period... */ - if ((ptr = DNSSDHostName + strlen(DNSSDHostName) - 1) >= DNSSDHostName && *ptr == '.') + if (host_len && DNSSDHostName[host_len - 1] == '.') strlcpy(admin_hostname, DNSSDHostName, sizeof(admin_hostname)); else snprintf(admin_hostname, sizeof(admin_hostname), "%s.", DNSSDHostName); diff --git a/scheduler/log.c b/scheduler/log.c index 8aafb66b72..af9fddb46b 100644 --- a/scheduler/log.c +++ b/scheduler/log.c @@ -153,8 +153,7 @@ cupsdCheckLogFile(cups_file_t **lf, /* IO - Log file */ filename[0] = '\0'; for (logptr = logname, ptr = filename + strlen(filename); - *logptr && ptr < (filename + sizeof(filename) - 1); - logptr ++) + *logptr && ptr < (filename + (sizeof(filename) - 1)); logptr++) if (*logptr == '%') { /* @@ -798,7 +797,7 @@ cupsdLogPage(cupsd_job_t *job, /* I - Job being printed */ switch (*format) { case '%' : /* Literal % */ - if (bufptr < (buffer + sizeof(buffer) - 1)) + if (bufptr < (buffer + (sizeof(buffer) - 1))) *bufptr++ = '%'; break; @@ -872,7 +871,7 @@ cupsdLogPage(cupsd_job_t *job, /* I - Job being printed */ for (i = 0; i < attr->num_values && - bufptr < (buffer + sizeof(buffer) - 1); + bufptr < (buffer + (sizeof(buffer) - 1)); i ++) { if (i) @@ -931,13 +930,13 @@ cupsdLogPage(cupsd_job_t *job, /* I - Job being printed */ } } } - else if (bufptr < (buffer + sizeof(buffer) - 1)) + else if (bufptr < (buffer + (sizeof(buffer) - 1))) *bufptr++ = '-'; break; } default : - if (bufptr < (buffer + sizeof(buffer) - 2)) + if (bufptr < (buffer + (sizeof(buffer) - 2))) { *bufptr++ = '%'; *bufptr++ = *format; @@ -945,7 +944,7 @@ cupsdLogPage(cupsd_job_t *job, /* I - Job being printed */ break; } } - else if (bufptr < (buffer + sizeof(buffer) - 1)) + else if (bufptr < (buffer + (sizeof(buffer) - 1))) *bufptr++ = *format; } diff --git a/scheduler/printers.c b/scheduler/printers.c index 5f9852e64f..dc70facf0c 100644 --- a/scheduler/printers.c +++ b/scheduler/printers.c @@ -1581,9 +1581,7 @@ cupsdSaveAllPrinters(void) snprintf(value, sizeof(value), "%s ", marker->name); for (i = 0, ptr = value + strlen(value); - i < marker->num_values && ptr < (value + sizeof(value) - 1); - i ++) - { + i < marker->num_values && ptr < (value + (sizeof(value) - 1)); i++) { if (i) *ptr++ = ','; @@ -1640,13 +1638,11 @@ cupsdSaveAllPrinters(void) snprintf(value, sizeof(value), "%s ", marker->name); for (i = 0, ptr = value + strlen(value); - i < marker->num_values && ptr < (value + sizeof(value) - 1); - i ++) - { + i < marker->num_values && ptr < (value + (sizeof(value) - 1)); i++) { if (i) *ptr++ = ','; - strlcpy(ptr, marker->values[i].string.text, (size_t)(value + sizeof(value) - ptr)); + strlcpy(ptr, marker->values[i].string.text, (size_t)(sizeof(value) - (ptr - value))); ptr += strlen(ptr); } @@ -1660,13 +1656,12 @@ cupsdSaveAllPrinters(void) snprintf(value, sizeof(value), "%s ", marker->name); for (i = 0, ptr = value + strlen(value); - i < marker->num_values && ptr < (value + sizeof(value) - 1); - i ++) + i < marker->num_values && ptr < (value + (sizeof(value) - 1)); i++) { if (i) *ptr++ = ','; - strlcpy(ptr, marker->values[i].string.text, (size_t)(value + sizeof(value) - ptr)); + strlcpy(ptr, marker->values[i].string.text, (size_t)(sizeof(value) - (ptr - value))); ptr += strlen(ptr); } @@ -2148,7 +2143,7 @@ cupsdSetPrinterAttr( level = ippGetInteger(levels, i); type = ippGetString(types, i, NULL); - for (psptr = pstype; *type && psptr < (pstype + sizeof(pstype) - 1); type ++) + for (psptr = pstype; *type && psptr < (pstype + (sizeof(pstype) - 1)); type++) if (*type == '-') { type ++; @@ -2594,7 +2589,7 @@ cupsdSetPrinterReasons( sptr ++; for (rptr = reason; *sptr && !isspace(*sptr & 255) && *sptr != ','; sptr ++) - if (rptr < (reason + sizeof(reason) - 1)) + if (rptr < (reason + (sizeof(reason) - 1))) *rptr++ = *sptr; if (rptr == reason) diff --git a/tools/ippeveprinter.c b/tools/ippeveprinter.c index 041a468ef7..08e3c51161 100644 --- a/tools/ippeveprinter.c +++ b/tools/ippeveprinter.c @@ -1253,7 +1253,7 @@ create_job_file( if ((job_name = ippGetString(ippFindAttribute(job->attrs, "job-name", IPP_TAG_NAME), 0, NULL)) == NULL) job_name = "untitled"; - for (nameptr = name; *job_name && nameptr < (name + sizeof(name) - 1); job_name ++) + for (nameptr = name; *job_name && nameptr < (name + (sizeof(name) - 1)); job_name++) { if (isalnum(*job_name & 255) || *job_name == '-') { @@ -3012,7 +3012,7 @@ html_printf(ippeve_client_t *client, /* I - Client */ while (isdigit(*format & 255)) { - if (tptr < (tformat + sizeof(tformat) - 1)) + if (tptr < (tformat + (sizeof(tformat) - 1))) *tptr++ = *format; width = width * 10 + *format++ - '0'; @@ -3021,8 +3021,8 @@ html_printf(ippeve_client_t *client, /* I - Client */ if (*format == '.') { - if (tptr < (tformat + sizeof(tformat) - 1)) - *tptr++ = *format; + if (tptr < (tformat + (sizeof(tformat) - 1))) + *tptr++ = *format; format ++; @@ -3044,7 +3044,7 @@ html_printf(ippeve_client_t *client, /* I - Client */ while (isdigit(*format & 255)) { - if (tptr < (tformat + sizeof(tformat) - 1)) + if (tptr < (tformat + (sizeof(tformat) - 1))) *tptr++ = *format; prec = prec * 10 + *format++ - '0'; @@ -3066,8 +3066,8 @@ html_printf(ippeve_client_t *client, /* I - Client */ } else if (*format == 'h' || *format == 'l' || *format == 'L') { - if (tptr < (tformat + sizeof(tformat) - 1)) - *tptr++ = *format; + if (tptr < (tformat + (sizeof(tformat) - 1))) + *tptr++ = *format; size = *format++; } @@ -3081,7 +3081,7 @@ html_printf(ippeve_client_t *client, /* I - Client */ break; } - if (tptr < (tformat + sizeof(tformat) - 1)) + if (tptr < (tformat + (sizeof(tformat) - 1))) *tptr++ = *format; type = *format++; @@ -4875,7 +4875,7 @@ load_legacy_attributes( ptr += strlen(ptr); prefix = ","; } - if (ptr < (device_id + sizeof(device_id) - 1)) + if (ptr < (device_id + (sizeof(device_id) - 1))) { *ptr++ = ';'; *ptr = '\0'; @@ -7208,13 +7208,13 @@ register_printer( if (!strcasecmp(value, "application/octet-stream")) continue; - if (ptr > formats && ptr < (formats + sizeof(formats) - 1)) + if (ptr > formats && ptr < (formats + (sizeof(formats) - 1))) *ptr++ = ','; strlcpy(ptr, value, sizeof(formats) - (size_t)(ptr - formats)); ptr += strlen(ptr); - if (ptr >= (formats + sizeof(formats) - 1)) + if (ptr >= (formats + (sizeof(formats) - 1))) break; } @@ -7223,13 +7223,13 @@ register_printer( { value = ippGetString(urf_supported, i, NULL); - if (ptr > urf && ptr < (urf + sizeof(urf) - 1)) + if (ptr > urf && ptr < (urf + (sizeof(urf) - 1))) *ptr++ = ','; strlcpy(ptr, value, sizeof(urf) - (size_t)(ptr - urf)); ptr += strlen(ptr); - if (ptr >= (urf + sizeof(urf) - 1)) + if (ptr >= (urf + (sizeof(urf) - 1))) break; } diff --git a/tools/ippfind.c b/tools/ippfind.c index 2e4272653d..3c9274226b 100644 --- a/tools/ippfind.c +++ b/tools/ippfind.c @@ -1980,7 +1980,7 @@ exec_program(ippfind_srv_t *service, /* I - Service */ */ for (kptr = keyword, ptr ++; *ptr && *ptr != '}'; ptr ++) - if (kptr < (keyword + sizeof(keyword) - 1)) + if (kptr < (keyword + (sizeof(keyword) - 1))) *kptr++ = *ptr; if (*ptr != '}') @@ -2022,7 +2022,7 @@ exec_program(ippfind_srv_t *service, /* I - Service */ tptr += strlen(tptr); } - else if (tptr < (temp + sizeof(temp) - 1)) + else if (tptr < (temp + (sizeof(temp) - 1))) *tptr++ = *ptr; } @@ -2567,6 +2567,7 @@ resolve_callback( uint8_t valueLen; /* Length of value */ ippfind_srv_t *service = (ippfind_srv_t *)context; /* Service */ + size_t host_len; /* Length of host */ /* @@ -2590,9 +2591,12 @@ resolve_callback( service->host = strdup(hostTarget); service->port = ntohs(port); - value = service->host + strlen(service->host) - 1; - if (value >= service->host && *value == '.') - *value = '\0'; + host_len = strlen(service->host); + + if (host_len > 1 && service->host[host_len - 1] == '.') + { + service->host[host_len - 1] = '\0'; + } /* * Loop through the TXT key/value pairs and add them to an array... @@ -2648,7 +2652,7 @@ resolve_callback( ippfind_srv_t *service = (ippfind_srv_t *)context; /* Service */ AvahiStringList *current; /* Current TXT key/value pair */ - + size_t host_len; /* Length of host */ (void)address; @@ -2665,9 +2669,12 @@ resolve_callback( service->host = strdup(hostTarget); service->port = port; - value = service->host + strlen(service->host) - 1; - if (value >= service->host && *value == '.') - *value = '\0'; + host_len = strlen(service->host); + + if (host_len > 1 && service->host[host_len - 1] == '.') + { + service->host[host_len - 1] = '\0'; + } /* * Loop through the TXT key/value pairs and add them to an array... diff --git a/tools/ipptool.c b/tools/ipptool.c index 98f6657fab..60e9ec974b 100644 --- a/tools/ipptool.c +++ b/tools/ipptool.c @@ -803,13 +803,14 @@ compare_uris(const char *a, /* I - First URI */ auserpass[256], ahost[256], aresource[256]; + size_t aport_len; /* Size of aport */ int aport; char bscheme[32], /* Components of second URI */ buserpass[256], bhost[256], bresource[256]; - int bport; - char *ptr; /* Pointer into string */ + size_t bport_len; /* Size of bport */ + int bport; int result; /* Result of comparison */ @@ -827,11 +828,13 @@ compare_uris(const char *a, /* I - First URI */ * Strip trailing dots from the host components, if present... */ - if ((ptr = ahost + strlen(ahost) - 1) > ahost && *ptr == '.') - *ptr = '\0'; + aport_len = strlen(ahost); + if (aport_len > 1 && ahost[aport_len - 1] == '.') + ahost[aport_len - 1] = '\0'; - if ((ptr = bhost + strlen(bhost) - 1) > bhost && *ptr == '.') - *ptr = '\0'; + bport_len = strlen(bhost); + if (bport_len > 1 && ahost[bport_len - 1] == '.') + bhost[bport_len - 1] = '\0'; /* * Compare each component... @@ -2574,12 +2577,12 @@ get_string(ipp_attribute_t *attr, /* I - IPP attribute */ size_t bufsize) /* I - Size of temporary buffer */ { const char *value; /* Value */ - char *ptr, /* Pointer into value */ - scheme[256], /* URI scheme */ + char scheme[256], /* URI scheme */ userpass[256], /* Username/password */ hostname[256], /* Hostname */ resource[1024]; /* Resource */ int port; /* Port number */ + size_t len; /* Length of buffers */ value = ippGetString(attr, element, NULL); @@ -2587,11 +2590,14 @@ get_string(ipp_attribute_t *attr, /* I - IPP attribute */ if (flags & IPPTOOL_WITH_HOSTNAME) { if (httpSeparateURI(HTTP_URI_CODING_ALL, value, scheme, sizeof(scheme), userpass, sizeof(userpass), buffer, (int)bufsize, &port, resource, sizeof(resource)) < HTTP_URI_STATUS_OK) + { buffer[0] = '\0'; + return buffer; + } - ptr = buffer + strlen(buffer) - 1; - if (ptr >= buffer && *ptr == '.') - *ptr = '\0'; /* Drop trailing "." */ + len = strlen(buffer);; + if (len && buffer[len - 1] == '.') + buffer[len - 1] = '\0'; /* Drop trailing "." */ return (buffer); } @@ -2627,8 +2633,9 @@ get_string(ipp_attribute_t *attr, /* I - IPP attribute */ * Normalize URI with no trailing dot... */ - if ((ptr = hostname + strlen(hostname) - 1) >= hostname && *ptr == '.') - *ptr = '\0'; + len = strlen(hostname); + if (len && hostname[len - 1] == '.') + hostname[len - 1] = '\0'; httpAssembleURI(HTTP_URI_CODING_ALL, buffer, (int)bufsize, scheme, userpass, hostname, port, resource); }