[glib: 2/3] gfileutils: Correctly reset start value when canonicalising paths

[Sebastian Dröge <sdroege src gnome org>]

commit fc25f8d7ef0f4b60bbfaa148e31559006f84e614
Author: Philip Withnall <pwithnall endlessos org>
Date:   Thu Dec 2 11:33:09 2021 +0000

    gfileutils: Correctly reset start value when canonicalising paths
    
    If a path starts with more than two slashes, the `start` value was
    previously incorrect:
     1. As per the `g_path_skip_root()` call, `start` was set to point to
        after the final initial slash. For a path with three initial
        slashes, this is the character after the third slash.
     2. The canonicalisation loop to find the first dir separator sets
        `output` to point to the character after the first slash (and it
        overwrites the first slash to be `G_DIR_SEPARATOR`).
     3. At this point, with a string `///usr`, `output` points to the second
        `/`; and `start` points to the `u`. This is incorrect, as `start`
        should point to the starting character for output, as per the
        original call to `g_path_skip_root()`.
     4. For paths which subsequently include a `..`, this results in the
        `output > start` check in the `..` loop below not skipping all the
        characters of a preceding path component, which is then caught by
        the `G_IS_DIR_SEPARATOR (output[-1])` assertion.
    
    Fix this by resetting `start` to `output` after finding the final slash
    to keep in the output, but before starting the main parsing loop.
    
    Relatedly, split `start` into two variables: `after_root` and
    `output_start`, since the variable actually has two roles in the two
    parts of the function.
    
    Includes a test.
    
    This commit is heavily based on suggestions by Sebastian Wilhemi and
    Sebastian Dröge.
    
    Signed-off-by: Philip Withnall <pwithnall endlessos org>
    
    oss-fuzz#41563

 glib/gfileutils.c | 19 ++++++++++---------
 tests/testglib.c  |  3 +++
 2 files changed, 13 insertions(+), 9 deletions(-)
---
diff --git a/glib/gfileutils.c b/glib/gfileutils.c
index abcf29b25..189b756cc 100644
--- a/glib/gfileutils.c
+++ b/glib/gfileutils.c
@@ -2736,7 +2736,7 @@ gchar *
 g_canonicalize_filename (const gchar *filename,
                          const gchar *relative_to)
 {
-  gchar *canon, *input, *output, *start;
+  gchar *canon, *input, *output, *after_root, *output_start;
 
   g_return_val_if_fail (relative_to == NULL || g_path_is_absolute (relative_to), NULL);
 
@@ -2758,9 +2758,9 @@ g_canonicalize_filename (const gchar *filename,
       canon = g_strdup (filename);
     }
 
-  start = (char *)g_path_skip_root (canon);
+  after_root = (char *)g_path_skip_root (canon);
 
-  if (start == NULL)
+  if (after_root == NULL)
     {
       /* This shouldn't really happen, as g_get_current_dir() should
          return an absolute pathname, but bug 573843 shows this is
@@ -2770,7 +2770,7 @@ g_canonicalize_filename (const gchar *filename,
     }
 
   /* Find the first dir separator and use the canonical dir separator. */
-  for (output = start - 1;
+  for (output = after_root - 1;
        (output >= canon) && G_IS_DIR_SEPARATOR (*output);
        output--)
     *output = G_DIR_SEPARATOR;
@@ -2783,10 +2783,11 @@ g_canonicalize_filename (const gchar *filename,
    * (as does windows too). So, "//" != "/", but more than two slashes
    * is treated as "/".
    */
-  if (start - output == 1)
+  if (after_root - output == 1)
     output++;
 
-  input = start;
+  input = after_root;
+  output_start = output;
   while (*input)
     {
       /* input points to the next non-separator to be processed. */
@@ -2811,13 +2812,13 @@ g_canonicalize_filename (const gchar *filename,
       else if (input[0] == '.' && input[1] == '.' &&
                (input[2] == 0 || G_IS_DIR_SEPARATOR (input[2])))
         {
-          if (output > start)
+          if (output > output_start)
             {
               do
                 {
                   output--;
                 }
-              while (!G_IS_DIR_SEPARATOR (output[-1]) && output > start);
+              while (!G_IS_DIR_SEPARATOR (output[-1]) && output > output_start);
             }
           if (input[2] == 0)
             break;
@@ -2837,7 +2838,7 @@ g_canonicalize_filename (const gchar *filename,
     }
 
   /* Remove a potentially trailing dir separator */
-  if (output > start && G_IS_DIR_SEPARATOR (output[-1]))
+  if (output > output_start && G_IS_DIR_SEPARATOR (output[-1]))
     output--;
 
   *output = '\0';
diff --git a/tests/testglib.c b/tests/testglib.c
index a3546fae6..48cd74a2a 100644
--- a/tests/testglib.c
+++ b/tests/testglib.c
@@ -1063,6 +1063,7 @@ test_paths (void)
     { "/", "./", "/" },
     { "/", "/.", "/" },
     { "/", "/./", "/" },
+    { "/", "///usr/../usr", "/usr" },
 #else
     { "/etc", "../usr/share", "\\usr\\share" },
     { "/", "/foo/bar", "\\foo\\bar" },
@@ -1090,6 +1091,7 @@ test_paths (void)
     { "/", "./", "/" },
     { "/", "/.", "/" },
     { "/", "/./", "/" },
+    { "/", "///usr/../usr", "/usr" },
 
     { "\\etc", "..\\usr\\share", "\\usr\\share" },
     { "\\", "\\foo\\bar", "\\foo\\bar" },
@@ -1117,6 +1119,7 @@ test_paths (void)
     { "\\", ".\\", "\\" },
     { "\\", "\\.", "\\" },
     { "\\", "\\.\\", "\\" },
+    { "\\", "\\\\\\usr\\..\\usr", "\\usr" },
 #endif
   };
   const guint n_canonicalize_filename_checks = G_N_ELEMENTS (canonicalize_filename_checks);