mountlist: make parsing /proc/self/mountinfo more robust

Cater for the following issues with mountinfo parsing (the first
one was reported by Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
in <https://bugs.gnu.org/35137>).

1. The fields source, target, mntroot and fstype may contain characters
like '\r'; sscanf(3) fails to read such values with the %s format
specifier because it would stop at such characters.
Example: "mount -t tmpfs tmpfs /foo^Mbar".
The only true separator in that file is the ' ' character.

2. The source field may be an empty string, which happens e.g. with
"mount -t tmpfs '' /target".

3. The fstype field may contain mangled characters as well which need
unescaping.

* lib/mountlist.c (terminate_at_blank): Add utility function.
(read_file_system_list): In the block trying to read the mountinfo file,
avoid using sscanf(3) with %s format; instead, parse the above fields
separated by spaces one by one.
This also handles the case when the source field is an empty string.
Unescape the fstype field.

2 files changed, 69 insertions, 33 deletions

diff --git a/ChangeLog b/ChangeLog
index 87d95ed993..427c68fd98 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,25 @@
+2019-04-09  Bernhard Voelker  <mail@bernhard-voelker.de>
+
+	mountlist: make parsing /proc/self/mountinfo more robust
+	Cater for the following issues with mountinfo parsing (the first
+	one was reported by Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
+	in <https://bugs.gnu.org/35137>).
+	1. The fields source, target, mntroot and fstype may contain characters
+	like '\r'; sscanf(3) fails to read such values with the %s format
+	specifier because it would stop at such characters.
+	Example: "mount -t tmpfs tmpfs /foo^Mbar".
+	The only true separator in that file is the ' ' character.
+	2. The source field may be an empty string, which happens e.g. with
+	"mount -t tmpfs '' /target".
+	3. The fstype field may contain mangled characters as well which need
+	unescaping.
+	* lib/mountlist.c (terminate_at_blank): Add utility function.
+	(read_file_system_list): In the block trying to read the mountinfo file,
+	avoid using sscanf(3) with %s format; instead, parse the above fields
+	separated by spaces one by one.
+	This also handles the case when the source field is an empty string.
+	Unescape the fstype field.
+
 2019-04-09  Bruno Haible  <bruno@clisp.org>
 
 	openmp: Add workaround for 32-bit programs on AIX.
diff --git a/lib/mountlist.c b/lib/mountlist.c
index 9b54a2cf70..f3e80af1ea 100644
--- a/lib/mountlist.c
+++ b/lib/mountlist.c
@@ -418,6 +418,18 @@ unescape_tab (char *str)
         str[j++] = str[i];
     }
 }
+
+/* Find the next space in STR, terminate the string there in place,
+   and return that position.  Otherwise return NULL.  */
+
+static char *
+terminate_at_blank (char *str)
+{
+  char *s = strchr (str, ' ');
+  if (s)
+    *s = '\0';
+  return s;
+}
 #endif
 
 /* Return a list of the currently mounted file systems, or NULL on error.
@@ -453,56 +465,58 @@ read_file_system_list (bool need_fs_type)
         while (getline (&line, &buf_size, fp) != -1)
           {
             unsigned int devmaj, devmin;
-            int target_s, target_e, type_s, type_e;
-            int source_s, source_e, mntroot_s, mntroot_e;
-            char test;
-            char *dash;
-            int rc;
+            int rc, mntroot_s;
 
             rc = sscanf(line, "%*u "        /* id - discarded  */
-                              "%*u "        /* parent - discarded */
+                              "%*u "        /* parent - discarded  */
                               "%u:%u "      /* dev major:minor  */
-                              "%n%*s%n "    /* mountroot */
-                              "%n%*s%n"     /* target, start and end  */
-                              "%c",         /* more data...  */
+                              "%n",         /* mountroot (start)  */
                               &devmaj, &devmin,
-                              &mntroot_s, &mntroot_e,
-                              &target_s, &target_e,
-                              &test);
+                              &mntroot_s);
 
-            if (rc != 3 && rc != 7)  /* 7 if %n included in count.  */
+            if (rc != 2 && rc != 3)  /* 3 if %n included in count.  */
+              continue;
+
+            /* find end of MNTROOT.  */
+            char *mntroot = line + mntroot_s;
+            char *blank = terminate_at_blank (mntroot);
+            if (! blank)
+              continue;
+
+            /* find end of TARGET.  */
+            char *target = blank + 1;
+            blank = terminate_at_blank (target);
+            if (! blank)
               continue;
 
             /* skip optional fields, terminated by " - "  */
-            dash = strstr (line + target_e, " - ");
+            char *dash = strstr (blank + 1, " - ");
             if (! dash)
               continue;
 
-            rc = sscanf(dash, " - "
-                              "%n%*s%n "    /* FS type, start and end  */
-                              "%n%*s%n "    /* source, start and end  */
-                              "%c",         /* more data...  */
-                              &type_s, &type_e,
-                              &source_s, &source_e,
-                              &test);
-            if (rc != 1 && rc != 5)  /* 5 if %n included in count.  */
+            /* advance past the " - " separator.  */
+            char *fstype = dash + 3;
+            blank = terminate_at_blank (fstype);
+            if (! blank)
+              continue;
+
+            /* find end of SOURCE.  */
+            char *source = blank + 1;
+            if (! terminate_at_blank (source))
               continue;
 
             /* manipulate the sub-strings in place.  */
-            line[mntroot_e] = '\0';
-            line[target_e] = '\0';
-            dash[type_e] = '\0';
-            dash[source_e] = '\0';
-            unescape_tab (dash + source_s);
-            unescape_tab (line + target_s);
-            unescape_tab (line + mntroot_s);
+            unescape_tab (source);
+            unescape_tab (target);
+            unescape_tab (mntroot);
+            unescape_tab (fstype);
 
             me = xmalloc (sizeof *me);
 
-            me->me_devname = xstrdup (dash + source_s);
-            me->me_mountdir = xstrdup (line + target_s);
-            me->me_mntroot = xstrdup (line + mntroot_s);
-            me->me_type = xstrdup (dash + type_s);
+            me->me_devname = xstrdup (source);
+            me->me_mountdir = xstrdup (target);
+            me->me_mntroot = xstrdup (mntroot);
+            me->me_type = xstrdup (fstype);
             me->me_type_malloced = 1;
             me->me_dev = makedev (devmaj, devmin);
             /* we pass "false" for the "Bind" option as that's only