Bug 19422
| Summary: | users can overflow argv to make ps segfault | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | Base System | Reporter: | Marc Olzheim <marcolz> | ||||||
| Component: | bin | Assignee: | Mike Heffner <mikeh> | ||||||
| Status: | Closed FIXED | ||||||||
| Severity: | Affects Only Me | ||||||||
| Priority: | Normal | ||||||||
| Version: | 3.4-RELEASE | ||||||||
| Hardware: | Any | ||||||||
| OS: | Any | ||||||||
| Attachments: |
|
||||||||
|
Description
Marc Olzheim
2000-06-21 21:40:01 UTC
I must've been sleeping... A better patch would be: strvisx(dst, src, arg_max - (dst - buf) - 1, VIS_NL | VIS_CSTYLE); Btw. This is also the case with FreeBSD 4.0. Marc State Changed From-To: open->feedback I can't reproduce this on a recent -current or -stable. Is this still a problem? Responsible Changed From-To: freebsd-bugs->dd My reminder to deal with this. When I looked at this it appears that rev. 1.10 of fmt.c was meant to fix this. However it looks like the change just increased the buffer size, but didn't put any restrictions on the strvis() -- which just means a bigger string is needed to overflow `buf'. But I haven't looked at the code in detail, so there might be caps on the size of argv[0] some where else that would block any overflow. On 28-Jun-2001 dd@FreeBSD.org wrote: | Synopsis: users can overflow argv to make ps segfault | | State-Changed-From-To: open->feedback | State-Changed-By: dd | State-Changed-When: Wed Jun 27 18:39:20 PDT 2001 | State-Changed-Why: | I can't reproduce this on a recent -current or -stable. Is this still | a problem? Mike -- Mike Heffner <mheffner@[acm.]vt.edu> Fredericksburg, VA <mikeh@FreeBSD.org> It's still possible:
#include <stdio.h>
#include <sys/exec.h>
#include <sys/param.h>
#include <sys/sysctl.h>
#include <sys/types.h>
#include <unistd.h>
int
main(int argc, char *argv[])
{
int oid[4];
char buf[65537];
if (argc >= 2)
argv[0] = argv[1];
else
{
memset(buf, 'A', 65537);
argv[0] = buf;
}
// setproctitle("%s", argv[0]);
oid[0] = CTL_KERN;
oid[1] = KERN_PROC;
oid[2] = KERN_PROC_ARGS;
oid[3] = getpid();
sysctl(oid, 4, 0, 0, buf, 65537);
sleep(600);
return(0);
}
Run it and do a ps uxwww
ps either segfaults or shows you some strvis-d data.
Marc
And if you put a #if __FreeBSD__ == 4 and a #endif around the setproctitle stuff, it will run on both FreeBSD 3 and 4 Marc FreeBSD 5.0 still has the same problem... Marc Responsible Changed From-To: dd->freebsd-bugs I still can't reproduce this with the originator's code, and don't have time to persure this further. Responsible Changed From-To: freebsd-bugs->mikeh I'll look at this in the next couple weeks. Well, I've looked at this a little more. I was able to reproduce it (it took a few times though). Unfortunately, the patch isn't as simple as the one in the PR. Could you please try the attached patch? There is still a problem though, and that is that the strlen()s can seg. fault if the argv[] strings aren't NULL terminated - I don't know how to fix this problem though :( Mike -- Mike Heffner <mheffner@[acm.]vt.edu> Blacksburg, VA <mikeh@FreeBSD.org> On Tue, Dec 11, 2001 at 11:18:54PM -0500, Mike Heffner wrote:
>
> Well, I've looked at this a little more. I was able to reproduce it (it
> took a few times though). Unfortunately, the patch isn't as simple as the
> one in the PR. Could you please try the attached patch? There is still a
> problem though, and that is that the strlen()s can seg. fault if the
> argv[] strings aren't NULL terminated - I don't know how to fix this
> problem though :(
If argv[] is the program arguments' array, as passed to main(), then
you should not worry about it - its elements are supposed to be proper
C strings, i.e. terminated by a '\0' character, and I still have to see
a platform where they are not :)
G'luck,
Peter
--
This sentence would be seven words long if it were six words shorter.
On 12-Dec-2001 Peter Pentchev wrote:
| On Tue, Dec 11, 2001 at 11:18:54PM -0500, Mike Heffner wrote:
|>
|> Well, I've looked at this a little more. I was able to reproduce it (it
|> took a few times though). Unfortunately, the patch isn't as simple as
|> the
|> one in the PR. Could you please try the attached patch? There is still
|> a
|> problem though, and that is that the strlen()s can seg. fault if the
|> argv[] strings aren't NULL terminated - I don't know how to fix this
|> problem though :(
|
| If argv[] is the program arguments' array, as passed to main(), then
| you should not worry about it - its elements are supposed to be proper
| C strings, i.e. terminated by a '\0' character, and I still have to see
| a platform where they are not :)
But when a user modifies those arguments by explicilty setting argv[0], or
whatever, is where the problem is:
test5.c:
#include <stdio.h>
#include <sys/exec.h>
#include <sys/param.h>
#include <sys/sysctl.h>
#include <sys/types.h>
#include <unistd.h>
int
main(int argc, char *argv[])
{
int oid[4];
char before[] = "BBBBBBB";
char after[5];
memset(after, 'A', sizeof(after));
argv[0] = after;
oid[0] = CTL_KERN;
oid[1] = KERN_PROC;
oid[2] = KERN_PROC_ARGS;
oid[3] = getpid();
sysctl(oid, 4, 0, 0, after, 65537);
sleep(600);
return(0);
}
$ ./test5
on another terminal:
$ ps auxwww
...
spock 290 0.0 0.3 980 109 p0 S+ 10:30AM 0:00.01 \
AAAAA\M-{\M-?\M-?BBBBBBB (test5)
^^^^^^^^^^^^^^^^^^^
Mike
--
Mike Heffner <mheffner@[acm.]vt.edu>
Blacksburg, VA <mikeh@FreeBSD.org>
State Changed From-To: feedback->analyzed Fix committed to current, awaiting MFC. State Changed From-To: analyzed->closed Fix MFC'd. |
