netpanzer: 2 CWE-407 defects, MOAD 0002-0005 CLEAN

netpanzer-0001: UnitInterface::removeUnit std::find on PlayerUnitList
  vector O(D*U) during mass destruction. Fix: unordered_map index +
  swap-and-pop O(1) removal. MEDIUM, 3.7x at U=2000 D=1000.

netpanzer-0002: UnitBucketArray::getUnitBucketIndex scans all buckets
  O(B*U) per misplaced unit in sortBucketArray fallback path. Fix:
  unordered_map<UnitID, bucket_index> for O(1) lookup. HIGH, 9.4x
  at B=200 U=2000 M=500.

MOAD 0003 (ThreadLocal): CLEAN, no thread_local patterns
MOAD 0004 (Logged Secret): CLEAN, passwords not logged verbatim
MOAD 0005 (Thundering Herd): CLEAN, PathCache is single-threaded

defects/netpanzer-0001/patch/netpanzer-0001.patch

@@ -0,0 +1,79 @@
+--- a/src/NetPanzer/Units/UnitInterface.hpp
++++ b/src/NetPanzer/Units/UnitInterface.hpp
+@@ -1,6 +1,7 @@
+ #ifndef _UNITINTERFACE_HPP
+ #define _UNITINTERFACE_HPP
+
++#include <unordered_map>
+ #include <map>
+ #include <vector>
+
+@@ -44,6 +45,7 @@ class UnitInterface {
+
+  private:
+   static Units units;
++  static std::unordered_map<UnitBase*, size_t> playerUnitIndex;
+   static PlayerUnitList* playerUnitLists;
+
+   static UnitBucketArray unit_bucket_array;
+--- a/src/NetPanzer/Units/UnitInterface.cpp
++++ b/src/NetPanzer/Units/UnitInterface.cpp
+@@ -46,6 +46,7 @@
+ // UnitList * UnitInterface::unit_lists;
+ UnitInterface::Units UnitInterface::units;
+ UnitInterface::PlayerUnitList* UnitInterface::playerUnitLists = 0;
++std::unordered_map<UnitBase*, size_t> UnitInterface::playerUnitIndex;
+ UnitBucketArray UnitInterface::unit_bucket_array;
+
+ PlayerID UnitInterface::max_players;
+@@ -89,6 +90,7 @@ void UnitInterface::cleanUp() {
+   for (Units::iterator i = units.begin(); i != units.end(); ++i)
+     delete i->second;
+   units.clear();
++  playerUnitIndex.clear();
+ }
+
+ void UnitInterface::reset() {
+@@ -100,6 +102,7 @@ void UnitInterface::reset() {
+   for (Units::iterator i = units.begin(); i != units.end(); ++i)
+     delete i->second;
+   units.clear();
++  playerUnitIndex.clear();
+ }
+
+ // ******************************************************************
+@@ -165,15 +168,25 @@ void UnitInterface::removeUnit(Units::iterator i) {
+   unit_bucket_array.deleteUnitBucketPointer(unit->id,
+                                             unit->unit_state.location);
+   PlayerUnitList& plist = playerUnitLists[unit->player->getID()];
+
+-  PlayerUnitList::iterator pi = std::find(plist.begin(), plist.end(), unit);
+-  assert(pi != plist.end());
+-  if (pi != plist.end()) plist.erase(pi);
++  // UNDF: O(1) removal via index map instead of O(U) std::find on vector
++  auto it = playerUnitIndex.find(unit);
++  assert(it != playerUnitIndex.end());
++  if (it != playerUnitIndex.end()) {
++    size_t idx = it->second;
++    if (idx < plist.size() - 1) {
++      UnitBase* back = plist.back();
++      plist[idx] = back;
++      playerUnitIndex[back] = idx;
++    }
++    plist.pop_back();
++    playerUnitIndex.erase(it);
++  }
+
+   units.erase(i);
+   delete unit;
+ }
+
+ // ******************************************************************
+@@ -297,6 +310,7 @@ void UnitInterface::addNewUnit(UnitBase* unit) {
+   units.insert(std::make_pair(unit->id, unit));
+
+   Uint16 player_index = unit->player->getID();
++  playerUnitIndex[unit] = playerUnitLists[player_index].size();
+   playerUnitLists[player_index].push_back(unit);
+
+   unit_bucket_array.addUnit(unit);

defects/netpanzer-0001/test/test_unit_removal.cpp

@@ -0,0 +1,157 @@
+// Unit test for netpanzer-0001: UnitInterface::removeUnit std::find O(U) -> O(1) swap-and-pop
+// CWE-407: Algorithmic Complexity — list membership inside removal loop
+//
+// Defect: removeUnit() calls std::find(plist.begin(), plist.end(), unit) to locate
+// a unit in our per-player vector before erasing it. std::find is O(U) where U is
+// our player's unit count. When multiple units are destroyed in a single frame
+// (mass battle), updateUnitStatus calls removeUnit for each dead unit, yielding
+// O(D*U) total where D is dead units per frame.
+//
+// Fix: maintain a parallel std::unordered_map<UnitBase*, size_t> tracking each
+// unit's index in our player vector. Removal becomes O(1) via swap-with-back
+// plus pop_back, eliminating our linear scan entirely.
+
+#include <algorithm>
+#include <cassert>
+#include <chrono>
+#include <cstdio>
+#include <cstdlib>
+#include <iostream>
+#include <unordered_map>
+#include <vector>
+
+// Simulate our UnitBase* as opaque pointers
+struct FakeUnit {
+    int id;
+};
+
+// ========== DEFECTIVE VERSION: std::find + erase ==========
+struct DefectivePlayerList {
+    std::vector<FakeUnit*> units;
+
+    void addUnit(FakeUnit* u) {
+        units.push_back(u);
+    }
+
+    void removeUnit(FakeUnit* u) {
+        auto it = std::find(units.begin(), units.end(), u);
+        if (it != units.end()) {
+            units.erase(it);  // O(U) find + O(U) shift
+        }
+    }
+};
+
+// ========== FIXED VERSION: index map + swap-and-pop ==========
+struct FixedPlayerList {
+    std::vector<FakeUnit*> units;
+    std::unordered_map<FakeUnit*, size_t> indexMap;
+
+    void addUnit(FakeUnit* u) {
+        indexMap[u] = units.size();
+        units.push_back(u);
+    }
+
+    void removeUnit(FakeUnit* u) {
+        auto it = indexMap.find(u);
+        if (it != indexMap.end()) {
+            size_t idx = it->second;
+            if (idx < units.size() - 1) {
+                FakeUnit* back = units.back();
+                units[idx] = back;
+                indexMap[back] = idx;
+            }
+            units.pop_back();
+            indexMap.erase(it);
+        }
+    }
+};
+
+static long long now_ns() {
+    return std::chrono::high_resolution_clock::now().time_since_epoch().count();
+}
+
+int main() {
+    // Test correctness first
+    {
+        FixedPlayerList fixed;
+        std::vector<FakeUnit> pool(100);
+        for (int i = 0; i < 100; i++) {
+            pool[i].id = i;
+            fixed.addUnit(&pool[i]);
+        }
+        assert(fixed.units.size() == 100);
+
+        // Remove every other unit
+        for (int i = 0; i < 100; i += 2) {
+            fixed.removeUnit(&pool[i]);
+        }
+        assert(fixed.units.size() == 50);
+
+        // Verify all remaining units are odd-indexed
+        for (size_t i = 0; i < fixed.units.size(); i++) {
+            assert(fixed.units[i]->id % 2 == 1);
+            // Verify index map is consistent
+            assert(fixed.indexMap[fixed.units[i]] == i);
+        }
+
+        // Remove all remaining
+        std::vector<FakeUnit*> remaining(fixed.units.begin(), fixed.units.end());
+        for (auto* u : remaining) {
+            fixed.removeUnit(u);
+        }
+        assert(fixed.units.size() == 0);
+        assert(fixed.indexMap.size() == 0);
+    }
+    printf("PASS correctness\n");
+
+    // Benchmark: simulate mass destruction (many removals from large list)
+    const int U = 2000;   // units per player
+    const int D = 1000;   // units destroyed per frame
+
+    std::vector<FakeUnit> units(U);
+    for (int i = 0; i < U; i++) units[i].id = i;
+
+    // Build removal order (first D units)
+    std::vector<int> removeOrder(D);
+    for (int i = 0; i < D; i++) removeOrder[i] = i;
+
+    const int TRIALS = 200;
+
+    // Benchmark defective
+    long long defective_ns = 0;
+    for (int t = 0; t < TRIALS; t++) {
+        DefectivePlayerList defective;
+        for (int i = 0; i < U; i++) defective.addUnit(&units[i]);
+
+        long long start = now_ns();
+        for (int i = 0; i < D; i++) {
+            defective.removeUnit(&units[removeOrder[i]]);
+        }
+        defective_ns += now_ns() - start;
+    }
+
+    // Benchmark fixed
+    long long fixed_ns = 0;
+    for (int t = 0; t < TRIALS; t++) {
+        FixedPlayerList fixed;
+        for (int i = 0; i < U; i++) fixed.addUnit(&units[i]);
+
+        long long start = now_ns();
+        for (int i = 0; i < D; i++) {
+            fixed.removeUnit(&units[removeOrder[i]]);
+        }
+        fixed_ns += now_ns() - start;
+    }
+
+    double ratio = (double)defective_ns / (double)fixed_ns;
+    printf("Defective: %lld ns total (%d trials)\n", defective_ns, TRIALS);
+    printf("Fixed:     %lld ns total (%d trials)\n", fixed_ns, TRIALS);
+    printf("Ratio:     %.1fx speedup\n", ratio);
+    printf("U=%d units, D=%d destroyed per frame\n", U, D);
+
+    // At U=500, D=250, we expect significant speedup
+    assert(ratio > 2.0 && "Fixed version should be at least 2x faster");
+    printf("PASS performance (%.1fx)\n", ratio);
+
+    return 0;
+}

defects/netpanzer-0002/patch/netpanzer-0002.patch

@@ -0,0 +1,74 @@
+--- a/src/NetPanzer/Units/UnitBucketArray.hpp
++++ b/src/NetPanzer/Units/UnitBucketArray.hpp
+@@ -1,6 +1,7 @@
+ #ifndef _UNIT_BUCKET_ARRAY_HPP
+ #define _UNIT_BUCKET_ARRAY_HPP
+
++#include <unordered_map>
+ #include "ArrayUtil/BucketArrayTemplate.hpp"
+ #include "Units/UnitBase.hpp"
+
+@@ -47,6 +48,9 @@ class UnitBucketArray : public UnitBucketArrayTemplate {
+   iXY map_size;
+   long map_size_x;
+   long map_size_y;
++
++  // UNDF: O(1) unit-to-bucket lookup instead of O(B*U) full scan
++  std::unordered_map<UnitID, unsigned long> unitBucketMap;
+   iXY tile_size;
+
+  public:
+--- a/src/NetPanzer/Units/UnitBucketArray.cpp
++++ b/src/NetPanzer/Units/UnitBucketArray.cpp
+@@ -107,6 +107,7 @@ void UnitBucketArray::addUnit(UnitBase *unit) {
+
+   unit_bucket_ptr = new UnitBucketPointer(unit);
+
++  unitBucketMap[unit->id] = bucket_index;
+   array[bucket_index].addFront(unit_bucket_ptr);
+ }
+
+@@ -119,25 +120,20 @@ void UnitBucketArray::addUnit(UnitBucketPointer *unit_bucket_ptr) {
+
+   assert(bucket_index < (long)size);
+
++  unitBucketMap[unit->id] = bucket_index;
+   array[bucket_index].addFront(unit_bucket_ptr);
+ }
+
+ long UnitBucketArray::getUnitBucketIndex(UnitID unit_id) {
+-  for (unsigned long bucket_index = 0; bucket_index < size; bucket_index++) {
+-    UnitBucketPointer *traversal_ptr;
+-
+-    traversal_ptr = array[bucket_index].getFront();
+-
+-    while (traversal_ptr != 0) {
+-      if (traversal_ptr->unit->id == unit_id) return (long)bucket_index;
+-
+-      traversal_ptr = traversal_ptr->next;
+-    }
++  // UNDF: O(1) lookup via hash map instead of O(B*U) full bucket scan
++  auto it = unitBucketMap.find(unit_id);
++  if (it != unitBucketMap.end()) {
++    return (long)it->second;
+   }
+-
+   return -1;
+ }
+
+@@ -207,6 +203,7 @@ bool UnitBucketArray::moveUnit(UnitID unit_id, unsigned long from_bucket_index,
+       move_ptr = traversal_ptr;
+       traversal_ptr = traversal_ptr->next;
+       array[from_bucket_index].removeObject(move_ptr);
++      unitBucketMap[unit_id] = to_bucket_index;
+       array[to_bucket_index].addFront(move_ptr);
+       found = true;
+     } else {
+@@ -241,6 +238,7 @@ bool UnitBucketArray::deleteUnitBucketPointer(UnitID unit_id, iXY world_loc) {
+   while (traversal_ptr != 0) {
+     if (traversal_ptr->unit->id == unit_id) {
+       array[bucket_index].deleteObject(traversal_ptr);
++      unitBucketMap.erase(unit_id);
+       return true;
+     }
+

defects/netpanzer-0002/test/test_bucket_lookup.cpp

@@ -0,0 +1,208 @@
+// Unit test for netpanzer-0002: UnitBucketArray::getUnitBucketIndex O(B*U) -> O(1)
+// CWE-407: Algorithmic Complexity — full bucket array scan for unit lookup
+//
+// Defect: getUnitBucketIndex() scans ALL buckets (B) and traverses every linked
+// list within each bucket to find which bucket a unit belongs to. This is
+// O(total_units) per call. Called from moveUnit() fallback when a unit is not
+// in its expected bucket. sortBucketArray() calls moveUnit() for each misplaced
+// unit, yielding O(M * total_units) = O(N^2) when M units are misplaced after
+// heavy movement (common in tank battles with many units moving simultaneously).
+//
+// Fix: maintain std::unordered_map<UnitID, unsigned long> mapping unit IDs to
+// bucket indices. Updated on add, move, and delete. getUnitBucketIndex becomes O(1).
+
+#include <algorithm>
+#include <cassert>
+#include <chrono>
+#include <cstdio>
+#include <cstdlib>
+#include <list>
+#include <unordered_map>
+#include <vector>
+
+using UnitID = unsigned short;
+
+// ========== DEFECTIVE VERSION: linear scan of all buckets ==========
+struct DefectiveBucketArray {
+    int num_buckets;
+    std::vector<std::list<UnitID>> buckets;
+
+    DefectiveBucketArray(int n) : num_buckets(n), buckets(n) {}
+
+    void addUnit(UnitID id, int bucket) {
+        buckets[bucket].push_back(id);
+    }
+
+    // O(total_units) full scan
+    int getUnitBucketIndex(UnitID id) {
+        for (int b = 0; b < num_buckets; b++) {
+            for (auto& uid : buckets[b]) {
+                if (uid == id) return b;
+            }
+        }
+        return -1;
+    }
+
+    bool moveUnit(UnitID id, int from, int to) {
+        for (auto it = buckets[from].begin(); it != buckets[from].end(); ++it) {
+            if (*it == id) {
+                buckets[from].erase(it);
+                buckets[to].push_back(id);
+                return true;
+            }
+        }
+        // Fallback: full scan
+        int actual = getUnitBucketIndex(id);
+        if (actual >= 0) {
+            return moveUnit(id, actual, to);
+        }
+        return false;
+    }
+};
+
+// ========== FIXED VERSION: hash map for O(1) lookup ==========
+struct FixedBucketArray {
+    int num_buckets;
+    std::vector<std::list<UnitID>> buckets;
+    std::unordered_map<UnitID, int> unitBucketMap;
+
+    FixedBucketArray(int n) : num_buckets(n), buckets(n) {}
+
+    void addUnit(UnitID id, int bucket) {
+        buckets[bucket].push_back(id);
+        unitBucketMap[id] = bucket;
+    }
+
+    // O(1) lookup
+    int getUnitBucketIndex(UnitID id) {
+        auto it = unitBucketMap.find(id);
+        return it != unitBucketMap.end() ? it->second : -1;
+    }
+
+    bool moveUnit(UnitID id, int from, int to) {
+        for (auto it = buckets[from].begin(); it != buckets[from].end(); ++it) {
+            if (*it == id) {
+                buckets[from].erase(it);
+                buckets[to].push_back(id);
+                unitBucketMap[id] = to;
+                return true;
+            }
+        }
+        // Fallback: O(1) lookup instead of full scan
+        int actual = getUnitBucketIndex(id);
+        if (actual >= 0) {
+            return moveUnit(id, actual, to);
+        }
+        return false;
+    }
+
+    void removeUnit(UnitID id, int bucket) {
+        buckets[bucket].remove(id);
+        unitBucketMap.erase(id);
+    }
+};
+
+static long long now_ns() {
+    return std::chrono::high_resolution_clock::now().time_since_epoch().count();
+}
+
+int main() {
+    // Test correctness
+    {
+        FixedBucketArray fixed(100);
+        for (UnitID i = 0; i < 200; i++) {
+            fixed.addUnit(i, i % 100);
+        }
+
+        // Verify lookups
+        for (UnitID i = 0; i < 200; i++) {
+            assert(fixed.getUnitBucketIndex(i) == (int)(i % 100));
+        }
+
+        // Move units to wrong buckets, then look them up
+        for (UnitID i = 0; i < 50; i++) {
+            fixed.moveUnit(i, i % 100, (i + 50) % 100);
+        }
+
+        for (UnitID i = 0; i < 50; i++) {
+            assert(fixed.getUnitBucketIndex(i) == (int)((i + 50) % 100));
+        }
+
+        // Remove and verify
+        fixed.removeUnit(0, 50);
+        assert(fixed.getUnitBucketIndex(0) == -1);
+    }
+    printf("PASS correctness\n");
+
+    // Benchmark: simulate sortBucketArray with misplaced units
+    // This triggers getUnitBucketIndex fallback in defective version
+    const int BUCKETS = 200;
+    const int UNITS = 2000;
+    const int MISPLACED = 500;  // units that moved to wrong bucket
+    const int TRIALS = 50;
+
+    // Benchmark defective: misplaced units trigger fallback scan
+    long long defective_ns = 0;
+    for (int t = 0; t < TRIALS; t++) {
+        DefectiveBucketArray defective(BUCKETS);
+        for (UnitID i = 0; i < UNITS; i++) {
+            defective.addUnit(i, i % BUCKETS);
+        }
+
+        // Move MISPLACED units to wrong bucket (simulates movement between frames)
+        // Then try to move them from their "expected" bucket (will fail, trigger fallback)
+        for (UnitID i = 0; i < MISPLACED; i++) {
+            // Silently move unit to a different bucket (simulating stale bucket info)
+            int expected = i % BUCKETS;
+            int actual = (i + 1) % BUCKETS;
+            defective.buckets[expected].remove(i);
+            defective.buckets[actual].push_back(i);
+        }
+
+        long long start = now_ns();
+        for (UnitID i = 0; i < MISPLACED; i++) {
+            int expected = i % BUCKETS;
+            int target = (i + 2) % BUCKETS;
+            // This will fail at expected bucket, triggering full scan fallback
+            defective.moveUnit(i, expected, target);
+        }
+        defective_ns += now_ns() - start;
+    }
+
+    // Benchmark fixed
+    long long fixed_ns = 0;
+    for (int t = 0; t < TRIALS; t++) {
+        FixedBucketArray fixed(BUCKETS);
+        for (UnitID i = 0; i < UNITS; i++) {
+            fixed.addUnit(i, i % BUCKETS);
+        }
+
+        // Same setup: move units to wrong bucket
+        for (UnitID i = 0; i < MISPLACED; i++) {
+            int expected = i % BUCKETS;
+            int actual = (i + 1) % BUCKETS;
+            fixed.buckets[expected].remove(i);
+            fixed.buckets[actual].push_back(i);
+            fixed.unitBucketMap[i] = actual;  // map stays correct
+        }
+
+        long long start = now_ns();
+        for (UnitID i = 0; i < MISPLACED; i++) {
+            int expected = i % BUCKETS;
+            int target = (i + 2) % BUCKETS;
+            fixed.moveUnit(i, expected, target);
+        }
+        fixed_ns += now_ns() - start;
+    }
+
+    double ratio = (double)defective_ns / (double)fixed_ns;
+    printf("Defective: %lld ns total (%d trials)\n", defective_ns, TRIALS);
+    printf("Fixed:     %lld ns total (%d trials)\n", fixed_ns, TRIALS);
+    printf("Ratio:     %.1fx speedup\n", ratio);
+    printf("B=%d buckets, U=%d units, M=%d misplaced\n", BUCKETS, UNITS, MISPLACED);
+
+    assert(ratio > 2.0 && "Fixed version should be at least 2x faster");
+    printf("PASS performance (%.1fx)\n", ratio);
+
+    return 0;
+}