qwbarch / mirage

A mod that mimics player voices for Unity games
https://thunderstore.io/c/lethal-company/p/qwbarch/Mirage
Other
12 stars 2 forks source link

Security issue during config syncing #44

Closed Owen3H closed 6 months ago

Owen3H commented 6 months ago

Hi, just letting you know that your config syncing implementation relies on the outdated BinaryFormatter which is insecure. Essentially, users of your mod are prone to RCE when deserializing the config.

While it likely won't matter in most cases, it isn't worth taking the risk when DataContractSerializer exists (or just use CSync). It is also worth mentioning that Mirage is at risk of being suspended from Thunderstore because of this.

exverge-0 commented 6 months ago

This actually has a much bigger implication, since from what I can tell it is using BinaryFormatter to deserialize the config sent by the host, which could be used maliciously, such as in public lobbies.

edit: Just realized I mostly rephrased what was originally said, sorry

qwbarch commented 6 months ago

Does the wiki contain a proper implementation now? I'll rewrite it using whatever is on there.

While I appreciate that csync exists (thanks for creating it), I already have way too many dependencies so I'm just avoiding pulling yet another one

Owen3H commented 6 months ago

It has become a lot more stable now and its usage has been simplified with v2.0.0 I understand dependencies are hassle and you are welcome to integrate it into Mirage as long as you provide some credit in acknowledgements :)

qwbarch commented 6 months ago

Fixed in 722909661f5cc14b710310e3bb4541cfaa5d7b23, thanks for bringing up the issue with BinaryFormatter to my attention!
I've also included you in the acknowledgements section of the README: https://github.com/qwbarch/lc-mirage/blob/4359a4a61ca8860a31f6d8f87b4f2bd16eb0d480/README.md?plain=1#L86