Re: Security Audit Notes: OpenSSL d1_srvr.c Overflow - Advanced Information Security

[Jann Horn <jann () thejh net>]

On Tue, Mar 17, 2015 at 08:40:53PM +0000, Nicholas Lemonias. wrote:
> The provided  buffer (s->d1->cookie) is for the user-app to fill in;
> however, a custom user application could provide an overlong cookie to
> exceed it's buffer, and to overflow beyond and into other volatile
> memory locations.
>
> Therefore any security is purely on the basis of trust that the remote
> peer will not act contrary to protocol, and that the user app  is
> trusted not to abuse functionality.

How would the remote peer influence what cookie a program returns in the
app_gen_cookie_cb callback? As far as I can tell, the cookie is chosen
by the local software, not by the remote peer. It also seems very
unlikely that the remote peer can influence it in any way, given that
there is no reason to let anything apart from the peer's IP address
influence the cookie generation. And a developer wondering about what a
cookie even is is going to look into the DTLS spec, where the maximum
length of a cookie is clearly shown.

I see how adding a bounds check there would be a bit nicer, but I can't
see any security issue here.


>  PoC - Code Snippet
>  ========================
>  (.../openssl/ssl/d1_srvr.c)
>
>  { Lines 918 - 942 }
>
>
> int dtls1_send_hello_verify_request(SSL *s)
> {
>     unsigned int msg_len;
>     unsigned char *msg, *buf, *p;
>
>     if (s->state == DTLS1_ST_SW_HELLO_VERIFY_REQUEST_A) {
>         buf = (unsigned char *)s->init_buf->data;
>
>        msg = p = &(buf[DTLS1_HM_HEADER_LENGTH]); //buf is equal to 12
>         /* Always use DTLS 1.0 version: see RFC 6347 */
>         *(p++) = DTLS1_VERSION >> 8;
>         *(p++) = DTLS1_VERSION & 0xFF;
>
>         if (s->ctx->app_gen_cookie_cb == NULL ||
>             s->ctx->app_gen_cookie_cb(s, s->d1->cookie,
>                                       &(s->d1->cookie_len)) == 0) {
>             SSLerr(SSL_F_DTLS1_SEND_HELLO_VERIFY_REQUEST,
>                    ERR_R_INTERNAL_ERROR);
>             return 0;
>         }
>
>         *(p++) = (unsigned char)s->d1->cookie_len;
>         memcpy(p, s->d1->cookie, s->d1->cookie_len);
> // Cookie_len is provided by a callback function above - and where,
> input is  // provided by the user-app.
>
>         p += s->d1->cookie_len;
>         msg_len = p - msg;

Attachment: signature.asc
Description: Digital signature