diff options
author | Michael Brown <mcb30@ipxe.org> | 2015-07-31 23:47:50 +0100 |
---|---|---|
committer | Michael Brown <mcb30@ipxe.org> | 2015-08-01 00:06:58 +0100 |
commit | 1ac743411192af0a83043c9135039a1f1269608e (patch) | |
tree | 69fe7edb6f6193fe0ff424aeb9dbf3896b043bcf /src/net | |
parent | 2849932c4853ba36edafb5ae16c67e5976d95372 (diff) | |
download | ipxe-1ac743411192af0a83043c9135039a1f1269608e.zip ipxe-1ac743411192af0a83043c9135039a1f1269608e.tar.gz ipxe-1ac743411192af0a83043c9135039a1f1269608e.tar.bz2 |
[tls] Do not access beyond the end of a 24-bit integer
The current implementation handles big-endian 24-bit integers (which
occur in several TLS record types) by treating them as big-endian
32-bit integers which are shifted by 8 bits. This can result in
"Invalid read" errors when running under valgrind, if the 24-bit field
happens to be exactly at the end of an I/O buffer.
Fix by ensuring that we touch only the three bytes which comprise the
24-bit integer.
Signed-off-by: Michael Brown <mcb30@ipxe.org>
Diffstat (limited to 'src/net')
-rw-r--r-- | src/net/tls.c | 51 |
1 files changed, 29 insertions, 22 deletions
diff --git a/src/net/tls.c b/src/net/tls.c index 8f4bec7..58e958b 100644 --- a/src/net/tls.c +++ b/src/net/tls.c @@ -179,20 +179,29 @@ static void tls_clear_cipher ( struct tls_session *tls, ****************************************************************************** */ +/** A TLS 24-bit integer + * + * TLS uses 24-bit integers in several places, which are awkward to + * parse in C. + */ +typedef struct { + /** High byte */ + uint8_t high; + /** Low word */ + uint16_t low; +} __attribute__ (( packed )) tls24_t; + /** * Extract 24-bit field value * * @v field24 24-bit field * @ret value Field value * - * TLS uses 24-bit integers in several places, which are awkward to - * parse in C. */ static inline __attribute__ (( always_inline )) unsigned long -tls_uint24 ( const uint8_t field24[3] ) { - const uint32_t *field32 __attribute__ (( may_alias )) = - ( ( const void * ) field24 ); - return ( be32_to_cpu ( *field32 ) >> 8 ); +tls_uint24 ( const tls24_t *field24 ) { + + return ( ( field24->high << 16 ) | be16_to_cpu ( field24->low ) ); } /** @@ -200,13 +209,11 @@ tls_uint24 ( const uint8_t field24[3] ) { * * @v field24 24-bit field * @v value Field value - * - * The field must be pre-zeroed. */ -static void tls_set_uint24 ( uint8_t field24[3], unsigned long value ) { - uint32_t *field32 __attribute__ (( may_alias )) = - ( ( void * ) field24 ); - *field32 |= cpu_to_be32 ( value << 8 ); +static void tls_set_uint24 ( tls24_t *field24, unsigned long value ) { + + field24->high = ( value >> 16 ); + field24->low = cpu_to_be16 ( value ); } /** @@ -1038,9 +1045,9 @@ static int tls_send_client_hello ( struct tls_session *tls ) { static int tls_send_certificate ( struct tls_session *tls ) { struct { uint32_t type_length; - uint8_t length[3]; + tls24_t length; struct { - uint8_t length[3]; + tls24_t length; uint8_t data[ tls->cert->raw.len ]; } __attribute__ (( packed )) certificates[1]; } __attribute__ (( packed )) *certificate; @@ -1058,9 +1065,9 @@ static int tls_send_certificate ( struct tls_session *tls ) { ( cpu_to_le32 ( TLS_CERTIFICATE ) | htonl ( sizeof ( *certificate ) - sizeof ( certificate->type_length ) ) ); - tls_set_uint24 ( certificate->length, + tls_set_uint24 ( &certificate->length, sizeof ( certificate->certificates ) ); - tls_set_uint24 ( certificate->certificates[0].length, + tls_set_uint24 ( &certificate->certificates[0].length, sizeof ( certificate->certificates[0].data ) ); memcpy ( certificate->certificates[0].data, tls->cert->raw.data, @@ -1412,7 +1419,7 @@ static int tls_parse_chain ( struct tls_session *tls, const void *data, size_t len ) { const void *end = ( data + len ); const struct { - uint8_t length[3]; + tls24_t length; uint8_t data[0]; } __attribute__ (( packed )) *certificate; size_t certificate_len; @@ -1436,7 +1443,7 @@ static int tls_parse_chain ( struct tls_session *tls, /* Extract raw certificate data */ certificate = data; - certificate_len = tls_uint24 ( certificate->length ); + certificate_len = tls_uint24 ( &certificate->length ); next = ( certificate->data + certificate_len ); if ( next > end ) { DBGC ( tls, "TLS %p overlength certificate:\n", tls ); @@ -1482,10 +1489,10 @@ static int tls_parse_chain ( struct tls_session *tls, static int tls_new_certificate ( struct tls_session *tls, const void *data, size_t len ) { const struct { - uint8_t length[3]; + tls24_t length; uint8_t certificates[0]; } __attribute__ (( packed )) *certificate = data; - size_t certificates_len = tls_uint24 ( certificate->length ); + size_t certificates_len = tls_uint24 ( &certificate->length ); const void *end = ( certificate->certificates + certificates_len ); int rc; @@ -1634,11 +1641,11 @@ static int tls_new_handshake ( struct tls_session *tls, while ( data != end ) { const struct { uint8_t type; - uint8_t length[3]; + tls24_t length; uint8_t payload[0]; } __attribute__ (( packed )) *handshake = data; const void *payload = &handshake->payload; - size_t payload_len = tls_uint24 ( handshake->length ); + size_t payload_len = tls_uint24 ( &handshake->length ); const void *next = ( payload + payload_len ); /* Sanity check */ |