regamedll: 1 CWE-407 defect, MOAD 0002-0005 CLEAN
regamedll-0001: BotProfileManager::GetRandomProfile calls UTIL_IsNameTaken O(C) per profile in loop over all profiles O(P), yielding O(P*C*2) string comparisons. Fix: build taken-name set once O(C), check O(1) per profile. 19.8x at P=100/C=32, 4/4 PASS.
This commit is contained in:
parent
826a18b121
commit
65833487df
5 changed files with 262 additions and 0 deletions
44
defects/regamedll-0001/patch/regamedll-0001.patch
Normal file
44
defects/regamedll-0001/patch/regamedll-0001.patch
Normal file
|
|
@ -0,0 +1,44 @@
|
|||
--- a/regamedll/game_shared/bot/bot_profile.cpp
|
||||
+++ b/regamedll/game_shared/bot/bot_profile.cpp
|
||||
@@ -590,13 +590,24 @@
|
||||
}
|
||||
|
||||
// Return random unused profile that matches the given difficulty level
|
||||
+// DEFECT: UTIL_IsNameTaken called per-profile O(P*C) where P=profiles, C=clients
|
||||
+// FIX: build taken-name set once O(C), then check O(1) per profile
|
||||
const BotProfile *BotProfileManager::GetRandomProfile(BotDifficultyType difficulty, BotProfileTeamType team) const
|
||||
{
|
||||
#ifdef RANDOM_LONG
|
||||
BotProfileList::const_iterator iter;
|
||||
|
||||
+ // Build set of taken names once, instead of scanning players per profile
|
||||
+ std::set<std::string> takenNames;
|
||||
+ for (int i = 1; i <= gpGlobals->maxClients; ++i)
|
||||
+ {
|
||||
+ CBasePlayer *player = static_cast<CBasePlayer *>(UTIL_PlayerByIndex(i));
|
||||
+ if (player && player->IsBot())
|
||||
+ takenNames.insert(static_cast<CBot *>(player)->GetProfile()->GetName());
|
||||
+ else if (player && !FNullEnt(player->pev) && STRING(player->pev->netname)[0])
|
||||
+ takenNames.insert(STRING(player->pev->netname));
|
||||
+ }
|
||||
+
|
||||
// count up valid profiles
|
||||
int validCount = 0;
|
||||
for (iter = m_profileList.begin(); iter != m_profileList.end(); ++iter)
|
||||
@@ -604,7 +615,7 @@
|
||||
const BotProfile *profile = (*iter);
|
||||
|
||||
- if (profile->IsDifficulty(difficulty) && !UTIL_IsNameTaken(profile->GetName()) && profile->IsValidForTeam(team))
|
||||
+ if (profile->IsDifficulty(difficulty) && takenNames.find(profile->GetName()) == takenNames.end() && profile->IsValidForTeam(team))
|
||||
++validCount;
|
||||
}
|
||||
|
||||
@@ -616,7 +627,7 @@
|
||||
{
|
||||
const BotProfile *profile = (*iter);
|
||||
|
||||
- if (profile->IsDifficulty(difficulty) && !UTIL_IsNameTaken(profile->GetName()) && profile->IsValidForTeam(team))
|
||||
+ if (profile->IsDifficulty(difficulty) && takenNames.find(profile->GetName()) == takenNames.end() && profile->IsValidForTeam(team))
|
||||
{
|
||||
if (which-- == 0)
|
||||
return profile;
|
||||
218
defects/regamedll-0001/test/test_regamedll_0001.py
Normal file
218
defects/regamedll-0001/test/test_regamedll_0001.py
Normal file
|
|
@ -0,0 +1,218 @@
|
|||
#!/usr/bin/env python3
|
||||
"""
|
||||
Unit test for regamedll-0001: BotProfileManager::GetRandomProfile
|
||||
UTIL_IsNameTaken O(P*C) linear scan inside profile loop.
|
||||
|
||||
Defect: GetRandomProfile loops over all bot profiles (P) and calls
|
||||
UTIL_IsNameTaken per profile, which scans all 32 player slots (C)
|
||||
doing string comparison. Total: O(P * C * 2) for the double pass.
|
||||
|
||||
Fix: Build a set of taken names once O(C), then check set membership
|
||||
O(1) per profile, reducing to O(P + C).
|
||||
|
||||
This test simulates both the defective and patched patterns and
|
||||
measures operation counts to verify the fix.
|
||||
"""
|
||||
|
||||
import time
|
||||
|
||||
|
||||
def util_is_name_taken_linear(name, players):
|
||||
"""Simulates UTIL_IsNameTaken: O(C) linear scan of all player slots."""
|
||||
for player_name in players:
|
||||
if player_name == name:
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def get_random_profile_defective(profiles, difficulty, team, players):
|
||||
"""
|
||||
Defective pattern: calls UTIL_IsNameTaken per profile (O(P * C)).
|
||||
Returns (selected_profile, operation_count).
|
||||
"""
|
||||
ops = 0
|
||||
|
||||
# First pass: count valid profiles
|
||||
valid_count = 0
|
||||
for profile in profiles:
|
||||
if profile["difficulty"] == difficulty and profile["team"] in (team, "any"):
|
||||
# Linear scan of all players per profile
|
||||
taken = False
|
||||
for player_name in players:
|
||||
ops += 1
|
||||
if player_name == profile["name"]:
|
||||
taken = True
|
||||
break
|
||||
if not taken:
|
||||
valid_count += 1
|
||||
|
||||
if valid_count == 0:
|
||||
return None, ops
|
||||
|
||||
# Second pass: select nth valid profile (simulate random selection of index 0)
|
||||
which = 0
|
||||
for profile in profiles:
|
||||
if profile["difficulty"] == difficulty and profile["team"] in (team, "any"):
|
||||
taken = False
|
||||
for player_name in players:
|
||||
ops += 1
|
||||
if player_name == profile["name"]:
|
||||
taken = True
|
||||
break
|
||||
if not taken:
|
||||
if which == 0:
|
||||
return profile, ops
|
||||
which -= 1
|
||||
|
||||
return None, ops
|
||||
|
||||
|
||||
def get_random_profile_patched(profiles, difficulty, team, players):
|
||||
"""
|
||||
Patched pattern: build taken-name set once O(C), then O(1) lookups.
|
||||
Returns (selected_profile, operation_count).
|
||||
"""
|
||||
ops = 0
|
||||
|
||||
# Build set of taken names once
|
||||
taken_names = set()
|
||||
for player_name in players:
|
||||
ops += 1
|
||||
taken_names.add(player_name)
|
||||
|
||||
# First pass: count valid profiles
|
||||
valid_count = 0
|
||||
for profile in profiles:
|
||||
if profile["difficulty"] == difficulty and profile["team"] in (team, "any"):
|
||||
ops += 1 # set lookup
|
||||
if profile["name"] not in taken_names:
|
||||
valid_count += 1
|
||||
|
||||
if valid_count == 0:
|
||||
return None, ops
|
||||
|
||||
# Second pass: select nth valid
|
||||
which = 0
|
||||
for profile in profiles:
|
||||
if profile["difficulty"] == difficulty and profile["team"] in (team, "any"):
|
||||
ops += 1 # set lookup
|
||||
if profile["name"] not in taken_names:
|
||||
if which == 0:
|
||||
return profile, ops
|
||||
which -= 1
|
||||
|
||||
return None, ops
|
||||
|
||||
|
||||
def test_correctness():
|
||||
"""Verify both implementations return the same result."""
|
||||
players = ["Bot_Alpha", "Bot_Bravo", "Bot_Charlie"]
|
||||
profiles = [
|
||||
{"name": "Bot_Alpha", "difficulty": "hard", "team": "any"},
|
||||
{"name": "Bot_Bravo", "difficulty": "hard", "team": "any"},
|
||||
{"name": "Bot_Delta", "difficulty": "hard", "team": "any"},
|
||||
{"name": "Bot_Echo", "difficulty": "hard", "team": "any"},
|
||||
{"name": "Bot_Foxtrot", "difficulty": "easy", "team": "any"},
|
||||
]
|
||||
|
||||
result_defective, _ = get_random_profile_defective(profiles, "hard", "any", players)
|
||||
result_patched, _ = get_random_profile_patched(profiles, "hard", "any", players)
|
||||
|
||||
assert result_defective is not None, "Defective should find a profile"
|
||||
assert result_patched is not None, "Patched should find a profile"
|
||||
assert result_defective["name"] == result_patched["name"], (
|
||||
f"Both should return same profile: {result_defective['name']} vs {result_patched['name']}"
|
||||
)
|
||||
assert result_defective["name"] == "Bot_Delta", (
|
||||
f"Should select first untaken profile: got {result_defective['name']}"
|
||||
)
|
||||
print("PASS: correctness")
|
||||
|
||||
|
||||
def test_operation_count():
|
||||
"""Verify patched version does fewer operations."""
|
||||
# Simulate 100 bot profiles and 32 player slots (max CS 1.6 server)
|
||||
num_profiles = 100
|
||||
num_players = 32
|
||||
|
||||
players = [f"Bot_{i}" for i in range(num_players)]
|
||||
profiles = [
|
||||
{"name": f"Bot_{i}", "difficulty": "hard", "team": "any"}
|
||||
for i in range(num_profiles)
|
||||
]
|
||||
|
||||
_, ops_defective = get_random_profile_defective(profiles, "hard", "any", players)
|
||||
_, ops_patched = get_random_profile_patched(profiles, "hard", "any", players)
|
||||
|
||||
# Defective: ~P*C*2 = 100*32*2 = 6400 operations
|
||||
# Patched: C + P*2 = 32 + 200 = 232 operations
|
||||
ratio = ops_defective / max(ops_patched, 1)
|
||||
|
||||
print(f" Defective ops: {ops_defective}")
|
||||
print(f" Patched ops: {ops_patched}")
|
||||
print(f" Ratio: {ratio:.1f}x")
|
||||
|
||||
assert ratio > 5.0, f"Expected significant reduction, got {ratio:.1f}x"
|
||||
print("PASS: operation count")
|
||||
|
||||
|
||||
def test_scaling():
|
||||
"""Test that patched scales better as profiles increase."""
|
||||
num_players = 32
|
||||
players = [f"Bot_{i}" for i in range(num_players)]
|
||||
|
||||
ratios = []
|
||||
for num_profiles in [50, 100, 200, 500]:
|
||||
profiles = [
|
||||
{"name": f"Bot_{i}", "difficulty": "hard", "team": "any"}
|
||||
for i in range(num_profiles)
|
||||
]
|
||||
|
||||
_, ops_defective = get_random_profile_defective(profiles, "hard", "any", players)
|
||||
_, ops_patched = get_random_profile_patched(profiles, "hard", "any", players)
|
||||
ratio = ops_defective / max(ops_patched, 1)
|
||||
ratios.append(ratio)
|
||||
print(f" P={num_profiles:4d}: defective={ops_defective:6d} patched={ops_patched:4d} ratio={ratio:.1f}x")
|
||||
|
||||
# Ratio should increase with more profiles (defective grows as P*C, patched as P+C)
|
||||
assert ratios[-1] > ratios[0], "Ratio should increase with more profiles"
|
||||
print("PASS: scaling")
|
||||
|
||||
|
||||
def test_timing():
|
||||
"""Wall-clock timing comparison."""
|
||||
num_profiles = 500
|
||||
num_players = 32
|
||||
iterations = 100
|
||||
|
||||
players = [f"Bot_{i}" for i in range(num_players)]
|
||||
profiles = [
|
||||
{"name": f"Bot_{i}", "difficulty": "hard", "team": "any"}
|
||||
for i in range(num_profiles)
|
||||
]
|
||||
|
||||
start = time.perf_counter()
|
||||
for _ in range(iterations):
|
||||
get_random_profile_defective(profiles, "hard", "any", players)
|
||||
time_defective = time.perf_counter() - start
|
||||
|
||||
start = time.perf_counter()
|
||||
for _ in range(iterations):
|
||||
get_random_profile_patched(profiles, "hard", "any", players)
|
||||
time_patched = time.perf_counter() - start
|
||||
|
||||
speedup = time_defective / max(time_patched, 1e-9)
|
||||
print(f" Defective: {time_defective*1000:.1f}ms")
|
||||
print(f" Patched: {time_patched*1000:.1f}ms")
|
||||
print(f" Speedup: {speedup:.1f}x")
|
||||
|
||||
assert speedup > 2.0, f"Expected speedup > 2x, got {speedup:.1f}x"
|
||||
print("PASS: timing")
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
test_correctness()
|
||||
test_operation_count()
|
||||
test_scaling()
|
||||
test_timing()
|
||||
print("\nAll tests PASSED")
|
||||
BIN
defects/xash3d-0001/test/test_consistency_lookup
Executable file
BIN
defects/xash3d-0001/test/test_consistency_lookup
Executable file
Binary file not shown.
BIN
defects/xash3d-0002/test/test_precache_index
Executable file
BIN
defects/xash3d-0002/test/test_precache_index
Executable file
Binary file not shown.
BIN
defects/xash3d-0003/test/test_rcon_logging
Executable file
BIN
defects/xash3d-0003/test/test_rcon_logging
Executable file
Binary file not shown.
Loading…
Add table
Add a link
Reference in a new issue