From 5ddb8caa97cf2f13eeaec3b26e97df2c33d4494d Mon Sep 17 00:00:00 2001
From: Mick Sayson <mick@sayson.com>
Date: Sun, 10 Oct 2021 15:12:36 -0700
Subject: [PATCH] fix(toxext): Protect use of toxext modifying functions

When running with -DASAN on and a mod to send 10k messages in quick
succession I was seeing memory corruption within toxext. This was caused
by a race between toxext_iterate and toxext_send being called from
different threads.

Protect the use of functions coming from different threads with a mutex
---
 src/core/coreext.cpp | 13 +++++++++++--
 src/core/coreext.h   |  6 ++++++
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/src/core/coreext.cpp b/src/core/coreext.cpp
index 375093002..a97fbe4c1 100644
--- a/src/core/coreext.cpp
+++ b/src/core/coreext.cpp
@@ -59,12 +59,14 @@ CoreExt::CoreExt(ExtensionPtr<ToxExt> toxExt_)
 
 void CoreExt::process()
 {
+    std::lock_guard<std::mutex> lock(toxext_mutex);
     toxext_iterate(toxExt.get());
 }
 
 void CoreExt::onLosslessPacket(uint32_t friendId, const uint8_t* data, size_t length)
 {
     if (is_toxext_packet(data, length)) {
+        std::lock_guard<std::mutex> lock(toxext_mutex);
         toxext_handle_lossless_custom_packet(toxExt.get(), friendId, data, length);
     }
 }
@@ -73,11 +75,15 @@ CoreExt::Packet::Packet(
     ToxExtPacketList* packetList,
     ToxExtensionMessages* toxExtMessages,
     uint32_t friendId,
+    std::mutex* toxext_mutex,
     PacketPassKey)
-    : toxExtMessages(toxExtMessages)
+    : toxext_mutex(toxext_mutex)
+    , toxExtMessages(toxExtMessages)
     , packetList(packetList)
     , friendId(friendId)
-{}
+{
+    assert(toxext_mutex != nullptr);
+}
 
 std::unique_ptr<ICoreExtPacket> CoreExt::getPacket(uint32_t friendId)
 {
@@ -85,6 +91,7 @@ std::unique_ptr<ICoreExtPacket> CoreExt::getPacket(uint32_t friendId)
         toxext_packet_list_create(toxExt.get(), friendId),
         toxExtMessages.get(),
         friendId,
+        &toxext_mutex,
         PacketPassKey{}));
 }
 
@@ -130,6 +137,8 @@ uint64_t CoreExt::Packet::addExtendedMessage(QString message)
 
 bool CoreExt::Packet::send()
 {
+    std::lock_guard<std::mutex> lock(*toxext_mutex);
+
     auto ret = toxext_send(packetList);
     if (ret != TOXEXT_SUCCESS) {
         qWarning() << "Failed to send packet";
diff --git a/src/core/coreext.h b/src/core/coreext.h
index f47553887..141427ad7 100644
--- a/src/core/coreext.h
+++ b/src/core/coreext.h
@@ -27,6 +27,7 @@
 
 #include <bitset>
 #include <memory>
+#include <mutex>
 #include <unordered_map>
 
 struct Tox;
@@ -86,6 +87,7 @@ public:
             ToxExtPacketList* packetList,
             ToxExtensionMessages* toxExtMessages,
             uint32_t friendId,
+            std::mutex* toxext_mutex,
             PacketPassKey);
 
         // Delete copy constructor, we shouldn't be able to copy
@@ -97,16 +99,19 @@ public:
             packetList = other.packetList;
             friendId = other.friendId;
             hasBeenSent = other.hasBeenSent;
+            toxext_mutex = other.toxext_mutex;
             other.toxExtMessages = nullptr;
             other.packetList = nullptr;
             other.friendId = 0;
             other.hasBeenSent = false;
+            other.toxext_mutex = nullptr;
         }
 
         uint64_t addExtendedMessage(QString message) override;
 
         bool send() override;
     private:
+        std::mutex* toxext_mutex;
         bool hasBeenSent = false;
         // Note: non-owning pointer
         ToxExtensionMessages* toxExtMessages;
@@ -141,6 +146,7 @@ private:
 
     CoreExt(ExtensionPtr<ToxExt> toxExt);
 
+    std::mutex toxext_mutex;
     std::unordered_map<uint32_t, Status::Status> currentStatuses;
     ExtensionPtr<ToxExt> toxExt;
     ExtensionPtr<ToxExtensionMessages> toxExtMessages;