jeltz/hostap
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

TLS: Avoid undefined behavior in pointer arithmetic

Browse source 
Reorder terms in a way that no invalid pointers are generated with
pos+len operations. end-pos is always defined (with a valid pos pointer)
while pos+len could end up pointing beyond the end pointer which would
be undefined behavior.

Signed-off-by: Jouni Malinen <j@w1.fi>
This commit is contained in:
Jouni Malinen 2015-10-18 17:28:35 +03:00
parent 2461724c05
commit d2eb91e08f
3 changed files with 32 additions and 16 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

16
src/tls/tlsv1_client_write.c
View file

@ -134,6 +134,11 @@ static int tls_write_client_certificate(struct tlsv1_client *conn,
struct x509_certificate *cert;
pos = *msgpos;
if (TLS_RECORD_HEADER_LEN + 1 + 3 + 3 > end - pos) {
tls_alert(conn, TLS_ALERT_LEVEL_FATAL,
TLS_ALERT_INTERNAL_ERROR);
return -1;
}
wpa_printf(MSG_DEBUG, "TLSv1: Send Certificate");
rhdr = pos;
@ -154,7 +159,7 @@ static int tls_write_client_certificate(struct tlsv1_client *conn,
pos += 3;
cert = conn->cred ? conn->cred->cert : NULL;
while (cert) {
if (pos + 3 + cert->cert_len > end) {
if (3 + cert->cert_len > (size_t) (end - pos)) {
wpa_printf(MSG_DEBUG, "TLSv1: Not enough buffer space "
"for Certificate (cert_len=%lu left=%lu)",
(unsigned long) cert->cert_len,
@ -265,9 +270,16 @@ static int tlsv1_key_x_dh(struct tlsv1_client *conn, u8 **pos, u8 *end)
wpa_hexdump(MSG_DEBUG, "TLSv1: DH Yc (client's public value)",
dh_yc, dh_yc_len);
if (end - *pos < 2) {
tls_alert(conn, TLS_ALERT_LEVEL_FATAL,
TLS_ALERT_INTERNAL_ERROR);
os_free(csecret);
os_free(dh_yc);
return -1;
}
WPA_PUT_BE16(*pos, dh_yc_len);
*pos += 2;
if (*pos + dh_yc_len > end) {
if (dh_yc_len > (size_t) (end - *pos)) {
wpa_printf(MSG_DEBUG, "TLSv1: Not enough room in the "
"message buffer for Yc");
tls_alert(conn, TLS_ALERT_LEVEL_FATAL,

15
src/tls/tlsv1_server_write.c
View file

@ -168,6 +168,11 @@ static int tls_write_server_certificate(struct tlsv1_server *conn,
}
pos = *msgpos;
if (TLS_RECORD_HEADER_LEN + 1 + 3 + 3 > end - pos) {
tlsv1_server_alert(conn, TLS_ALERT_LEVEL_FATAL,
TLS_ALERT_INTERNAL_ERROR);
return -1;
}
tlsv1_server_log(conn, "Send Certificate");
rhdr = pos;
@ -188,7 +193,7 @@ static int tls_write_server_certificate(struct tlsv1_server *conn,
pos += 3;
cert = conn->cred->cert;
while (cert) {
if (pos + 3 + cert->cert_len > end) {
if (3 + cert->cert_len > (size_t) (end - pos)) {
wpa_printf(MSG_DEBUG, "TLSv1: Not enough buffer space "
"for Certificate (cert_len=%lu left=%lu)",
(unsigned long) cert->cert_len,
@ -371,7 +376,7 @@ static int tls_write_server_key_exchange(struct tlsv1_server *conn,
/* body - ServerDHParams */
server_params = pos;
/* dh_p */
if (pos + 2 + dh_p_len > end) {
if (2 + dh_p_len > (size_t) (end - pos)) {
wpa_printf(MSG_DEBUG, "TLSv1: Not enough buffer space for "
"dh_p");
tlsv1_server_alert(conn, TLS_ALERT_LEVEL_FATAL,
@ -385,7 +390,7 @@ static int tls_write_server_key_exchange(struct tlsv1_server *conn,
pos += dh_p_len;
/* dh_g */
if (pos + 2 + conn->cred->dh_g_len > end) {
if (2 + conn->cred->dh_g_len > (size_t) (end - pos)) {
wpa_printf(MSG_DEBUG, "TLSv1: Not enough buffer space for "
"dh_g");
tlsv1_server_alert(conn, TLS_ALERT_LEVEL_FATAL,
@ -399,7 +404,7 @@ static int tls_write_server_key_exchange(struct tlsv1_server *conn,
pos += conn->cred->dh_g_len;
/* dh_Ys */
if (pos + 2 + dh_ys_len > end) {
if (2 + dh_ys_len > (size_t) (end - pos)) {
wpa_printf(MSG_DEBUG, "TLSv1: Not enough buffer space for "
"dh_Ys");
tlsv1_server_alert(conn, TLS_ALERT_LEVEL_FATAL,
@ -457,7 +462,7 @@ static int tls_write_server_key_exchange(struct tlsv1_server *conn,
* SignatureAlgorithm signature;
* } SignatureAndHashAlgorithm;
*/
if (hlen < 0 || pos + 2 > end) {
if (hlen < 0 || end - pos < 2) {
tlsv1_server_alert(conn, TLS_ALERT_LEVEL_FATAL,
TLS_ALERT_INTERNAL_ERROR);
return -1;

17
src/tls/x509v3.c
View file

@ -199,12 +199,11 @@ static int x509_parse_algorithm_identifier(
hdr.class, hdr.tag);
return -1;
}
if (hdr.length > buf + len - hdr.payload)
return -1;
pos = hdr.payload;
end = pos + hdr.length;
if (end > buf + len)
return -1;
*next = end;
if (asn1_get_oid(pos, end - pos, &id->oid, &pos))
@ -243,7 +242,7 @@ static int x509_parse_public_key(const u8 *buf, size_t len,
}
pos = hdr.payload;
if (pos + hdr.length > end)
if (hdr.length > end - pos)
return -1;
end = pos + hdr.length;
*next = end;
@ -319,7 +318,7 @@ static int x509_parse_name(const u8 *buf, size_t len, struct x509_name *name,
}
pos = hdr.payload;
if (pos + hdr.length > buf + len)
if (hdr.length > buf + len - pos)
return -1;
end = *next = pos + hdr.length;
@ -677,7 +676,7 @@ static int x509_parse_validity(const u8 *buf, size_t len,
pos = hdr.payload;
plen = hdr.length;
if (pos + plen > buf + len)
if (plen > (size_t) (buf + len - pos))
return -1;
*next = pos + plen;
@ -801,7 +800,7 @@ static int x509_parse_ext_basic_constraints(struct x509_certificate *cert,
}
cert->ca = hdr.payload[0];
if (hdr.payload + hdr.length == pos + len) {
if (hdr.length == pos + len - hdr.payload) {
wpa_printf(MSG_DEBUG, "X509: BasicConstraints - cA=%d",
cert->ca);
return 0;
@ -1503,12 +1502,12 @@ struct x509_certificate * x509_certificate_parse(const u8 *buf, size_t len)
}
pos = hdr.payload;
if (pos + hdr.length > end) {
if (hdr.length > end - pos) {
x509_certificate_free(cert);
return NULL;
}
if (pos + hdr.length < end) {
if (hdr.length < end - pos) {
wpa_hexdump(MSG_MSGDUMP, "X509: Ignoring extra data after DER "
"encoded certificate",
pos + hdr.length, end - (pos + hdr.length));