supertuxkart-0001: item_manager randomItemsForArena O(N^2) invalid_location scan

CWE-407 defect in src/items/item_manager.cpp:635. std::find on
std::vector<int> invalid_location inside retry loop inside placement
loop. Fix: std::unordered_set<int> for O(1) membership. 13.8x at N=500.

MOAD-0002 through MOAD-0005 scanned CLEAN:
- MOAD-0002 (Intertangle): standard game engine singleton pattern
- MOAD-0003 (Leaked Context): thread_local g_process_type is process-scoped
- MOAD-0004 (Logged Secret): HTTP request logging already has credential denylist
- MOAD-0005 (Thundering Herd): no shared cache patterns in network code
This commit is contained in:
russell@unturf.com 2026-03-31 10:09:46 -04:00
parent 77d70fdc46
commit a7907f0e12
3 changed files with 162 additions and 1 deletions

View file

@ -944,5 +944,6 @@
"monogame-0003-0003": "UNDF-2026-000000943",
"supertuxkart-0001-0001": "UNDF-2026-000000944",
"supertuxkart-0002-0002": "UNDF-2026-000000945",
"minetest-0004-0004": "UNDF-2026-000000946"
"minetest-0004-0004": "UNDF-2026-000000946",
"minetest-0005-0005": "UNDF-2026-000000947"
}

View file

@ -0,0 +1,63 @@
# UNDF: UNDF-2026-000000944
--- a/src/items/item_manager.cpp
+++ b/src/items/item_manager.cpp
@@ -593,8 +593,8 @@ bool ItemManager::randomItemsForArena(const AlignedArray<btTransform>& pos)
const ArenaGraph* ag = ArenaGraph::get();
std::vector<int> used_location;
- std::vector<int> invalid_location;
+ std::unordered_set<int> invalid_location;
for (unsigned int i = 0; i < pos.size(); i++)
{
// Load all starting positions of arena, so no items will be near them
@@ -602,7 +602,7 @@ bool ItemManager::randomItemsForArena(const AlignedArray<btTransform>& pos)
ag->findRoadSector(pos[i].getOrigin(), &node, NULL, true);
assert(node != -1);
used_location.push_back(node);
- invalid_location.push_back(node);
+ invalid_location.insert(node);
}
const unsigned int ALL_NODES = ag->getNumNodes();
@@ -620,7 +620,6 @@ bool ItemManager::randomItemsForArena(const AlignedArray<btTransform>& pos)
while(true)
{
- if (used_location.size() - pos.size() +
- invalid_location.size() == ALL_NODES)
+ if (used_location.size() - pos.size() + invalid_location.size() == ALL_NODES)
{
Log::warn("[ItemManager]","Can't place more random items! "
"Use default item location.");
@@ -632,9 +631,7 @@ bool ItemManager::randomItemsForArena(const AlignedArray<btTransform>& pos)
const int node = number % ALL_NODES;
// Check if tried
- std::vector<int>::iterator it = std::find(invalid_location.begin(),
- invalid_location.end(), node);
- if (it != invalid_location.end())
+ if (invalid_location.count(node))
continue;
// Check if near edge
if (ag->getNode(node)->isNearEdge())
{
- invalid_location.push_back(node);
+ invalid_location.insert(node);
continue;
}
// Check if too close
@@ -654,12 +651,12 @@ bool ItemManager::randomItemsForArena(const AlignedArray<btTransform>& pos)
if (found)
{
chosen_node = node;
- invalid_location.push_back(node);
+ invalid_location.insert(node);
random_numbers.push_back(number);
break;
}
else
- invalid_location.push_back(node);
+ invalid_location.insert(node);
}
assert(chosen_node != -1);

View file

@ -0,0 +1,97 @@
// Unit test for supertuxkart-0001: item_manager.cpp randomItemsForArena
// invalid_location membership check O(N) per retry -> O(1) with unordered_set
//
// Defect: std::find on std::vector<int> invalid_location inside retry loop
// inside placement loop. O(TOTAL_ITEM * retries * I) where I grows
// toward ALL_NODES.
//
// Fix: Replace std::vector<int> invalid_location with std::unordered_set<int>.
// Membership check becomes O(1) amortized.
#include <algorithm>
#include <cassert>
#include <chrono>
#include <cstdio>
#include <unordered_set>
#include <vector>
// Simulate the defective pattern: growing invalid_location with linear scans
// In the worst case, the arena is dense and many nodes get marked invalid.
// We simulate placing items when most nodes are already invalid.
long long benchmark_vector(int invalid_size, int lookups)
{
std::vector<int> invalid_location;
for (int i = 0; i < invalid_size; i++)
invalid_location.push_back(i * 2); // even numbers are invalid
auto start = std::chrono::high_resolution_clock::now();
volatile int found_count = 0;
for (int q = 0; q < lookups; q++)
{
int node = q * 3; // mix of hits and misses
auto it = std::find(invalid_location.begin(),
invalid_location.end(), node);
if (it != invalid_location.end())
found_count++;
}
auto end = std::chrono::high_resolution_clock::now();
return std::chrono::duration_cast<std::chrono::nanoseconds>(end - start).count();
}
// Simulate the fixed pattern
long long benchmark_set(int invalid_size, int lookups)
{
std::unordered_set<int> invalid_location;
for (int i = 0; i < invalid_size; i++)
invalid_location.insert(i * 2);
auto start = std::chrono::high_resolution_clock::now();
volatile int found_count = 0;
for (int q = 0; q < lookups; q++)
{
int node = q * 3;
if (invalid_location.count(node))
found_count++;
}
auto end = std::chrono::high_resolution_clock::now();
return std::chrono::duration_cast<std::chrono::nanoseconds>(end - start).count();
}
int main()
{
// Arena graph sizes: small arenas ~100 nodes, large arenas ~1000 nodes.
// When most nodes become invalid, the vector grows to hundreds of entries.
const int INVALID_SIZE = 500; // half of a 1000-node arena
const int LOOKUPS = 500; // retries across all item placements
// Warm up
benchmark_vector(INVALID_SIZE, LOOKUPS);
benchmark_set(INVALID_SIZE, LOOKUPS);
const int ROUNDS = 2000;
long long vec_total = 0, set_total = 0;
for (int i = 0; i < ROUNDS; i++)
{
vec_total += benchmark_vector(INVALID_SIZE, LOOKUPS);
set_total += benchmark_set(INVALID_SIZE, LOOKUPS);
}
double vec_avg = (double)vec_total / ROUNDS;
double set_avg = (double)set_total / ROUNDS;
double ratio = vec_avg / set_avg;
printf("supertuxkart-0001: randomItemsForArena invalid_location membership\n");
printf(" INVALID_SIZE=%d, LOOKUPS=%d\n", INVALID_SIZE, LOOKUPS);
printf(" vector std::find : %.0f ns avg\n", vec_avg);
printf(" unordered_set : %.0f ns avg\n", set_avg);
printf(" ratio : %.1fx\n", ratio);
printf(" %s\n", ratio > 1.5 ? "PASS" : "FAIL");
assert(ratio > 1.5);
return 0;
}