(apologies for the duplicate. adding the OpenSSL RT prefix)
On Jul 11, 2014, at 1:10 AM, Philip Guenther <[email protected]> wrote:
> Hmm, I agree the n2s() calls can read past the end of the buffer, but the
> checks that then occur against param_len appear to be correct to me.
>
...
>
> Let's say a zero-length record was received, so n = 0. The n2s() will
> read past the end, but param_len is set to the read length plus 2 for the
> length bytes, so param_len > n should indeed be the correct test there.
> No?
>
> (param and param_len in this need to cover the entire record, as they're
> the input to the packet signature check.)
The problem I see and that our tool alerted me to is that this check (parem_len
> n) doesn't cover the *next* n2s (line 1416 for SRP). This is illustrated more
clearly at line 1431:
i = (unsigned int)(p[0]);
p++;
param_len+=i+1;
if (param_len > n)
{
al=SSL_AD_DECODE_ERROR;
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,SSL_R_BAD_SRP_S_LENGTH);
goto f_err;
}
Here, the i+1 only includes the byte we just read at 1431, not the next two
bytes read by n2s at line 1447. So, each check includes the 1-2 bytes just read
(too late!), not those that will be subsequently read. Any of these reads can
be forced slightly out-of-bounds by the right packet.
______________________________________________________________________
OpenSSL Project http://www.openssl.org
Development Mailing List [email protected]
Automated List Manager [email protected]