Simplify bounds checks

We can save a lot of work this way. Also, detect truncation in strlcpy.
OpenPrinting · Jun 3, 2023 · bfaa423 · bfaa423
1 parent ff9492a
commit bfaa423
Show file tree

Hide file tree

Showing 22 changed files with 209 additions and 156 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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);
   }