[Bug 2474] Enabling ECDSA in PKCS#11 support for ssh-agent

bugzilla-daemon at bugzilla.mindrot.org bugzilla-daemon at bugzilla.mindrot.org
Tue Nov 28 04:28:33 AEDT 2017


--- Comment #14 from Dmitry S. <dsavints at gmail.com> ---
(In reply to Mathias from comment #12)
> Created attachment 3095 [details]
> Sixth iteration
> I've updated my patch with the fix from Dmitry [...]

Thanks Mathias - but I wonder if the fix was properly propagated.  In
my patch, I had the following in ssh-pkcs11.c function

+       /* identify key object on smartcard */
+       k11->keyid_len = keyid_attrib->ulValueLen;
+       if (k11->keyid_len > 0) {
+         k11->keyid = xmalloc(k11->keyid_len);
+       }

in your latest ("Sixth iteration") patch I see the statements in
different order:

+       /* identify key object on smartcard */
+       if (k11->keyid_len > 0) {
+               k11->keyid_len = keyid_attrib->ulValueLen;
+               k11->keyid = xmalloc(k11->keyid_len);
+    }

Is it a typo or have you done it for a reason that I'm missing?  Should
not we first extract the value from keyid_attrib->ulValueLen and assign
it to k11->keyid_len and only then use it in the if condiftion for zero
check?  This is how it is done in pkcs11_rsa_wrap in master ov

I believe in your code the k11->keyid_len is uninitialized and
therefore can take arbitrary values leading to undefined behavior. 
Please let me know if I'm missing something here.  If it is a bug, I
wonder if we could add a test to catch it, so it would fail on the
current patch and succeed with a fix?

You are receiving this mail because:
You are watching the assignee of the bug.

More information about the openssh-bugs mailing list