diff options
| author | Daiki Ueno <dueno@redhat.com> | 2019-07-08 16:54:56 +0200 |
|---|---|---|
| committer | Daiki Ueno <dueno@redhat.com> | 2019-07-10 15:11:45 +0200 |
| commit | 1f6bbceeeeb613cf4d790874bdd1e917a7071159 (patch) | |
| tree | c71055a658ff5ea02012d7b75e84f195224e8235 | |
| parent | 96a6ee60395242b015d712ca4a5e9e3501105297 (diff) | |
| download | gnutls-tmp-session-ticket-valgrind.tar.gz | |
ext/session_ticket: avoid calling memcpy on overlapping memory areastmp-session-ticket-valgrind
In _gnutls_encrypt_session_ticket, ticket.encrypted_state is allocated
from ticket_data->data, thus those memory areas may overlap. Using
memcpy here leads to undefined behavior.
Spotted by valgrind run on ppc64le.
==95231== Source and destination overlap in memcpy(0x47ce3a2, 0x47ce3a2, 160)
==95231== at 0x408A840: memcpy (vg_replace_strmem.c:1023)
==95231== by 0x424EE9F: pack_ticket (session_ticket.c:139)
==95231== by 0x424FA4F: _gnutls_encrypt_session_ticket (session_ticket.c:335)
==95231== by 0x4199E3B: generate_session_ticket (session_ticket.c:249)
==95231== by 0x419A333: _gnutls13_send_session_ticket (session_ticket.c:307)
==95231== by 0x40F8817: _gnutls13_handshake_server (handshake-tls13.c:511)
==95231== by 0x4110DEB: handshake_server (handshake.c:3331)
==95231== by 0x410C70B: gnutls_handshake (handshake.c:2727)
==95231== by 0x10009EBF: retry_handshake (serv.c:1306)
==95231== by 0x1000AB67: tcp_server (serv.c:1500)
==95231== by 0x10009E5B: main (serv.c:1297)
==95231==
Signed-off-by: Daiki Ueno <dueno@redhat.com>
| -rw-r--r-- | lib/ext/session_ticket.c | 6 |
1 files changed, 5 insertions, 1 deletions
diff --git a/lib/ext/session_ticket.c b/lib/ext/session_ticket.c index 09e240c2d0..98db39ff88 100644 --- a/lib/ext/session_ticket.c +++ b/lib/ext/session_ticket.c @@ -136,7 +136,11 @@ pack_ticket(const struct ticket_st *ticket, gnutls_datum_t *ticket_data) _gnutls_write_uint16(ticket->encrypted_state_len, p); p += 2; - memcpy(p, ticket->encrypted_state, ticket->encrypted_state_len); + /* We use memmove instead of memcpy here because + * ticket->encrypted_state is allocated from + * ticket_data->data, and thus both memory areas may overlap. + */ + memmove(p, ticket->encrypted_state, ticket->encrypted_state_len); p += ticket->encrypted_state_len; memcpy(p, ticket->mac, TICKET_MAC_SIZE); |