[systemd-devel] [PATCH 7/9] nspawn: escape paths in overlay mount options

[Lennart Poettering]

On Thu, 28.05.15 13:02, Richard Maw (richard.maw at codethink.co.uk) wrote:

> Overlayfs uses , as an option separator and : as a list separator. These
> characters are both valid in file paths, so overlayfs allows file paths
> which contain these characters to backslash escape these values.
> ---
>  src/nspawn/nspawn.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 56 insertions(+), 7 deletions(-)
> 
> diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
> index c40d50f..f7580f9 100644
> --- a/src/nspawn/nspawn.c
> +++ b/src/nspawn/nspawn.c
> @@ -1237,6 +1237,42 @@ static int mount_tmpfs(const char *dest, CustomMount *m) {
>          return 0;
>  }
>  
> +static char *escaped_overlay_path(const char *path) {
> +        _cleanup_free_ char *colon_escaped = NULL;
> +        char *comma_escaped = NULL;
> +
> +        colon_escaped = strreplace(path, ":", "\\:");
> +        if (!colon_escaped)
> +                return NULL;
> +
> +        comma_escaped = strreplace(colon_escaped, ",", "\\,");
> +
> +        return comma_escaped;

This looks incomplete. What happens with "\\" itself?

Also, it's really inefficient, since strreplace() goes through the
string each time from the beginning.

I think it would make sense to add a generic 

  char *shell_escape(const char *s, const char *bad);

that works like xescape() but applies shell-style escaping instead of
C-style escaping.

> +}
> +
> +static char *joined_and_escaped_lower_dirs(char * const *lower) {
> +        _cleanup_free_ char *s = NULL;
> +        char *ret = NULL;
> +        char * const *path;
> +        bool first = true;
> +
> +        STRV_FOREACH_BACKWARDS(path, lower) {
> +                _cleanup_free_ char *escaped_path = NULL;
> +                escaped_path = escaped_overlay_path(*path);
> +                if (first) {
> +                        if (!strextend(&s, escaped_path, NULL))
> +                                return NULL;
> +                        first = false;
> +                } else
> +                        if (!strextend(&s, ":", escaped_path, NULL))
> +                                return NULL;
> +        }
> +
> +        ret = s;
> +        s = NULL;
> +        return ret;
> +}

I'd prefer if we could have a routine:

    char **shell_escape_strv(char **l, const char *bad);

that goes through the strv list "l", and replaces all items in-place
with shell_escape() and returns that.

If we have that, then we can nicely invoke strv_join() after
shell_escape_strv(), and all would be good and simple.

Lennart

-- 
Lennart Poettering, Red Hat