openssh unix dev - Aug 2013 - ssh.c - allocated wrong size for sensitive_data.keys?

Hello,

There's a memory allocation for sensitive_data.keys in ssh.c:848 which uses
size of Key instead of Key*.
This is probably harmless but seems to be wrong.

--- a/ssh.c
+++ b/ssh.c
@@ -846,7 +846,7 @@ main(int ac, char **av)
             options.hostbased_authentication) {
                 sensitive_data.nkeys = 7;
                 sensitive_data.keys = xcalloc(sensitive_data.nkeys,
-                   sizeof(Key));
+                   sizeof(Key *));
                 for (i = 0; i < sensitive_data.nkeys; i++)
                         sensitive_data.keys[i] = NULL;



Petr

On 14/08/13 11:23, Petr Lautrbach wrote:
> Hello,
>
> There's a memory allocation for sensitive_data.keys in ssh.c:848 which 
> uses size of Key instead of Key*.
> This is probably harmless but seems to be wrong.
>
> --- a/ssh.c
> +++ b/ssh.c
> @@ -846,7 +846,7 @@ main(int ac, char **av)
>             options.hostbased_authentication) {
>                 sensitive_data.nkeys = 7;
>                 sensitive_data.keys = xcalloc(sensitive_data.nkeys,
> -                   sizeof(Key));
> +                   sizeof(Key *));
>                 for (i = 0; i < sensitive_data.nkeys; i++)
>                         sensitive_data.keys[i] = NULL;
>
>
> Petr
I agree with you. Moreover, why is the loop below? The calloc should 
take care of it.
And if we are on one of those weird machines where NULL is not 
represented by zero bytes (are there still
boxes like this out there?), when the xcalloc can be a 
xmalloc(sensitive_data.nkeys * sizeof(Key*));

And if this wasn't enough, the next lines set them again* Those 7 slots 
are written thrice between line 819 and 842.
(plus a fourth time on 850-865, but that's conditional to the previous 
result)


* Ok... it would need a couple of #else for the case where 
OPENSSL_HAS_ECC is not defined.