Re: [PATCH/RFC] do not source/exec scripts on noexec mount points

[Piotr Grzybowski]

Hello Mike,

 you want to forbid reading and interpreting scripts from the mount
point that is marked as noexec. If nothing gets executed from the
noexec area, as in your example, this is going to far.
 After this, do I have to move all my scripts away from the noexec
area if I want bash to read them and run the commands (neither of
which executes from the noexec mountpoint)?

sincerely,
pg




On Sat, Dec 12, 2015 at 10:01 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> From: Mike Frysinger <vapier@chromium.org>
>
> Today, if you have a script that lives on a noexec mount point, the
> kernel will reject attempts to run it directly:
>   $ printf '#!/bin/sh\necho hi\n' > /dev/shm/test.sh
>   $ chmod a+rx /dev/shm/test.sh
>   $ /dev/shm/test.sh
>   bash: /dev/shm/test.sh: Permission denied
>
> But bash itself has no problem running this file:
>   $ bash /dev/shm/test.sh
>   hi
> Or with letting other scripts run this file:
>   $ bash -c '. /dev/shm/test.sh'
>   hi
> Or with reading the script from stdin:
>   $ bash </dev/shm/test.sh
>   hi
>
> This detracts from the security of the overall system.  People writing
> scripts sometimes want to save/restore state (like variables) and will
> restore the content from a noexec point using the aforementioned source
> command without realizing that it executes code too.  Of course their
> code is wrong, but it would be nice if the system would catch & reject
> it explicitly to stave of inadvertent usage.
>
> This is not a perfect solution as it can still be worked around by
> inlining the code itself:
>   $ bash -c "$(cat /dev/shm/test.sh)"
>   hi
>
> But this makes things a bit harder for malicious attackers (depending
> how exactly they've managed to escalate), but it also helps developers
> from getting it wrong in the first place.
> ---
>  builtins/evalfile.c | 24 ++++++++++++++++++++++++
>  config.h.in         |  3 +++
>  configure           |  2 +-
>  configure.ac        |  2 +-
>  shell.c             | 31 +++++++++++++++++++++++++++++++
>  5 files changed, 60 insertions(+), 2 deletions(-)
>
> diff --git a/builtins/evalfile.c b/builtins/evalfile.c
> index eb51c27..ed031d6 100644
> --- a/builtins/evalfile.c
> +++ b/builtins/evalfile.c
> @@ -32,6 +32,10 @@
>  #include <signal.h>
>  #include <errno.h>
>
> +#if defined (HAVE_SYS_STATVFS_H)
> +#  include <sys/statvfs.h>
> +#endif
> +
>  #include "../bashansi.h"
>  #include "../bashintl.h"
>
> @@ -160,6 +164,26 @@ file_error_and_exit:
>        return ((flags & FEVAL_BUILTIN) ? EXECUTION_FAILURE : -1);
>      }
>
> +#if defined (HAVE_SYS_STATVFS_H) && defined (ST_NOEXEC)
> +  /* If the script is loaded from a noexec mount point, throw an error.  */
> +  {
> +    struct statvfs stvfs;
> +
> +    if (fstatvfs (fd, &stvfs) == -1)
> +      {
> +       close (fd);
> +       goto file_error_and_exit;
> +      }
> +
> +    if (stvfs.f_flag & ST_NOEXEC)
> +      {
> +       close (fd);
> +       errno = EACCES;
> +       goto file_error_and_exit;
> +      }
> +  }
> +#endif
> +
>    if (S_ISREG (finfo.st_mode) && file_size <= SSIZE_MAX)
>      {
>        string = (char *)xmalloc (1 + file_size);
> diff --git a/config.h.in b/config.h.in
> index 894892f..b16f1d6 100644
> --- a/config.h.in
> +++ b/config.h.in
> @@ -1039,6 +1039,9 @@
>  /* Define if you have the <sys/stat.h> header file. */
>  #undef HAVE_SYS_STAT_H
>
> +/* Define if you have <sys/statvfs.h>.  */
> +#undef HAVE_SYS_STATVFS_H
> +
>  /* Define if you have the <sys/stream.h> header file.  */
>  #undef HAVE_SYS_STREAM_H
>
> diff --git a/configure b/configure
> index 52f6f5c..061b15e 100755
> --- a/configure
> +++ b/configure
> @@ -9301,7 +9301,7 @@ fi
>  done
>
>  for ac_header in sys/pte.h sys/stream.h sys/select.h sys/file.h sys/ioctl.h \
> -                sys/param.h sys/socket.h sys/stat.h \
> +                sys/param.h sys/socket.h sys/stat.h sys/statvfs.h \
>                  sys/time.h sys/times.h sys/types.h sys/wait.h
>  do :
>    as_ac_Header=`$as_echo "ac_cv_header_$ac_header" | $as_tr_sh`
> diff --git a/configure.ac b/configure.ac
> index f0d4aee..81b2a7c 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -717,7 +717,7 @@ AC_CHECK_HEADERS(unistd.h stdlib.h stdarg.h varargs.h 
> limits.h string.h \
>                  stdbool.h stddef.h stdint.h netdb.h pwd.h grp.h strings.h \
>                  regex.h syslog.h ulimit.h)
>  AC_CHECK_HEADERS(sys/pte.h sys/stream.h sys/select.h sys/file.h sys/ioctl.h \
> -                sys/param.h sys/socket.h sys/stat.h \
> +                sys/param.h sys/socket.h sys/stat.h sys/statvfs.h \
>                  sys/time.h sys/times.h sys/types.h sys/wait.h)
>  AC_CHECK_HEADERS(netinet/in.h arpa/inet.h)
>
> diff --git a/shell.c b/shell.c
> index 0e47cf4..4739a31 100644
> --- a/shell.c
> +++ b/shell.c
> @@ -46,6 +46,10 @@
>  #  include <unistd.h>
>  #endif
>
> +#if defined (HAVE_SYS_STATVFS_H)
> +#  include <sys/statvfs.h>
> +#endif
> +
>  #include "bashintl.h"
>
>  #define NEED_SH_SETLINEBUF_DECL                /* used in externs.h */
> @@ -334,6 +338,8 @@ static void shell_reinitialize __P((void));
>
>  static void show_shell_usage __P((FILE *, int));
>
> +static void check_noexec __P((int, const char *));
> +
>  #ifdef __CYGWIN__
>  static void
>  _cygwin32_check_tmp ()
> @@ -717,6 +723,7 @@ main (argc, argv, env)
>      {
>        /* In this mode, bash is reading a script from stdin, which is a
>          pipe or redirected file. */
> +      check_noexec (0, "stdin");
>  #if defined (BUFFERED_INPUT)
>        default_buffered_input = fileno (stdin); /* == 0 */
>  #else
> @@ -1442,6 +1449,28 @@ start_debugger ()
>  #endif
>  }
>
> +static void
> +check_noexec (int fd, const char *filename)
> +{
> +#if defined (HAVE_SYS_STATVFS_H) && defined (ST_NOEXEC)
> +  /* Make sure the file isn't on a noexec mount point.  */
> +  struct statvfs stvfs;
> +
> +  if (fstatvfs (fd, &stvfs) == -1)
> +    {
> +      file_error (filename);
> +      exit (EX_NOTFOUND);
> +    }
> +
> +  if (stvfs.f_flag & ST_NOEXEC)
> +    {
> +      errno = EACCES;
> +      file_error (filename);
> +      exit (EX_NOEXEC);
> +    }
> +#endif
> +}
> +
>  static int
>  open_shell_script (script_name)
>       char *script_name;
> @@ -1579,6 +1608,8 @@ open_shell_script (script_name)
>      SET_CLOSE_ON_EXEC (fileno (default_input));
>  #endif /* !BUFFERED_INPUT */
>
> +  check_noexec (fd, filename);
> +
>    /* Just about the only way for this code to be executed is if something
>       like `bash -i /dev/stdin' is executed. */
>    if (interactive_shell && fd_is_tty)
> --
> 2.6.2
>
>