On Tue, 5 Jan 2016 19:38:13 +0100 Jann Horn <jann@xxxxxxxxx> wrote:
> Let %h and %e print empty values as "!", "." as "!" and
> ".." as "!.".
How can an empty name cause the core file name to contain a "//"?
And how can a "//" "cause the coredump to happen one directory level
too high"? That's what ".." does. And a "//" in a pathname is the
same as a "/"?
And what's wrong with a plain old "."? That doesn't change the
directory level.
Confused.
> This prevents hostnames and comm values that are empty or
> consist of one or two dots from changing the directory
> level at which the corefile will be stored.
>
> It seems very unlikely that this caused security issues
> anywhere, so I'm not requesting a stable backport.
>
> ...
>
> --- a/fs/coredump.c
> +++ b/fs/coredump.c
> @@ -120,6 +120,26 @@ int cn_esc_printf(struct core_name *cn, const char *fmt, ...)
> ret = cn_vprintf(cn, fmt, arg);
> va_end(arg);
>
> + if (ret == 0) {
> + /*
> + * Ensure that this coredump name component can't cause the
> + * resulting corefile path to contain a ".." or "." component.
> + */
> + if ((cn->used - cur == 1 && cn->corename[cur] == '.') ||
> + (cn->used - cur == 2 && cn->corename[cur] == '.'
> + && cn->corename[cur+1] == '.'))
> + cn->corename[cur] = '!';
> +
> + /*
> + * Empty names are fishy and could be used to create a "//" in a
> + * corefile name, causing the coredump to happen one directory
> + * level too high. Enforce that all components of the core
> + * pattern are at least one character long.
> + */
> + if (cn->used == cur)
> + ret = cn_printf(cn, "!");
> + }
> +
> for (; cur < cn->used; ++cur) {
> if (cn->corename[cur] == '/')
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
