fish: handle some out-of-memory conditions

* fish/destpaths.c (xalloc_oversized): Define.
(complete_dest_paths_generator): Use size_t as type for a few
variables, rather than int.
Don't deref NULL or undef on failed heap alloc.
Don't leak on failed realloc.
Detect theoretical overflow when count_strings returns a very
large number of strings.
Handle asprintf failure.
(APPEND_STRS_AND_FREE): Rewrite as do {...}while(0), so that each use
can/must be followed by a semicolon.  Better for auto-formatters.

1 files changed, 62 insertions, 29 deletions

diff --git a/fish/destpaths.c b/fish/destpaths.c
index 6cddafa2..90970dea 100644
--- a/fish/destpaths.c
+++ b/fish/destpaths.c
@@ -1,5 +1,5 @@
 /* guestfish - the filesystem interactive shell
- * Copyright (C) 2009 Red Hat Inc. 
+ * Copyright (C) 2009 Red Hat Inc.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -22,6 +22,7 @@
 
 #include <stdio.h>
 #include <stdlib.h>
+#include <stddef.h>
 #include <string.h>
 
 #ifdef HAVE_LIBREADLINE
@@ -32,6 +33,22 @@
 
 #include "fish.h"
 
+// From gnulib's xalloc.h:
+/* Return 1 if an array of N objects, each of size S, cannot exist due
+   to size arithmetic overflow.  S must be positive and N must be
+   nonnegative.  This is a macro, not an inline function, so that it
+   works correctly even when SIZE_MAX < N.
+
+   By gnulib convention, SIZE_MAX represents overflow in size
+   calculations, so the conservative dividend to use here is
+   SIZE_MAX - 1, since SIZE_MAX might represent an overflowed value.
+   However, malloc (SIZE_MAX) fails on all known hosts where
+   sizeof (ptrdiff_t) <= sizeof (size_t), so do not bother to test for
+   exactly-SIZE_MAX allocations on such hosts; this avoids a test and
+   branch when S is known to be 1.  */
+# define xalloc_oversized(n, s) \
+    ((size_t) (sizeof (ptrdiff_t) <= sizeof (size_t) ? -1 : -2) / (s) < (n))
+
 /* Readline completion for paths on the guest filesystem, also for
  * devices and LVM names.
  */
@@ -53,9 +70,9 @@ complete_dest_paths_generator (const char *text, int state)
 {
 #ifdef HAVE_LIBREADLINE
 
-  static int len, index;
+  static size_t len, index;
   static char **words = NULL;
-  static int nr_words = 0;
+  static size_t nr_words = 0;
   char *word;
   guestfs_error_handler_cb old_error_cb;
   void *old_error_cb_data;
@@ -73,12 +90,12 @@ complete_dest_paths_generator (const char *text, int state)
 
   if (!state) {
     char **strs;
-    int i, n;
 
     len = strlen (text);
     index = 0;
 
     if (words) {
+      size_t i;
       /* NB. 'words' array is NOT NULL-terminated. */
       for (i = 0; i < nr_words; ++i)
 	free (words[i]);
@@ -90,26 +107,38 @@ complete_dest_paths_generator (const char *text, int state)
 
     SAVE_ERROR_CB
 
+/* Silently do nothing if an allocation fails */
 #define APPEND_STRS_AND_FREE						\
+  do {									\
     if (strs) {								\
-      n = count_strings (strs);						\
-      words = realloc (words, sizeof (char *) * (nr_words + n));	\
-      for (i = 0; i < n; ++i)						\
-	words[nr_words++] = strs[i];					\
-      free (strs);							\
-    }
+      size_t n = count_strings (strs);					\
+      if ( ! xalloc_oversized (nr_words + n, sizeof (char *))) {	\
+	char *w = realloc (words, sizeof (char *) * (nr_words + n));	\
+	if (w == NULL) {						\
+	  free (words);							\
+	  words = NULL;							\
+	  nr_words = 0;							\
+	} else {							\
+	  size_t i;							\
+	  for (i = 0; i < n; ++i)					\
+	    words[nr_words++] = strs[i];				\
+	}								\
+	free (strs);							\
+      }									\
+    }									\
+  } while (0)
 
     /* Is it a device? */
     if (len < 5 || strncmp (text, "/dev/", 5) == 0) {
       /* Get a list of everything that can possibly begin with /dev/ */
       strs = guestfs_list_devices (g);
-      APPEND_STRS_AND_FREE
+      APPEND_STRS_AND_FREE;
 
       strs = guestfs_list_partitions (g);
-      APPEND_STRS_AND_FREE
+      APPEND_STRS_AND_FREE;
 
       strs = guestfs_lvs (g);
-      APPEND_STRS_AND_FREE
+      APPEND_STRS_AND_FREE;
     }
 
     if (len < 1 || text[0] == '/') {
@@ -120,24 +149,28 @@ complete_dest_paths_generator (const char *text, int state)
 
       p = strrchr (text, '/');
       dir = p && p > text ? strndup (text, p - text) : strdup ("/");
-
-      strs = guestfs_ls (g, dir);
-
-      /* Prepend directory to names. */
-      if (strs) {
-	for (i = 0; strs[i]; ++i) {
-	  p = NULL;
-	  if (strcmp (dir, "/") == 0)
-	    asprintf (&p, "/%s", strs[i]);
-	  else
-	    asprintf (&p, "%s/%s", dir, strs[i]);
-	  free (strs[i]);
-	  strs[i] = p;
+      if (dir) {
+	strs = guestfs_ls (g, dir);
+
+	/* Prepend directory to names. */
+	if (strs) {
+	  size_t i;
+	  for (i = 0; strs[i]; ++i) {
+	    int err;
+	    if (strcmp (dir, "/") == 0)
+	      err = asprintf (&p, "/%s", strs[i]);
+	    else
+	      err = asprintf (&p, "%s/%s", dir, strs[i]);
+	    if (0 <= err) {
+	      free (strs[i]);
+	      strs[i] = p;
+	    }
+	  }
 	}
-      }
 
-      free (dir);
-      APPEND_STRS_AND_FREE
+	free (dir);
+	APPEND_STRS_AND_FREE;
+      }
     }
 
     /* else ...  In theory we could complete other things here such as VG