Skip to content

Commit 9eff07c

Browse files
committed
sec: introduce radius_client_sec_prot_int_t.recv_eap_msg_own
An invalid free() could occur in radius_client_sec_prot_release() when the received message came from another module through radius_client_sec_prot_radius_eap_receive(). WARN: This is not a proper fix, valgrind still reports invalid memory writes, and there are likely additional edge cases which could create security issues.
1 parent 0124adc commit 9eff07c

File tree

1 file changed

+7
-1
lines changed

1 file changed

+7
-1
lines changed

app_wsbrd/security/protocols/radius_sec_prot/radius_client_sec_prot.c

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ typedef struct radius_client_sec_prot_int {
9595
uint8_t new_pmk[PMK_LEN]; /**< New Pair Wise Master Key */
9696
uint16_t recv_eap_msg_len; /**< Received EAP message length */
9797
uint8_t *recv_eap_msg; /**< Received EAP message */
98+
bool recv_eap_msg_own; /**< This module owns the received buffer and must manage its memory */
9899
uint16_t send_radius_msg_len; /**< Send radius message length */
99100
uint8_t *send_radius_msg; /**< Send radius message */
100101
uint8_t identity_len; /**< Supplicant EAP identity length */
@@ -257,7 +258,7 @@ static void radius_client_sec_prot_release(sec_prot_t *prot)
257258
{
258259
radius_client_sec_prot_int_t *data = radius_client_sec_prot_get(prot);
259260

260-
if (data->recv_eap_msg != NULL) {
261+
if (data->recv_eap_msg_own && data->recv_eap_msg != NULL) {
261262
free(data->recv_eap_msg);
262263
}
263264
if (data->send_radius_msg != NULL) {
@@ -454,6 +455,7 @@ static int8_t radius_client_sec_prot_receive(sec_prot_t *prot, const void *pdu,
454455

455456
// Allocate memory for continuous EAP buffer
456457
data->recv_eap_msg = malloc(data->recv_eap_msg_len + data->radius_eap_tls_header_size);
458+
data->recv_eap_msg_own = true;
457459
if (data->recv_eap_msg == NULL) {
458460
tr_error("Cannot allocate eap msg");
459461
return -1;
@@ -522,6 +524,10 @@ static int8_t radius_client_sec_prot_radius_eap_receive(sec_prot_t *prot, const
522524

523525
data->recv_eap_msg_len = size;
524526
data->recv_eap_msg = (uint8_t *)pdu; // FIXME
527+
// FIXME: This buffer can be either allocated from this module, or come
528+
// from the outside. Ownership should be changed to be consistent for all
529+
// usages.
530+
data->recv_eap_msg_own = false;
525531

526532
prot->state_machine(prot);
527533

0 commit comments

Comments
 (0)