Add extra validation of EAP header length field
These validation steps are already done in the EAP parsing code and in the EAP methods, but the additional check is defensive programming and can make the validation of received EAP messages more easier to understand. Signed-hostap: Jouni Malinen <j@w1.fi>
This commit is contained in:
parent
bf0d9ae40f
commit
f54eb34995
4 changed files with 66 additions and 17 deletions
|
@ -1,6 +1,6 @@
|
||||||
/*
|
/*
|
||||||
* EAP common peer/server definitions
|
* EAP common peer/server definitions
|
||||||
* Copyright (c) 2004-2007, Jouni Malinen <j@w1.fi>
|
* Copyright (c) 2004-2012, Jouni Malinen <j@w1.fi>
|
||||||
*
|
*
|
||||||
* This software may be distributed under the terms of the BSD license.
|
* This software may be distributed under the terms of the BSD license.
|
||||||
* See README for more details.
|
* See README for more details.
|
||||||
|
@ -12,6 +12,41 @@
|
||||||
#include "eap_defs.h"
|
#include "eap_defs.h"
|
||||||
#include "eap_common.h"
|
#include "eap_common.h"
|
||||||
|
|
||||||
|
/**
|
||||||
|
* eap_hdr_len_valid - Validate EAP header length field
|
||||||
|
* @msg: EAP frame (starting with EAP header)
|
||||||
|
* @min_payload: Minimum payload length needed
|
||||||
|
* Returns: 1 for valid header, 0 for invalid
|
||||||
|
*
|
||||||
|
* This is a helper function that does minimal validation of EAP messages. The
|
||||||
|
* length field is verified to be large enough to include the header and not
|
||||||
|
* too large to go beyond the end of the buffer.
|
||||||
|
*/
|
||||||
|
int eap_hdr_len_valid(const struct wpabuf *msg, size_t min_payload)
|
||||||
|
{
|
||||||
|
const struct eap_hdr *hdr;
|
||||||
|
size_t len;
|
||||||
|
|
||||||
|
if (msg == NULL)
|
||||||
|
return 0;
|
||||||
|
|
||||||
|
hdr = wpabuf_head(msg);
|
||||||
|
|
||||||
|
if (wpabuf_len(msg) < sizeof(*hdr)) {
|
||||||
|
wpa_printf(MSG_INFO, "EAP: Too short EAP frame");
|
||||||
|
return 0;
|
||||||
|
}
|
||||||
|
|
||||||
|
len = be_to_host16(hdr->length);
|
||||||
|
if (len < sizeof(*hdr) + min_payload || len > wpabuf_len(msg)) {
|
||||||
|
wpa_printf(MSG_INFO, "EAP: Invalid EAP length");
|
||||||
|
return 0;
|
||||||
|
}
|
||||||
|
|
||||||
|
return 1;
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* eap_hdr_validate - Validate EAP header
|
* eap_hdr_validate - Validate EAP header
|
||||||
* @vendor: Expected EAP Vendor-Id (0 = IETF)
|
* @vendor: Expected EAP Vendor-Id (0 = IETF)
|
||||||
|
@ -35,19 +70,11 @@ const u8 * eap_hdr_validate(int vendor, EapType eap_type,
|
||||||
const u8 *pos;
|
const u8 *pos;
|
||||||
size_t len;
|
size_t len;
|
||||||
|
|
||||||
|
if (!eap_hdr_len_valid(msg, 1))
|
||||||
|
return NULL;
|
||||||
|
|
||||||
hdr = wpabuf_head(msg);
|
hdr = wpabuf_head(msg);
|
||||||
|
|
||||||
if (wpabuf_len(msg) < sizeof(*hdr)) {
|
|
||||||
wpa_printf(MSG_INFO, "EAP: Too short EAP frame");
|
|
||||||
return NULL;
|
|
||||||
}
|
|
||||||
|
|
||||||
len = be_to_host16(hdr->length);
|
len = be_to_host16(hdr->length);
|
||||||
if (len < sizeof(*hdr) + 1 || len > wpabuf_len(msg)) {
|
|
||||||
wpa_printf(MSG_INFO, "EAP: Invalid EAP length");
|
|
||||||
return NULL;
|
|
||||||
}
|
|
||||||
|
|
||||||
pos = (const u8 *) (hdr + 1);
|
pos = (const u8 *) (hdr + 1);
|
||||||
|
|
||||||
if (*pos == EAP_TYPE_EXPANDED) {
|
if (*pos == EAP_TYPE_EXPANDED) {
|
||||||
|
|
|
@ -1,6 +1,6 @@
|
||||||
/*
|
/*
|
||||||
* EAP common peer/server definitions
|
* EAP common peer/server definitions
|
||||||
* Copyright (c) 2004-2007, Jouni Malinen <j@w1.fi>
|
* Copyright (c) 2004-2012, Jouni Malinen <j@w1.fi>
|
||||||
*
|
*
|
||||||
* This software may be distributed under the terms of the BSD license.
|
* This software may be distributed under the terms of the BSD license.
|
||||||
* See README for more details.
|
* See README for more details.
|
||||||
|
@ -11,6 +11,7 @@
|
||||||
|
|
||||||
#include "wpabuf.h"
|
#include "wpabuf.h"
|
||||||
|
|
||||||
|
int eap_hdr_len_valid(const struct wpabuf *msg, size_t min_payload);
|
||||||
const u8 * eap_hdr_validate(int vendor, EapType eap_type,
|
const u8 * eap_hdr_validate(int vendor, EapType eap_type,
|
||||||
const struct wpabuf *msg, size_t *plen);
|
const struct wpabuf *msg, size_t *plen);
|
||||||
struct wpabuf * eap_msg_alloc(int vendor, EapType type, size_t payload_len,
|
struct wpabuf * eap_msg_alloc(int vendor, EapType type, size_t payload_len,
|
||||||
|
|
|
@ -350,6 +350,8 @@ SM_STATE(EAP, METHOD)
|
||||||
}
|
}
|
||||||
|
|
||||||
eapReqData = eapol_get_eapReqData(sm);
|
eapReqData = eapol_get_eapReqData(sm);
|
||||||
|
if (!eap_hdr_len_valid(eapReqData, 1))
|
||||||
|
return;
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Get ignore, methodState, decision, allowNotifications, and
|
* Get ignore, methodState, decision, allowNotifications, and
|
||||||
|
@ -438,6 +440,8 @@ SM_STATE(EAP, IDENTITY)
|
||||||
|
|
||||||
SM_ENTRY(EAP, IDENTITY);
|
SM_ENTRY(EAP, IDENTITY);
|
||||||
eapReqData = eapol_get_eapReqData(sm);
|
eapReqData = eapol_get_eapReqData(sm);
|
||||||
|
if (!eap_hdr_len_valid(eapReqData, 1))
|
||||||
|
return;
|
||||||
eap_sm_processIdentity(sm, eapReqData);
|
eap_sm_processIdentity(sm, eapReqData);
|
||||||
wpabuf_free(sm->eapRespData);
|
wpabuf_free(sm->eapRespData);
|
||||||
sm->eapRespData = NULL;
|
sm->eapRespData = NULL;
|
||||||
|
@ -454,6 +458,8 @@ SM_STATE(EAP, NOTIFICATION)
|
||||||
|
|
||||||
SM_ENTRY(EAP, NOTIFICATION);
|
SM_ENTRY(EAP, NOTIFICATION);
|
||||||
eapReqData = eapol_get_eapReqData(sm);
|
eapReqData = eapol_get_eapReqData(sm);
|
||||||
|
if (!eap_hdr_len_valid(eapReqData, 1))
|
||||||
|
return;
|
||||||
eap_sm_processNotify(sm, eapReqData);
|
eap_sm_processNotify(sm, eapReqData);
|
||||||
wpabuf_free(sm->eapRespData);
|
wpabuf_free(sm->eapRespData);
|
||||||
sm->eapRespData = NULL;
|
sm->eapRespData = NULL;
|
||||||
|
@ -871,13 +877,17 @@ static struct wpabuf * eap_sm_buildNak(struct eap_sm *sm, int id)
|
||||||
|
|
||||||
static void eap_sm_processIdentity(struct eap_sm *sm, const struct wpabuf *req)
|
static void eap_sm_processIdentity(struct eap_sm *sm, const struct wpabuf *req)
|
||||||
{
|
{
|
||||||
const struct eap_hdr *hdr = wpabuf_head(req);
|
const u8 *pos;
|
||||||
const u8 *pos = (const u8 *) (hdr + 1);
|
size_t msg_len;
|
||||||
pos++;
|
|
||||||
|
|
||||||
wpa_msg(sm->msg_ctx, MSG_INFO, WPA_EVENT_EAP_STARTED
|
wpa_msg(sm->msg_ctx, MSG_INFO, WPA_EVENT_EAP_STARTED
|
||||||
"EAP authentication started");
|
"EAP authentication started");
|
||||||
|
|
||||||
|
pos = eap_hdr_validate(EAP_VENDOR_IETF, EAP_TYPE_IDENTITY, req,
|
||||||
|
&msg_len);
|
||||||
|
if (pos == NULL)
|
||||||
|
return;
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* RFC 3748 - 5.1: Identity
|
* RFC 3748 - 5.1: Identity
|
||||||
* Data field may contain a displayable message in UTF-8. If this
|
* Data field may contain a displayable message in UTF-8. If this
|
||||||
|
@ -888,7 +898,7 @@ static void eap_sm_processIdentity(struct eap_sm *sm, const struct wpabuf *req)
|
||||||
/* TODO: could save displayable message so that it can be shown to the
|
/* TODO: could save displayable message so that it can be shown to the
|
||||||
* user in case of interaction is required */
|
* user in case of interaction is required */
|
||||||
wpa_hexdump_ascii(MSG_DEBUG, "EAP: EAP-Request Identity data",
|
wpa_hexdump_ascii(MSG_DEBUG, "EAP: EAP-Request Identity data",
|
||||||
pos, be_to_host16(hdr->length) - 5);
|
pos, msg_len);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
|
|
@ -275,6 +275,11 @@ SM_STATE(EAP, INTEGRITY_CHECK)
|
||||||
{
|
{
|
||||||
SM_ENTRY(EAP, INTEGRITY_CHECK);
|
SM_ENTRY(EAP, INTEGRITY_CHECK);
|
||||||
|
|
||||||
|
if (!eap_hdr_len_valid(sm->eap_if.eapRespData, 1)) {
|
||||||
|
sm->ignore = TRUE;
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
if (sm->m->check) {
|
if (sm->m->check) {
|
||||||
sm->ignore = sm->m->check(sm, sm->eap_method_priv,
|
sm->ignore = sm->m->check(sm, sm->eap_method_priv,
|
||||||
sm->eap_if.eapRespData);
|
sm->eap_if.eapRespData);
|
||||||
|
@ -309,6 +314,9 @@ SM_STATE(EAP, METHOD_RESPONSE)
|
||||||
{
|
{
|
||||||
SM_ENTRY(EAP, METHOD_RESPONSE);
|
SM_ENTRY(EAP, METHOD_RESPONSE);
|
||||||
|
|
||||||
|
if (!eap_hdr_len_valid(sm->eap_if.eapRespData, 1))
|
||||||
|
return;
|
||||||
|
|
||||||
sm->m->process(sm, sm->eap_method_priv, sm->eap_if.eapRespData);
|
sm->m->process(sm, sm->eap_method_priv, sm->eap_if.eapRespData);
|
||||||
if (sm->m->isDone(sm, sm->eap_method_priv)) {
|
if (sm->m->isDone(sm, sm->eap_method_priv)) {
|
||||||
eap_sm_Policy_update(sm, NULL, 0);
|
eap_sm_Policy_update(sm, NULL, 0);
|
||||||
|
@ -380,6 +388,9 @@ SM_STATE(EAP, NAK)
|
||||||
}
|
}
|
||||||
sm->m = NULL;
|
sm->m = NULL;
|
||||||
|
|
||||||
|
if (!eap_hdr_len_valid(sm->eap_if.eapRespData, 1))
|
||||||
|
return;
|
||||||
|
|
||||||
nak = wpabuf_head(sm->eap_if.eapRespData);
|
nak = wpabuf_head(sm->eap_if.eapRespData);
|
||||||
if (nak && wpabuf_len(sm->eap_if.eapRespData) > sizeof(*nak)) {
|
if (nak && wpabuf_len(sm->eap_if.eapRespData) > sizeof(*nak)) {
|
||||||
len = be_to_host16(nak->length);
|
len = be_to_host16(nak->length);
|
||||||
|
|
Loading…
Reference in a new issue