Eric Warnke
2007-08-10 15:50:46 UTC
Since I like SSHKeychain and want to get the last few really annoying
bugs worked out I started reviewing the code. Here are some major
security issues with TunnerRunner.c and another reason that you should
never suid-root an application.
SECURITY BUG #1: Kill any process
Lines 41-46
if(argc == 3 && strstr(argv[1], "-k") != NULL) {
setuid(geteuid());
kill(atoi(argv[2]), SIGTERM);
exit(0);
}
Because it's suid-root you can kill ANY process in the system.
SECURITY BUG #2: Privilege escalation
Lines 53-56
if(strstr(argv[1], "ssh") == NULL)
{
exit(1);
}
Lines 132-137
/* Execute ssh. */
if(execv(argv[0], argv) == -1)
{
fprintf(stderr, "execv() failed (%s)\n", strerror(errno));
exit(1);
}
As long as your command has the name ssh in it somewhere
TunnelRunner.c will happily run it as root. This is basically a blank
slate exploit.
BUG #3: Weird code
Lines 149-153
for(;;)
{
/* Read until we get EOF. */
for(;;)
{
Lines 191-192
}
}
Two endless loops? In side these loops I can find no reason why
TunnerRunner even needs to be running. I'm probably missing something
obvious, but why does the parent even need to continue running? If
the parent doesn't need to continue running then why does tunnel
runner even need to fork in the first place, it could close the files
and execv.
The security is easy to fix, quit SSHKeychain and chmod 755
TunnelRunner. I have also chown'ed it as well.
Cheers,
Eric
----------------------
Eric Warnke
518-396-5277
***@gmail.com
bugs worked out I started reviewing the code. Here are some major
security issues with TunnerRunner.c and another reason that you should
never suid-root an application.
SECURITY BUG #1: Kill any process
Lines 41-46
if(argc == 3 && strstr(argv[1], "-k") != NULL) {
setuid(geteuid());
kill(atoi(argv[2]), SIGTERM);
exit(0);
}
Because it's suid-root you can kill ANY process in the system.
SECURITY BUG #2: Privilege escalation
Lines 53-56
if(strstr(argv[1], "ssh") == NULL)
{
exit(1);
}
Lines 132-137
/* Execute ssh. */
if(execv(argv[0], argv) == -1)
{
fprintf(stderr, "execv() failed (%s)\n", strerror(errno));
exit(1);
}
As long as your command has the name ssh in it somewhere
TunnelRunner.c will happily run it as root. This is basically a blank
slate exploit.
BUG #3: Weird code
Lines 149-153
for(;;)
{
/* Read until we get EOF. */
for(;;)
{
Lines 191-192
}
}
Two endless loops? In side these loops I can find no reason why
TunnerRunner even needs to be running. I'm probably missing something
obvious, but why does the parent even need to continue running? If
the parent doesn't need to continue running then why does tunnel
runner even need to fork in the first place, it could close the files
and execv.
The security is easy to fix, quit SSHKeychain and chmod 755
TunnelRunner. I have also chown'ed it as well.
Cheers,
Eric
----------------------
Eric Warnke
518-396-5277
***@gmail.com