> 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.)


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

Reply via email to