Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix read after free on security code when remote participant is gone before an endpoint is matched [13035] #2328

Merged
merged 2 commits into from
Nov 26, 2021

Conversation

richiware
Copy link
Member

When a remote writer is discovered (SecurityManager::discovered_writer), the cryptohandle for this remote writer is
stored in local_reader->second.associated_writers.
But the writer is not matched with the local reader after the key exchange process finish.
If meantime the participant is unregistered, all crypthandle of its remote endpoints are removed.
But the local_reader still maintains a pointer, because this pointer is only removed when the reader unmatched with the
remote writer.
Then, when the local participant is deleted, SecurityManager access to this invalid reference.

This PR fixes this.

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
@richiware richiware changed the title Fix invalid memory access in security when remote participant gones before a endpoint matched Fix invalid memory access in security when remote participant gones before a endpoint matched [13035] Nov 25, 2021
@MiguelCompany MiguelCompany changed the title Fix invalid memory access in security when remote participant gones before a endpoint matched [13035] Fix read after free on security code when remote participant is gone before an endpoint is matched [13035] Nov 26, 2021
Copy link
Member

@MiguelCompany MiguelCompany left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@MiguelCompany
Copy link
Member

Unrelated failures

@MiguelCompany MiguelCompany merged commit c5596d3 into master Nov 26, 2021
@MiguelCompany MiguelCompany deleted the bugfix/13032 branch November 26, 2021 11:57
@MiguelCompany
Copy link
Member

@Mergifyio backport 2.3.x

mergify bot pushed a commit that referenced this pull request Dec 4, 2021
* Refs #13032. Fix error.
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Refs #13032. Remove security tests from xfail

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
(cherry picked from commit c5596d3)
@mergify
Copy link
Contributor

mergify bot commented Dec 4, 2021

backport 2.3.x

✅ Backports have been created

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants