[PATCH] fix bug in lpq parsing

Martin Pool mbp at samba.org
Fri Apr 19 00:34:04 GMT 2002


This patch merges some missing code from 1.2.4.4 on APPLIANCE_HEAD
into head, fixing a bug in the parser for lpq output.  

Basically we're trying to concatenate several fields into a single
string, but the calculation of the amount of space remaining is wrong.
This causes a crash when there are a lot of fields in the output,
because a negative value can be passed as the length parameter to
safe_strcat.

This was originally HP Nautilus CR #430.

Please let me know if it's OK.


Index: lpq_parse.c
===================================================================
RCS file: /data/cvs/samba/source/printing/lpq_parse.c,v
retrieving revision 1.11
diff -u -u -r1.11 lpq_parse.c
--- lpq_parse.c	2002/03/15 08:14:06	1.11
+++ lpq_parse.c	2002/04/19 07:28:02
@@ -149,21 +149,17 @@
   StrnCpy(buf->fs_file,tok[FILETOK],sizeof(buf->fs_file)-1);
 
   if ((FILETOK + 1) != TOTALTOK) {
-    int bufsize;
     int i;
 
-    bufsize = sizeof(buf->fs_file) - strlen(buf->fs_file) - 1;
-
     for (i = (FILETOK + 1); i < TOTALTOK; i++) {
-      safe_strcat(buf->fs_file," ",bufsize);
-      safe_strcat(buf->fs_file,tok[i],bufsize - 1);
-      bufsize = sizeof(buf->fs_file) - strlen(buf->fs_file) - 1;
-      if (bufsize <= 0) {
-        break;
-      }
+      /* FIXME: Using fstrcat rather than other means is a bit
+       * inefficient; this might be a problem for enormous queues with
+       * many fields. */
+      fstrcat(buf->fs_file, " ");
+      fstrcat(buf->fs_file, tok[i]);
     }
     /* Ensure null termination. */
-    buf->fs_file[sizeof(buf->fs_file)-1] = '\0';
+    fstrterminate(buf->fs_file);
   }
 
 #ifdef PRIOTOK
@@ -282,21 +278,17 @@
   StrnCpy(buf->fs_file,tokarr[LPRNG_FILETOK],sizeof(buf->fs_file)-1);
 
   if ((LPRNG_FILETOK + 1) != LPRNG_TOTALTOK) {
-    int bufsize;
     int i;
 
-    bufsize = sizeof(buf->fs_file) - strlen(buf->fs_file) - 1;
-
     for (i = (LPRNG_FILETOK + 1); i < LPRNG_TOTALTOK; i++) {
-      safe_strcat(buf->fs_file," ",bufsize);
-      safe_strcat(buf->fs_file,tokarr[i],bufsize - 1);
-      bufsize = sizeof(buf->fs_file) - strlen(buf->fs_file) - 1;
-      if (bufsize <= 0) {
-        break;
-      }
+      /* FIXME: Using fstrcat rather than other means is a bit
+       * inefficient; this might be a problem for enormous queues with
+       * many fields. */
+      fstrcat(buf->fs_file, " ");
+      fstrcat(buf->fs_file, tokarr[i]);
     }
     /* Ensure null termination. */
-    buf->fs_file[sizeof(buf->fs_file)-1] = '\0';
+    fstrterminate(buf->fs_file);
   }
 
   return(True);

-- 
Martin 




More information about the samba-technical mailing list