Re: [PATCH] fs: create and use seq_show_option for escaping

[Paul Moore]

On Friday, August 07, 2015 04:41:50 PM Kees Cook wrote:
> Many file systems that implement the show_options hook fail to correctly
> escape their output which could lead to unescaped characters (e.g. new
> lines) leaking into /proc/mounts and /proc/[pid]/mountinfo files. This
> could lead to confusion, spoofed entries (resulting in things like
> systemd issuing false d-bus "mount" notifications), and who knows
> what else. This looks like it would only be the root user stepping on
> themselves, but it's possible weird things could happen in containers
> or in other situations with delegated mount privileges.
> 
> Here's an example using overlay with setuid fusermount trusting the
> contents of /proc/mounts (via the /etc/mtab symlink). Imagine the use of
> "sudo" is something more sneaky:
> 
> $ BASE="ovl"
> $ MNT="$BASE/mnt"
> $ LOW="$BASE/lower"
> $ UP="$BASE/upper"
> $ WORK="$BASE/work/ 0 0
> none /proc fuse.pwn user_id=1000"
> $ mkdir -p "$LOW" "$UP" "$WORK"
> $ sudo mount -t overlay -o "lowerdir=$LOW,upperdir=$UP,workdir=$WORK" none
> /mnt $ cat /proc/mounts
> none /root/ovl/mnt overlay
> rw,relatime,lowerdir=ovl/lower,upperdir=ovl/upper,workdir=ovl/work/ 0 0
> none /proc fuse.pwn user_id=1000 0 0
> $ fusermount -u /proc
> $ cat /proc/mounts
> cat: /proc/mounts: No such file or directory
> 
> This fixes the problem by adding new seq_show_option and seq_show_option_n
> helpers, and updating the vulnerable show_option handlers to use them as
> needed. Some, like SELinux, need to be open coded due to unusual existing
> escape mechanisms.
> 
> Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> ---
>  fs/ceph/super.c          |  2 +-
>  fs/cifs/cifsfs.c         |  6 +++---
>  fs/ext3/super.c          |  4 ++--
>  fs/ext4/super.c          |  4 ++--
>  fs/gfs2/super.c          |  6 +++---
>  fs/hfs/super.c           |  4 ++--
>  fs/hfsplus/options.c     |  4 ++--
>  fs/hostfs/hostfs_kern.c  |  2 +-
>  fs/ocfs2/super.c         |  4 ++--
>  fs/overlayfs/super.c     |  6 +++---
>  fs/reiserfs/super.c      |  8 +++++---
>  fs/xfs/xfs_super.c       |  4 ++--
>  include/linux/seq_file.h | 34 ++++++++++++++++++++++++++++++++++
>  kernel/cgroup.c          |  7 ++++---
>  net/ceph/ceph_common.c   |  7 +++++--
>  security/selinux/hooks.c |  2 +-
>  16 files changed, 72 insertions(+), 32 deletions(-)

The SELinux changes look fine to me.

Acked-by: Paul Moore <paul@xxxxxxxxxxxxxx>

-- 
paul moore
www.paul-moore.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/