On Tue, Jan 5, 2016 at 10:38 AM, Jann Horn <jann@xxxxxxxxx> wrote:
> Let %h and %e print empty values as "!", "." as "!" and
> ".." as "!.".
>
> 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.
>
> Signed-off-by: Jann Horn <jann@xxxxxxxxx>
Acked-by: Kees Cook <keescook@xxxxxxxxxxxx>
-Kees
> ---
> fs/coredump.c | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/fs/coredump.c b/fs/coredump.c
> index dfc87c5..689577b 100644
> --- 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] == '/')
> cn->corename[cur] = '!';
> --
> 2.1.4
>
--
Kees Cook
Chrome OS & Brillo Security
--
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
