> 1) {
> 2) static time_t last_kill_time = 0;
> 3) if (time(NULL) - last_kill_time > 60 && getppid() != 1)
> 4) {
> 5) last_kill_time = time(NULL);
> 6) kill(SIGALRM, getppid());
> 7) }
> 8) fatal("Bad result from rsa_private_decrypt");
> 9) }
>
> actually...if we look at the lines that the patch adds, i think it's
> pretty clear that the variable last_kill_time, declared at line 2 to
> be static and initialized to 0, will always be 0 at line 3 (since it
> can't get set to anything else from other code...it's static), which
> means that the kill (and setting the actual of last_kill_time to
> something other than 0) will almost always take place (now - 0 is
> usually a lot more than 60), and the fact that it finally get set
> doesn't mean anything to anyone, since the only process that recorded
> that the signal was sent immediately exits at line 8.
You are right. I apologize for the lousy patch. It was done in five
minutes, and I e-mailed it to two people at core-sdi with a note "The
patch is untested, other than compiling it. I no longer have SSH1 running
on my machines. I really recommend upgrading to SSH2." I never intended
it to go out on bugtraq.
In any case, please do not use that patch. Upgrade to SSH2 instead; it
fixes a lot more security problems, some of them fundamental in the
protocol. Using SSH1 under SSH2 in compatibility mode (as described at
http://www.ssh.com/products/ssh/ssh2-ssh1-server-howto.html) solves the
vulnerability as well for SSH1 compatibility mode. (SSH2 was not
vulnerable to the key recovery issue anyway.)
Tatu
SSH Communications Security http://www.ssh.com/
SSH IPSEC Toolkit http://www.ipsec.com/
SSH(R) Secure Shell(TM) http://www.ssh.com/products/ssh
