snprintf improvements for mg

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

snprintf improvements for mg

Han Boetes
Hi,

Here is a patch for mg which improves the use of snprintf for mg.
I replaced snprintf with strlcat where possible and added sanity
checks for the others.

Have a special look at the patch for undo.c

While busy I also added some #endif /* comments */


Index: buffer.c
===================================================================
RCS file: /cvs/src/usr.bin/mg/buffer.c,v
retrieving revision 1.50
diff -u -p -r1.50 buffer.c
--- buffer.c 2005/10/14 19:46:46 1.50
+++ buffer.c 2005/11/12 19:10:05
@@ -426,13 +426,12 @@ anycb(int f)
 {
  BUFFER *bp;
  int s = FALSE, save = FALSE;
- char prompt[NFILEN + 11];
+ char prompt[NFILEN + 11] = "Save file ";
 
  for (bp = bheadp; bp != NULL; bp = bp->b_bufp) {
  if (bp->b_fname != NULL && *(bp->b_fname) != '\0' &&
     (bp->b_flag & BFCHG) != 0) {
- snprintf(prompt, sizeof(prompt), "Save file %s",
-    bp->b_fname);
+ strlcat(prompt, bp->b_fname, sizeof(prompt));
  if ((f == TRUE || (save = eyorn(prompt)) == TRUE) &&
     buffsave(bp) == TRUE) {
  bp->b_flag &= ~BFCHG;
Index: dired.c
===================================================================
RCS file: /cvs/src/usr.bin/mg/dired.c,v
retrieving revision 1.28
diff -u -p -r1.28 dired.c
--- dired.c 2005/11/07 23:46:18 1.28
+++ dired.c 2005/11/12 19:10:06
@@ -588,7 +588,7 @@ dired_(char *dirname)
 {
  BUFFER *bp;
  FILE *dirpipe;
- char line[256];
+ char line[256] = "ls -la ";
  int len;
 
  if ((dirname = adjustname(dirname)) == NULL) {
@@ -608,8 +608,7 @@ dired_(char *dirname)
  if (bclear(bp) != TRUE)
  return (NULL);
  bp->b_flag |= BFREADONLY;
- if (snprintf(line, sizeof(line), "ls -al %s", dirname)
-    >= sizeof(line)) {
+ if (strlcat(line, dirname, sizeof(line)) >= sizeof(line)) {
  ewprintf("Path too long");
  return (NULL);
  }
Index: fileio.c
===================================================================
RCS file: /cvs/src/usr.bin/mg/fileio.c,v
retrieving revision 1.60
diff -u -p -r1.60 fileio.c
--- fileio.c 2005/11/11 18:51:49 1.60
+++ fileio.c 2005/11/12 19:10:06
@@ -322,17 +322,18 @@ startupfile(char *suffix)
 {
  static char file[NFILEN];
  char *home;
+ int              ret;
 
  if ((home = getenv("HOME")) == NULL || *home == '\0')
  goto nohome;
 
  if (suffix == NULL) {
- if (snprintf(file, sizeof(file), "%s/.mg", home)
-    >= sizeof(file))
+ ret = snprintf(file, sizeof(file), "%s/.mg", home);
+ if (ret < 0 || ret >= sizeof(file))
  return (NULL);
  } else {
- if (snprintf(file, sizeof(file), "%s/.mg-%s", home, suffix)
-    >= sizeof(file))
+ ret = snprintf(file, sizeof(file), "%s/.mg-%s", home, suffix);
+ if (ret < 0 || ret >= sizeof(file))
  return (NULL);
  }
 
@@ -341,21 +342,21 @@ startupfile(char *suffix)
 nohome:
 #ifdef STARTUPFILE
  if (suffix == NULL) {
- if (snprintf(file, sizeof(file), "%s", STARTUPFILE)
-    >= sizeof(file))
+ ret = snprintf(file, sizeof(file), "%s", STARTUPFILE);
+ if (ret < 0 || ret >= sizeof(file))
  return (NULL);
  } else {
- if (snprintf(file, sizeof(file), "%s%s", STARTUPFILE, suffix)
-    >= sizeof(file))
+ ret = snprintf(file, sizeof(file), "%s%s", STARTUPFILE, suffix);
+ if (ret < 0 || ret >= sizeof(file))
  return (NULL);
  }
 
  if (access(file, R_OK) == 0)
  return (file);
-#endif
+#endif /* STARTUPFILE */
  return (NULL);
 }
-#endif
+#endif /* NO_STARTUP */
 
 #ifndef NO_DIRED
 
@@ -421,7 +422,7 @@ LIST *
 make_file_list(char *buf)
 {
  char *dir, *file, *cp;
- int len, preflen;
+ int len, preflen, ret;
  DIR *dirp;
  struct dirent *dent;
  LIST *last;
@@ -516,10 +517,11 @@ make_file_list(char *buf)
  char statname[NFILEN + 2];
 
  statbuf.st_mode = 0;
- if (snprintf(statname, sizeof(statname), "%s/%s",
-    dir, dent->d_name) > sizeof(statname) - 1) {
+ ret = snprintf(statname, sizeof(statname), "%s/%s", dir,
+    dent->d_name);
+ if (ret < 0 || ret >= sizeof(statname))
  continue;
- }
+
  if (stat(statname, &statbuf) < 0)
  continue;
  if (statbuf.st_mode & S_IFDIR)
@@ -530,12 +532,13 @@ make_file_list(char *buf)
  if (current == NULL)
  break;
 
- if (snprintf(current->fl_name, sizeof(current->fl_name),
-    "%s%s%s", prefixx, dent->d_name, isdir ? "/" : "")
-    >= sizeof(current->fl_name)) {
+ ret = snprintf(current->fl_name, sizeof(current->fl_name),
+    "%s%s%s", prefixx, dent->d_name, isdir ? "/" : "");
+ if (ret < 0 || ret >= sizeof(current->fl_name)) {
  free(current);
  continue;
  }
+
  current->fl_l.l_next = last;
  current->fl_l.l_name = current->fl_name;
  last = (LIST *) current;
Index: undo.c
===================================================================
RCS file: /cvs/src/usr.bin/mg/undo.c,v
retrieving revision 1.32
diff -u -p -r1.32 undo.c
--- undo.c 2005/10/14 19:46:46 1.32
+++ undo.c 2005/11/12 19:10:07
@@ -385,7 +385,13 @@ undo_dump(int f, int n)
  strlcat(buf, "\"", sizeof(buf));
  }
  snprintf(tmp, sizeof(tmp), " [%d]", rec->region.r_size);
- strlcat(buf, tmp, sizeof(buf));
+ /* Only check the last chance on a buffer-overflow since any
+ * earlier overflow will also result in an overflow over
+ * here. */
+ if (strlcat(buf, tmp, sizeof(buf)) >= sizeof(buf)) {
+ ewprintf("Error: overflow while undoing!");
+ return (FALSE);
+ }
  addlinef(bp, "%s", buf);
  }
  return (TRUE);



# Han

Reply | Threaded
Open this post in threaded view
|

Re: snprintf improvements for mg

Han Boetes
Han Boetes wrote:

> Index: buffer.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/mg/buffer.c,v
> retrieving revision 1.50
> diff -u -p -r1.50 buffer.c
> --- buffer.c 2005/10/14 19:46:46 1.50
> +++ buffer.c 2005/11/12 19:10:05
> @@ -426,13 +426,12 @@ anycb(int f)
>  {
>   BUFFER *bp;
>   int s = FALSE, save = FALSE;
> - char prompt[NFILEN + 11];
> + char prompt[NFILEN + 11] = "Save file ";
>  
>   for (bp = bheadp; bp != NULL; bp = bp-> b_bufp) {
>   if (bp-> b_fname != NULL && *(bp->b_fname) != '\0' &&
>      (bp-> b_flag & BFCHG) != 0) {
> - snprintf(prompt, sizeof(prompt), "Save file %s",
> -    bp-> b_fname);
> + strlcat(prompt, bp-> b_fname, sizeof(prompt));
>   if ((f == TRUE || (save = eyorn(prompt)) == TRUE) &&
>      buffsave(bp) == TRUE) {
>   bp-> b_flag &= ~BFCHG;

OK, that was not such a good idea after all. Here is an alternative.

Index: buffer.c
===================================================================
RCS file: /cvs/src/usr.bin/mg/buffer.c,v
retrieving revision 1.50
diff -u -p -r1.50 buffer.c
--- buffer.c 2005/10/14 19:46:46 1.50
+++ buffer.c 2005/11/12 21:37:06
@@ -417,22 +417,26 @@ addlinef(BUFFER *bp, char *fmt, ...)
 
 /*
  * Look through the list of buffers, giving the user a chance to save them.
- * Return TRUE if there are any changed buffers afterwards.  Buffers that
- * don't have an associated file don't count.  Return FALSE if there are
- * no changed buffers.
+ * Return TRUE if there are any changed buffers afterwards.  Buffers that don't
+ * have an associated file don't count.  Return FALSE if there are no changed
+ * buffers.  Return ABORT if an error occurs or if the user presses c-g.
  */
 int
 anycb(int f)
 {
  BUFFER *bp;
- int s = FALSE, save = FALSE;
+ int s = FALSE, save = FALSE, ret;
  char prompt[NFILEN + 11];
 
  for (bp = bheadp; bp != NULL; bp = bp->b_bufp) {
  if (bp->b_fname != NULL && *(bp->b_fname) != '\0' &&
     (bp->b_flag & BFCHG) != 0) {
- snprintf(prompt, sizeof(prompt), "Save file %s",
+ ret = snprintf(prompt, sizeof(prompt), "Save file %s",
     bp->b_fname);
+ if (ret < 0 || ret >= sizeof(prompt)) {
+ ewprintf("Error: filename too long!");
+ return (ABORT);
+ }
  if ((f == TRUE || (save = eyorn(prompt)) == TRUE) &&
     buffsave(bp) == TRUE) {
  bp->b_flag &= ~BFCHG;
Index: dired.c
===================================================================
RCS file: /cvs/src/usr.bin/mg/dired.c,v
retrieving revision 1.29
diff -u -p -r1.29 dired.c
--- dired.c 2005/11/12 20:13:47 1.29
+++ dired.c 2005/11/12 21:37:06
@@ -590,7 +590,7 @@ dired_(char *dirname)
  BUFFER *bp;
  FILE *dirpipe;
  char line[256];
- int len;
+ int len, ret;
 
  if ((dirname = adjustname(dirname)) == NULL) {
  ewprintf("Bad directory name");
@@ -609,8 +609,8 @@ dired_(char *dirname)
  if (bclear(bp) != TRUE)
  return (NULL);
  bp->b_flag |= BFREADONLY;
- if (snprintf(line, sizeof(line), "ls -al %s", dirname)
-    >= sizeof(line)) {
+ ret = snprintf(line, sizeof(line), "ls -al %s", dirname);
+ if (ret < 0 || ret  >= sizeof(line)) {
  ewprintf("Path too long");
  return (NULL);
  }



# Han