equalitie / ouisync

A secure peer-to-peer file synchronization app.
https://ouisync.net
Mozilla Public License 2.0
49 stars 9 forks source link

Memory leak: MessageBrokers not removed from network/mod.rs/State #150

Closed inetic closed 10 months ago

inetic commented 10 months ago

The State structure contains message_brokers: Option<HashMap<PublicRuntimeId, MessageBroker>> hash map.

Each MessageBroker should contain at least one connection to the peer with the given PublicRuntimeId, but currently we don't have a code which would remove the message broker from the hash map once all connections are closed.

This could be exploited by an adversary who would keep connecting to and disconnection from user's replica always with a different PublicRuntimeId eventually filling up the memory.

madadam commented 10 months ago

but currently we don't have a code which would remove the message broker from the hash map once all connections are closed.

This is not correct. We do remove the message broker when its last connection closes: https://github.com/equalitie/ouisync/blob/master/lib/src/network/mod.rs#L826

inetic commented 10 months ago

Hm, I missed that. You're right.