From jjaakkol@cs.Helsinki.FI Sun Sep 16 22:01:58 2001
Date: Sun, 16 Sep 2001 21:54:12 +0300 (EEST)
From: Jani Jaakkola <jjaakkol@cs.Helsinki.FI>
To: xxx@users.sourceforge.net, yyy@users.sourceforge.net,
     zzz@users.sourceforge.net
Cc: Jani Jaakkola <jjaakkol@cs.helsinki.fi>
Subject: Two remote dos bugs in latest cvs rpc.rquotad


I just installed the latest quota package quota-3.01pre9-0.7.1 by RedHat
and noticed that it can be remotely crashed even accidentally. I
downloaded the latest cvs version (and checked the patches by RH) and
found and fixed two bugs which could be exploited as DOS attacks against
rpc.rquotad.

Bug #1: rquota_svc calls daemon(1,1) which leaves stderr/stdout
filedescriptors open. This can cause a SIGPIPE and will kill rpc.rquotad
if someone asks for quota on a non mountpoint directory when rpc.rquotad
tries to write to stderr: "rpc.rquotad: Mountpoint (or device) /fs/big not
found.". Patch againt rquota_svc.c which adds calls daemon(0,0) instead
and adds a new option --foreground to rpc.rquotad for debugging purposes
included.

Bug #2: there is a memory leak if an error happens in process_dirs() in
quotasys.c (like attempting to ask quota for nonexistent directory). This
can be exploited to eat up all memory in rpc.rquotad server. Patch against
quotasys.c included.

I would also like to implement two new features. I would like rpc.rquotad
to return the quota also when the nfs service is not exported from the
fs mountpoint or when nfs client has mounted a directory under the
mountpoint. Also I would like to have an option to disable remote
setquota() in rpc.rquotad (since there is no real authentication and I
don't like the idea of users doing setquota() for themselves if they run
out of quota).

Anyways, the code seems to have lots of goto:s when do, while and for
would work fine and sometimes even produce more compact and more readable
code.

I will implement at least some of these things for my own file server, but
would like to have them in the official linuxquota distribution. If you
are interested, please reply.

- Jani

diff -u -r1.1.1.1 rquota_svc.c
--- rquota_svc.c        2001/08/22 21:17:56     1.1.1.1
+++ rquota_svc.c        2001/09/16 17:40:58
@@ -49,9 +49,6 @@
  */
 struct authunix_parms *unix_cred;

-char **argvargs;
-int argcargs;
-
 #ifdef HOSTS_ACCESS
 int good_client(struct sockaddr_in *addr)
 {
@@ -263,9 +260,13 @@
 int main(int argc, char **argv)
 {
        register SVCXPRT *transp;
+       int background=1;

-       argcargs = argc;
-       argvargs = argv;
+       if (argc==2 &&
+           (strcmp(argv[1],"-F")==0 || strcmp(argv[1],"--foreground")==0)) {
+               background=0;
+       }
+

        gettexton();
        progname = basename(argv[0]);
@@ -303,7 +304,9 @@
                exit(1);
        }

-       daemon(1, 1);
+       if (background) {
+               daemon(0, 0);
+       }
        svc_run();
        errstr(_("svc_run returned\n"));
        exit(1);


diff -u -r1.1.1.1 quotasys.c
--- quotasys.c  2001/09/10 10:34:16     1.1.1.1
+++ quotasys.c  2001/09/16 17:30:59
@@ -688,7 +688,9 @@
                                        errstr(_("Can't find mountpoint for device %s\n"), dirs[i]);
                                        continue;
                                }
-                               strcpy(mntpointbuf, mnt_entries[mentry].me_dir);
+                               strncpy(mntpointbuf,
+                                       mnt_entries[mentry].me_dir,
+                                       PATH_MAX-1);
                        }
                        else {
                                errstr(_("Specified path %s is not directory nor device.\n"), dirs[i]);
@@ -698,6 +700,7 @@
                        check_dirs_cnt++;
                }
                if (!check_dirs_cnt) {
+                       free(check_dirs);
                        errstr(_("No correct mountpoint specified.\n"));
                        return -1;
                }