mediation-manager: Avoid potential use-after-free when checking online status
This is unlikely to be an issue in practice because only one caller
actually uses the ID and it does so immediately afterwards. So there is
only a tiny window in which the peer could terminate or rekey its SA to
cause the returned ID to get destroyed.
Fixes: d5cc175833 ("experimental P2P-NAT-T for IKEv2 merged back from branch")
This commit is contained in:
@@ -83,12 +83,14 @@ METHOD(job_t, execute, job_requeue_t,
|
|||||||
{
|
{
|
||||||
ike_sa_id_t *target_sa_id;
|
ike_sa_id_t *target_sa_id;
|
||||||
|
|
||||||
target_sa_id = charon->mediation_manager->check(charon->mediation_manager, this->target);
|
target_sa_id = charon->mediation_manager->check(charon->mediation_manager,
|
||||||
|
this->target);
|
||||||
|
|
||||||
if (target_sa_id)
|
if (target_sa_id)
|
||||||
{
|
{
|
||||||
ike_sa_t *target_sa = charon->ike_sa_manager->checkout(charon->ike_sa_manager,
|
ike_sa_t *target_sa = charon->ike_sa_manager->checkout(charon->ike_sa_manager,
|
||||||
target_sa_id);
|
target_sa_id);
|
||||||
|
target_sa_id->destroy(target_sa_id);
|
||||||
if (target_sa)
|
if (target_sa)
|
||||||
{
|
{
|
||||||
if (this->callback)
|
if (this->callback)
|
||||||
|
|||||||
@@ -259,7 +259,7 @@ METHOD(mediation_manager_t, check, ike_sa_id_t*,
|
|||||||
return NULL;
|
return NULL;
|
||||||
}
|
}
|
||||||
|
|
||||||
ike_sa_id = peer->ike_sa_id;
|
ike_sa_id = peer->ike_sa_id ? peer->ike_sa_id->clone(peer->ike_sa_id) : NULL;
|
||||||
|
|
||||||
this->mutex->unlock(this->mutex);
|
this->mutex->unlock(this->mutex);
|
||||||
|
|
||||||
@@ -292,7 +292,7 @@ METHOD(mediation_manager_t, check_and_register, ike_sa_id_t*,
|
|||||||
return NULL;
|
return NULL;
|
||||||
}
|
}
|
||||||
|
|
||||||
ike_sa_id = peer->ike_sa_id;
|
ike_sa_id = peer->ike_sa_id->clone(peer->ike_sa_id);
|
||||||
|
|
||||||
this->mutex->unlock(this->mutex);
|
this->mutex->unlock(this->mutex);
|
||||||
|
|
||||||
|
|||||||
@@ -54,9 +54,7 @@ struct mediation_manager_t {
|
|||||||
* Checks if a specific peer is online.
|
* Checks if a specific peer is online.
|
||||||
*
|
*
|
||||||
* @param peer_id the peer's ID
|
* @param peer_id the peer's ID
|
||||||
* @returns
|
* @returns ID of the peer's SA (cloned), NULL if offline
|
||||||
* - IKE_SA ID of the peer's SA.
|
|
||||||
* - NULL, if the peer is not online.
|
|
||||||
*/
|
*/
|
||||||
ike_sa_id_t* (*check) (mediation_manager_t* this,
|
ike_sa_id_t* (*check) (mediation_manager_t* this,
|
||||||
identification_t *peer_id);
|
identification_t *peer_id);
|
||||||
@@ -67,9 +65,7 @@ struct mediation_manager_t {
|
|||||||
*
|
*
|
||||||
* @param peer_id the peer's ID
|
* @param peer_id the peer's ID
|
||||||
* @param requester the requesters ID
|
* @param requester the requesters ID
|
||||||
* @returns
|
* @returns ID of the peer's SA (cloned), NULL if offline
|
||||||
* - IKE_SA ID of the peer's SA.
|
|
||||||
* - NULL, if the peer is not online.
|
|
||||||
*/
|
*/
|
||||||
ike_sa_id_t* (*check_and_register) (mediation_manager_t* this,
|
ike_sa_id_t* (*check_and_register) (mediation_manager_t* this,
|
||||||
identification_t *peer_id,
|
identification_t *peer_id,
|
||||||
|
|||||||
@@ -701,6 +701,7 @@ METHOD(task_t, build_r_ms, status_t,
|
|||||||
chunk_empty);
|
chunk_empty);
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
|
peer_sa->destroy(peer_sa);
|
||||||
|
|
||||||
job_t *job = (job_t*)mediation_job_create(this->peer_id,
|
job_t *job = (job_t*)mediation_job_create(this->peer_id,
|
||||||
this->ike_sa->get_other_id(this->ike_sa), this->connect_id,
|
this->ike_sa->get_other_id(this->ike_sa), this->connect_id,
|
||||||
|
|||||||
Reference in New Issue
Block a user