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
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/li...enssh-unix-dev