bugtraq - Re: Security Audit Notes: OpenSSL d1_srvr.c Overflow

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date: Wed, 18 Mar 2015 20:15:50 +0100
From: Jann Horn <jann@...jh.net>
To: "Nicholas Lemonias." <lem.nikolas@...glemail.com>
Cc: bugtraq@...urityfocus.com, cve-assign@...re.org
Subject: Re: Security Audit Notes: OpenSSL d1_srvr.c Overflow - Advanced
 Information Security

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;

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)