mirror of
https://gitlab.gnome.org/GNOME/glib.git
synced 2025-01-24 21:16:15 +01:00
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
This commit is contained in:
parent
96bccb89df
commit
149e5c5159
@ -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';
|
||||
|
@ -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);
|
||||
|
Loading…
Reference in New Issue
Block a user