(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]

Reply via email to