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;
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
> 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;
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
iQIcBAEBAgAGBQJVCc7mAAoJED4KNFJOeCOo6lIQAJmxe+nZrtkq1QHnmHG8VxPl
KZP61QoVaprwjIEjm/9vHY7mueuB77jWXIzvFvTadoyhic0NeJNI/OLCNqv3NGUG
MxotkKqGhzu4gWTHpWfdJx0B9EyAJ6PKma35HlmRKvdzrogL1WY6DEU8z1DDhTVm
fObu6hP2vdDXbhun/14Wr0Wmbuu4+VJPSVqVxEsFaTyM44MzdD9aRUebW2bTJ7+R
Qse4XKnZQDr0bnZObNYAoMCqnqtLe9oFP29OmuGJbIeJEyitSMNsf3O88b8uoF32
kECPVhbpWNFNfOGpLmDBRlwXfEQMzvJG5P45xRGHYxqTcIzbcEAaudtdKy70yGhz
EKmBvMa9CDqAzRr2/yzHOOdj4OWyn+x1MC07oG9NlORFp7k1h4SxWzzduBcOiY7/
2DRqzOGpY+a/z743iadiT4tVJxK2DTp7sFRiNYI8KJV7zZrWQj/bUgYlpI89Rjyr
LJf9AGwKgUKYeY/Q2tLCW61wBi4HTrFRHNI6FCTwfMm5GRR8iIpBGhOBdflRi46I
JYjbR7tsZrJYa3r1Kx1B+iQGfoSVNETsqq4qJ403Jz751Ab7vAasoelxqt7fP6AS
M+O2l3yhFPtuWhTprjbB2uwTBeeWFlIm3s5dbTyjNzQeTXj3OVLGQX37MfBxm6Xq
OCF32X+323r0zkMOUxKV
=OhpD
-----END PGP SIGNATURE-----
[ reply ]