You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Rob Hartill <ha...@hyperreal.com> on 1995/09/13 23:30:14 UTC
Apache secure CGI bin patch (fwd)
monster patch for consideration...
Forwarded message:
> From leitner@prz.tu-berlin.de Wed Sep 13 13:30:23 1995
> From: leitner@prz.tu-berlin.de
> Message-Id: <19...@tubkom.prz.tu-berlin.de>
> Subject: Apache secure CGI bin patch
> To: apache-bugs@mail.apache.org
> Date: Wed, 13 Sep 1995 22:29:21 +0100 (MESZ)
> Mime-Version: 1.0
> Content-Type: text/plain; charset=ISO-8859-1
> Content-Transfer-Encoding: 8bit
> Content-Length: 12694
>
> Hi folks,
>
> I wanted to patch some server for quite a while, and now I finally found
> a worthy server, Apache ;)
>
> This is my patch. Go into the apache_0.8.11 directory and run "patch -s
> -p1 < apache.diff" where apache.diff is included here. Please read the
> file README.SETUID in the src/ directory to see what this patch does.
>
> I hope I'll get on the Apache credits list now ;))
>
> Felix
>
>
> diff -u --recursive --new-file apache_0.8.11/src/Configuration apache_0.8.11.setuid/src/Configuration
> --- apache_0.8.11/src/Configuration Tue Aug 8 18:56:04 1995
> +++ apache_0.8.11.setuid/src/Configuration Tue Sep 5 23:16:42 1995
> @@ -34,11 +34,11 @@
> # defaults in. Note that this config file does not include DBM auth by
> # default --- configure it in below if you need it].
>
> -CFLAGS= -O2
> +CFLAGS= -O2 -DSETUID
>
> # Place here any flags you may need upon linking, such as a flag to
> # prevent dynamic linking (if desired)
> -LFLAGS=
> +LFLAGS= -s
>
> # Place here any extra libraries you may need to link to.
> # -lndbm is commonly required for DBM auth, if that is configured in.
> diff -u --recursive --new-file apache_0.8.11/src/README.SETUID apache_0.8.11.setuid/src/README.SETUID
> --- apache_0.8.11/src/README.SETUID
> +++ apache_0.8.11.setuid/src/README.SETUID Wed Sep 13 21:55:52 1995
> @@ -0,0 +1,80 @@
> +This README describes my SETUID patch to the wonderful Apache HTTP
> +server. It tries to fix Apache so that sysadmins have fewer reasons to
> +disable CGI binaries in user's directories. Normally, a sysadmin with
> +security in mind cannot allow CGI binaries in user's directories, since
> +user can kill each other's CGI bins, and they run under another UID, so
> +users can install backdoors to allow remote login under the UID of their
> +CGI bins. Besides, if users want to do some logging with their CGI
> +binaries, they have to chmod u+X them or make the log file world
> +writable. Both is unsatisfactory.
> +
> +Another thing that causes much grief to me is the .htaccess file. Now
> +users can password-protect their files, but every local user can just
> +copy the files from their account, since they have to be world readable
> +so that the HTTP daemon can read them.
> +
> +My patch changes the Apache daemon in the following ways :
> +
> + * When run standalone, the daemon does not setuid(wanted), but it
> + does seteuid(wanted). This makes it possible that the daemon can
> + later seteuid to some other uid. The daemon still does all the dirty
> + work (like writing log files, etc) under the EUID you set up.
> + * When run from inetd, it must be run as root and I changed the code
> + that it changes UID just like the standalone server.
> + * Files in home directories are now read under the EUID of the owner
> + of the home directory. That means they no longer need to be world
> + readable. Nor does the public_html directory need to be world
> + executable !
> + * CGI-bins in home directories are now run under the UID of the owner
> + of the home. That means, if a user does a "kill -9 -1", it just his
> + processes that die. And if you have a fascist Unix with process,
> + CPU and memory quota, you can keep users from killing your HTTP
> + server this way.
> + Note that before CGI-bins are executed, the real UID is changed to
> + the effective UID, so no CGI-bin can setuid(getuid) and be root !
> + * Server Parsed HTML files can include both files and program outputs.
> + Now this is a little tricky, because a malevolent user could use a
> + server parsed HTML file to read any file in another user's
> + public_html hierarchy. For this reason, no UID swapping takes place
> + when including files. The same is true for foreign CGI-bins ! They
> + have to be executable to the user to be includable.
> + * CGI-bins can only be executed if they belong to the owner of
> + the home directory. An exception are server parsed HTML included
> + CGI-bins, otherwise one could not include foreign CGI bins.
> + This is important, because otherwise people would compromise their
> + accounts if they accidentally left one directory in their
> + public_html hierarchy group or world writable by accident. Then any
> + local malevolent user could create a CGI-bin that would modify that
> + user's .rhosts file.
> + * CGI-bins can not be executed if they are group or world
> + writable. If someone had group or world writable CGI-bins, anyone
> + could modify that CGI-bin and then compromise that user's account by
> + modifying .rhosts.
> +
> +Please note that my code is ugly. It does not do error checking.
> +If seteuid fails, you'll not get the error you'll expected, but you'll
> +get an error that some files could not be read.
> +
> +To make the decision to integrate my patches easier, the changes are
> +only effective if you #define SETUID.
> +
> +If you do not understand what the heck I am talking about, I suggest
> +that you do not run a web server until you have consulted a security
> +specialist.
> +
> +Since I am but a humble admin and C/C++/Perl hacker, I can't guarantee
> +that this code works, does anything at all, and does not open new holes.
> +I advise you to think before enabling SETUID blindly.
> +
> +If you find a bug, add error handling, find a security hole, or just
> +want to tell me something, feel free to e-mail me as
> +leitner@prz.tu-berlin.de. Alternatively, you could try to e-mail me as
> +leitner@inf.fu-berlin.de. If that does not work, too, you can still
> +send me snail mail (I just *love* postcards) :
> +
> + Felix von Leitner
> + Gervinusstra�e 22 [Foreigners without ISO umlauts do s/�/ss/]
> + 10629 Berlin
> + Germany
> +
> +Felix
> diff -u --recursive --new-file apache_0.8.11/src/http_core.c apache_0.8.11.setuid/src/http_core.c
> --- apache_0.8.11/src/http_core.c Wed Aug 23 03:16:07 1995
> +++ apache_0.8.11.setuid/src/http_core.c Wed Sep 13 20:39:23 1995
> @@ -670,7 +670,15 @@
> || (errstatus = set_last_modified (r, r->finfo.st_mtime)))
> return errstatus;
>
> +#ifdef SETUID
> + seteuid(0); /* First become root again so we can */
> + seteuid(destuid); /* seteuid again */
> +#endif
> f = fopen (r->filename, "r");
> +#ifdef SETUID
> + seteuid(0); /* First become root again so we can */
> + seteuid(user_id); /* seteuid again */
> +#endif
>
> if (f == NULL) {
> log_reason("file permissions deny server access", r->filename, r);
> diff -u --recursive --new-file apache_0.8.11/src/http_main.c apache_0.8.11.setuid/src/http_main.c
> --- apache_0.8.11/src/http_main.c Wed Aug 23 03:16:10 1995
> +++ apache_0.8.11.setuid/src/http_main.c Wed Sep 13 20:38:53 1995
> @@ -102,6 +102,10 @@
>
> int standalone;
> uid_t user_id;
> +#ifdef SETUID
> +uid_t destuid;
> +char ininclude=0;
> +#endif
> char *user_name;
> gid_t group_id;
> int max_requests_per_child;
> @@ -711,7 +715,11 @@
> reopen_scoreboard (pconf);
>
> /* Only try to switch if we're running as root */
> +#ifdef SETUID
> + if(!getuid() && seteuid(user_id) == -1) {
> +#else
> if(!getuid() && setuid(user_id) == -1) {
> +#endif
> log_error ("unable to change uid", server_conf);
> exit (1);
> }
> @@ -987,8 +995,10 @@
> if (!server_conf->server_hostname)
> server_conf->server_hostname = get_local_host(pconf);
>
> +#ifndef SETUID
> user_id = getuid();
> group_id = getgid();
> +#endif
>
> server_conf->port = get_portnum(fileno(stdout));
> conn = new_connection (ptrans, server_conf, stdin, stdout);
> diff -u --recursive --new-file apache_0.8.11/src/httpd.h apache_0.8.11.setuid/src/httpd.h
> --- apache_0.8.11/src/httpd.h Thu Aug 24 02:09:47 1995
> +++ apache_0.8.11.setuid/src/httpd.h Wed Sep 13 20:45:06 1995
> @@ -101,6 +101,11 @@
> #define DEFAULT_USER "#-1"
> #define DEFAULT_GROUP "#-1"
>
> +#ifdef SETUID
> +extern uid_t destuid;
> +extern char ininclude;
> +#endif
> +
> /* The name of the log files */
> #define DEFAULT_XFERLOG "logs/access_log"
> #define DEFAULT_ERRORLOG "logs/error_log"
> diff -u --recursive --new-file apache_0.8.11/src/mod_asis.c apache_0.8.11.setuid/src/mod_asis.c
> --- apache_0.8.11/src/mod_asis.c Wed Aug 23 03:16:18 1995
> +++ apache_0.8.11.setuid/src/mod_asis.c Wed Sep 13 19:56:20 1995
> @@ -64,6 +64,8 @@
> #include "util_script.h"
> #include "http_main.h"
>
> +extern uid_t user_id;
> +
> int asis_handler (request_rec *r)
> {
> FILE *f;
> @@ -73,8 +75,15 @@
> log_reason("File does not exist", r->filename, r);
> return NOT_FOUND;
> }
> -
> +#ifdef SETUID
> + seteuid(0); /* First become root again so we can */
> + seteuid(destuid); /* seteuid again */
> +#endif
> f = fopen (r->filename, "r");
> +#ifdef SETUID
> + seteuid(0); /* First become root again so we can */
> + seteuid(user_id); /* seteuid again */
> +#endif
>
> if (f == NULL) {
> log_reason("file permissions deny server access", r->filename, r);
> diff -u --recursive --new-file apache_0.8.11/src/mod_cgi.c apache_0.8.11.setuid/src/mod_cgi.c
> --- apache_0.8.11/src/mod_cgi.c Wed Aug 23 03:16:20 1995
> +++ apache_0.8.11.setuid/src/mod_cgi.c Wed Sep 13 21:39:50 1995
> @@ -195,6 +195,9 @@
>
> cleanup_for_exec();
>
> +#ifdef SETUID
> + setuid(destuid);
> +#endif
> if((!r->args) || (!r->args[0]) || (ind(r->args,'=') >= 0))
> execle(r->filename, argv0, NULL, env);
> else
> diff -u --recursive --new-file apache_0.8.11/src/mod_include.c apache_0.8.11.setuid/src/mod_include.c
> --- apache_0.8.11/src/mod_include.c Wed Aug 23 03:16:23 1995
> +++ apache_0.8.11.setuid/src/mod_include.c Wed Sep 13 20:37:27 1995
> @@ -687,6 +687,10 @@
>
> /* This is a stub which parses a file descriptor. */
>
> +#ifdef SETUID
> +extern char ininclude;
> +#endif
> +
> void send_parsed_content(FILE *f, request_rec *r)
> {
> char directive[MAX_STRING_LEN], error[MAX_STRING_LEN];
> @@ -712,12 +716,25 @@
> rprintf(r,"%s",error);
> ret = find_string(f,ENDING_SEQUENCE,NULL);
> } else
> +#ifdef SETUID
> + { ininclude++;
> +#endif
> ret=handle_exec(f, r, error);
> +#ifdef SETUID
> + ininclude--; }
> +#endif
> }
> else if(!strcmp(directive,"config"))
> ret=handle_config(f, r, error, timefmt, &sizefmt);
> else if(!strcmp(directive,"include"))
> +#ifdef SETUID
> + { ininclude++;
> +#endif
> ret=handle_include(f, r, error, noexec);
> +#ifdef SETUID
> + ininclude--;
> + }
> +#endif
> else if(!strcmp(directive,"echo"))
> ret=handle_echo(f, r, error);
> else if(!strcmp(directive,"fsize"))
> diff -u --recursive --new-file apache_0.8.11/src/mod_userdir.c apache_0.8.11.setuid/src/mod_userdir.c
> --- apache_0.8.11/src/mod_userdir.c Wed Aug 23 03:16:28 1995
> +++ apache_0.8.11.setuid/src/mod_userdir.c Wed Sep 13 20:31:22 1995
> @@ -69,6 +69,7 @@
>
> #include "httpd.h"
> #include "http_config.h"
> +#include "http_conf_globals.h"
>
> module userdir_module;
>
> @@ -97,12 +98,19 @@
> { NULL }
> };
>
> +#ifdef SETUID
> +extern char ininclude;
> +#endif
> +
> int translate_userdir (request_rec *r)
> {
> void *server_conf = r->server->module_config;
> char *userdir = (char *)get_module_config(server_conf, &userdir_module);
> char *name = r->uri;
>
> +#ifdef SETUID
> + destuid=user_id;
> +#endif
> if (userdir && (name[0] == '/') && (name[1] == '~'))
> {
> struct passwd *pw;
> @@ -112,6 +120,10 @@
> w = getword(r->pool, &dname, '/');
> if(!(pw=getpwnam(w)))
> return NOT_FOUND;
> +#ifdef SETUID
> + if (ininclude==0)
> + destuid=pw->pw_uid;
> +#endif
>
> /* The 'dname' funny business involves backing it up to capture
> * the '/' delimiting the "/~user" part from the rest of the URL,
> diff -u --recursive --new-file apache_0.8.11/src/util.c apache_0.8.11.setuid/src/util.c
> --- apache_0.8.11/src/util.c Wed Aug 23 03:16:31 1995
> +++ apache_0.8.11.setuid/src/util.c Wed Sep 13 21:36:59 1995
> @@ -532,6 +532,25 @@
> }
>
> int can_exec(struct stat *finfo) {
> +#ifdef SETUID /* hack by Felix von Leitner <le...@prz.tu-berlin.de> */
> + if (finfo->st_mode & (S_IWGRP+S_IWOTH)) {
> + fprintf(stderr,"group or world writable\n");
> + return 0; /* Never allow to execute group or world writable CGIs */
> + }
> + if (ininclude==0)
> + if (finfo->st_uid != destuid) {
> + fprintf(stderr,"CGI uid=%d, home uid=%d\n",finfo->st_uid,destuid);
> + return 0; /* Do not allow to execute CGIs not belonging
> + to the owner of the home */
> + }
> + if(destuid == finfo->st_uid)
> + if(finfo->st_mode & S_IXUSR)
> + return 1;
> + if(group_id == finfo->st_gid)
> + if(finfo->st_mode & S_IXGRP)
> + return 1;
> + return (finfo->st_mode & S_IXOTH);
> +#else
> if(user_id == finfo->st_uid)
> if(finfo->st_mode & S_IXUSR)
> return 1;
> @@ -539,6 +558,7 @@
> if(finfo->st_mode & S_IXGRP)
> return 1;
> return (finfo->st_mode & S_IXOTH);
> +#endif
> }
>
> #ifdef NEED_STRDUP
>