[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] client: update max protocol exchange size


The standard (at http://tools.ietf.org/html/rfc4253#section-4.2 ) says:

"The maximum length of the string is 255 characters, including the
Carriage Return and Line Feed."

The if clause before the one you edited returns if buffer[i] == '\n'.
So if we want to permit exactly the maximum length of banner permitted
by the standard it seems to me the maximum value i should be allowed
to take there is 254 (making a 255 character buffer), making the value
that should be where you edited 253.  Though maybe that wasn't your
goal?

I'll note the standard also seems to talk about other things there
that I'm not sure we do (though I haven't looked thoroughly, so maybe
we do this elsewhere?), like accommodating lines that end in CR that
do not start with "SSH-" before the actual version string.

On Sat, Mar 22, 2014 at 9:02 AM, Luka Perkov <luka.perkov@xxxxxxxxxx> wrote:
> RFC4253 in section 4.2. (Protocol Version Exchange) defines maximum size of 255
> characters.
>
> Signed-off-by: Luka Perkov <luka.perkov@xxxxxxxxxx>
> ---
>  src/client.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/client.c b/src/client.c
> index af525c1..c4ea60b 100644
> --- a/src/client.c
> +++ b/src/client.c
> @@ -126,7 +126,7 @@ static int callback_receive_banner(const void *data, size_t len, void *user) {
>
>                 return ret;
>         }
> -       if(i>127){
> +       if(i>255){
>                 /* Too big banner */
>                 session->session_state=SSH_SESSION_STATE_ERROR;
>                 ssh_set_error(session,SSH_FATAL,"Receiving banner: too large banner");
> --
> 1.9.1
>
>

References:
[PATCH] client: update max protocol exchange sizeLuka Perkov <luka.perkov@xxxxxxxxxx>
Archive administrator: postmaster@lists.cynapses.org