Re: SHELLOPTS=xtrace security hardening

[up201407890]

Regarding my last email, instead of a "break;" it should be a "continue;".
Just realized that xtrace might not always be last in $SHELLOPTS, so  
it would break out of the while loop and stop the parsing of the rest  
of the options.

$ cat file.c
int main()
{
        system("env");
}
$ gcc file.c
$ env -i SHELLOPTS=xtrace:history ./a.out
SHELLOPTS=braceexpand:hashall:interactive-comments:posix
PWD=/home/saken
SHLVL=1
_=/bin/env

history should have been here, so it should be:

$ diff -Naur bash-4.2.53.patch/bash-4.2.53 bash-4.2.53
diff -Naur bash-4.2.53.patch/bash-4.2.53/builtins/set.def  
bash-4.2.53/builtins/set.def
--- bash-4.2.53.patch/bash-4.2.53/builtins/set.def      2011-01-10  
14:22:25.000000000 +0000
+++ bash-4.2.53/builtins/set.def        2015-12-10 18:29:54.494932199 +0000
@@ -542,6 +542,10 @@
   vptr = 0;
   while (vname = extract_colon_unit (value, &vptr))
     {
+      /* xtrace can be used to exploit bogus system()/popen() calls  
on setuid binaries
+         via a specially crafted PS4 environment variable leading to  
privilege escalation. */
+      if(!strcmp(vname, "xtrace"))
+        continue;
       set_minus_o_option (FLAG_ON, vname);
       free (vname);
     }


After bash compilation:

$ env -i SHELLOPTS=xtrace:history ./a.out
SHELLOPTS=braceexpand:hashall:history:interactive-comments:posix
PWD=/home/saken
SHLVL=1
_=/bin/env

history is now available as an option.


Thanks,
Federico Bento


Quoting up201407890@alunos.dcc.fc.up.pt:

> Hello,
>
> This is a suggestion for a bash security hardening patch which  
> prevents xtrace from being initialized to the SHELLOPTS environment  
> variable when a new shell starts.
>
> xtrace can be used to exploit bogus system()/popen() calls on setuid  
> binaries via a specially crafted PS4 environment variable leading to  
> privilege escalation, like so:
>
>
> # gcc -xc - -otest <<< 'int main() { setuid(0); system("/bin/date"); }'
> # chmod 4755 ./test
> # ls -l ./test
> -rwsr-xr-x. 1 root root 8549 Dec 10 18:06 ./test
> # exit
> $ env -i SHELLOPTS=xtrace PS4='$(id)' ./test
> uid=0(root)
> Thu Dec 10 18:06:36 WET 2015
>
>
> This means, that setuid programs which call system(3)/popen(3) (a  
> horrendously bad practice in any case) are vulnerable, since it's  
> safe to assume that most of these do not ignore such environment  
> variables.
>
> CVE-2005-2959 reported by Tavis Ormandy would be an example against sudo
>
>
> from bash-4.2.53/shell.c:
>
>
>
>   1722    /* Initialize the shell options.  Don't import the shell options
>   1723       from the environment variables $SHELLOPTS or $BASHOPTS if we are
>   1724       running in privileged or restricted mode or if the  
> shell is running
>   1725       setuid. */
>   1726  #if defined (RESTRICTED_SHELL)
>   1727    initialize_shell_options  
> (privileged_mode||restricted||running_setuid);
>   1728    initialize_bashopts (privileged_mode||restricted||running_setuid);
>   1729  #else
>   1730    initialize_shell_options (privileged_mode||running_setuid);
>   1731    initialize_bashopts (privileged_mode||running_setuid);
>   1732  #endif
>
>
>
> initialize_shell_options() is defined in bash-4.2.53/builtins/set.def
>
>
>
>    555  void
>    556  initialize_shell_options (no_shellopts)
>    557       int no_shellopts;
>    558  {
>    559    char *temp;
>    560    SHELL_VAR *var;
>    561
>    562    if (no_shellopts == 0)
>    563      {
>    564        var = find_variable ("SHELLOPTS");
>    565        /* set up any shell options we may have inherited. */
>    566        if (var && imported_p (var))
>    567          {
>    568		  temp = (array_p (var) || assoc_p (var)) ? (char *)NULL :  
> savestring (value_cell (var));
>    569            if (temp)
>    570              {
>    571                parse_shellopts (temp);
>    572                free (temp);
>    573              }
>    574          }
>    575      }
>    576
>    577    /* Set up the $SHELLOPTS variable. */
>    578    set_shellopts ();
>    579  }
>
>
>
> parse_shellopts() is also defined in bash-4.2.53/builtins/set.def
>
>
>
>    535  void
>    536  parse_shellopts (value)
>    537       char *value;
>    538  {
>    539    char *vname;
>    540    int vptr;
>    541
>    542    vptr = 0;
>    543    while (vname = extract_colon_unit (value, &vptr))
>    544      {
>    545        set_minus_o_option (FLAG_ON, vname);
>    546        free (vname);
>    547      }
>    548  }
>
>
>
>
> Suggested patch:
>
>
> $ diff -Naur bash-4.2.53.patch/bash-4.2.53 bash-4.2.53
> diff -Naur bash-4.2.53.patch/bash-4.2.53/builtins/set.def  
> bash-4.2.53/builtins/set.def
> --- bash-4.2.53.patch/bash-4.2.53/builtins/set.def      2011-01-10  
> 14:22:25.000000000 +0000
> +++ bash-4.2.53/builtins/set.def        2015-12-10 18:29:54.494932199 +0000
> @@ -542,6 +542,10 @@
>    vptr = 0;
>    while (vname = extract_colon_unit (value, &vptr))
>      {
> +      /* xtrace can be used to exploit bogus system()/popen() calls  
> on setuid binaries
> +         via a specially crafted PS4 environment variable leading  
> to privilege escalation. */
> +      if(!strcmp(vname, "xtrace"))
> +        break;
>        set_minus_o_option (FLAG_ON, vname);
>        free (vname);
>      }
>
>
>
>
> After compilation:
>
>
> # rm /bin/bash
> # cp bash-4.2.53/bash /bin/bash
> # ls -l /bin/sh
> lrwxrwxrwx. 1 root root 9 Oct  4 18:18 /bin/sh -> /bin/bash
> # exit
> $ env -i SHELLOPTS=xtrace PS4='$(id)' ./test
> Thu Dec 10 18:30:41 WET 2015
>
>
> Thanks,
> Federico Bento.
>
> ----------------------------------------------------------------
> This message was sent using IMP, the Internet Messaging Program.



----------------------------------------------------------------
This message was sent using IMP, the Internet Messaging Program.