OpenSSH PKCS#11merge

[Peter Stuge]

Hi again,

I could not resist looking at the code a bit.


On Tue, Jan 08, 2008 at 11:42:44PM +0100, Peter Stuge wrote:
> On Tue, Jan 08, 2008 at 03:39:15PM +0200, Alon Bar-Lev wrote:
> > However, I believe that it is somewhat confusing to introduce a
> > passphrase dialog for ok/cancel input... :)
> 
> ask_permission()

I see you already found it.


I've only had a quick look at 2000_all_pkcs11.patch and found some
simple but serious issues:

_pkcs11_ssh_pin_prompt() does not check for snprintf() failure, which
means that uninitialized memory may be passed to read_passphrase()
and at least in theory this is a buffer overflow vulnerability.

Also, there is a possible memory leak, since passphrase is not always
xfree()d; if the read passphrase is empty or longer than the max pin
length.

In _pkcs11_convert_to_ssh_key(), the choice of local variable names
make the code rather unreadable. It's not good to have one parameter
called key and a variable called _key. Please try to find better
names for all similar instances.

There may be other similar problems that I have not found because I
have not read the patch very carefully.

I really like where this is going though. :)


//Peter