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

This commit is contained in:
russell@unturf.com 2026-03-31 17:36:57 -04:00
parent 30abd28d6f
commit 6591438096
6 changed files with 230 additions and 0 deletions

View file

@ -0,0 +1,25 @@
# UNDF: UNDF-2026-XXXXXXXXX
--- a/src/core/cheats.cpp
+++ b/src/core/cheats.cpp
@@ -598,6 +598,8 @@ std::vector<std::string_view> Cheats::GetCodeListUniquePrefixes(const CodeInfoLi
std::vector<std::string_view> Cheats::GetCodeListUniquePrefixes(const CodeInfoList& list, bool include_empty)
{
std::vector<std::string_view> ret;
+ // Use a hash set to test prefix membership in O(1) instead of O(N) std::find,
+ // eliminating O(N^2) behaviour when building our unique-prefix list.
+ std::unordered_set<std::string_view> seen;
for (const Cheats::CodeInfo& code : list)
{
const std::string_view prefix = code.GetNameParentPart();
@@ -608,7 +610,9 @@ std::vector<std::string_view> Cheats::GetCodeListUniquePrefixes(const CodeInfoLi
continue;
}
- if (std::find(ret.begin(), ret.end(), prefix) == ret.end())
+ if (seen.insert(prefix).second)
+ {
ret.push_back(prefix);
+ }
}
return ret;
}

View file

@ -0,0 +1,92 @@
import java.util.*;
/**
* Unit test for DuckStation duckstation-0001:
* Cheats::GetCodeListUniquePrefixes uses std::find on a growing std::vector
* inside a loop over all cheat codes, giving O(N^2) behaviour.
*
* PS1 cheat databases (GameShark, CodeBreaker) contain 100s to 1000s of codes
* per game. This function is called every time our cheat settings UI tab opens.
*
* Defect file: src/core/cheats.cpp line 614
* Pattern: for each code, std::find(ret.begin(), ret.end(), prefix)
* Fix: insert into unordered_set for O(1) membership test per code.
*/
public class CheatsUniquePrefixesTest {
// --- Defective: O(N^2) linear scan dedup ---
static List<String> getUniquePrefixesDefective(List<String> codes) {
List<String> ret = new ArrayList<>();
for (String code : codes) {
// Simulate GetNameParentPart: prefix is everything before '/'
int slash = code.lastIndexOf('/');
String prefix = (slash >= 0) ? code.substring(0, slash) : "";
if (prefix.isEmpty()) continue;
if (!ret.contains(prefix)) { // O(N) scan per code
ret.add(prefix);
}
}
return ret;
}
// --- Fixed: O(N) with hash set ---
static List<String> getUniquePrefixesFixed(List<String> codes) {
List<String> ret = new ArrayList<>();
Set<String> seen = new HashSet<>();
for (String code : codes) {
int slash = code.lastIndexOf('/');
String prefix = (slash >= 0) ? code.substring(0, slash) : "";
if (prefix.isEmpty()) continue;
if (seen.add(prefix)) {
ret.add(prefix);
}
}
return ret;
}
public static void main(String[] args) {
int NUM_GROUPS = 200;
int CODES_PER_GROUP = 5;
int N = NUM_GROUPS * CODES_PER_GROUP;
// Build test data: codes named "Group_N/Code_M"
List<String> codes = new ArrayList<>(N);
for (int g = 0; g < NUM_GROUPS; g++) {
for (int c = 0; c < CODES_PER_GROUP; c++) {
codes.add("Group_" + g + "/Code_" + c);
}
}
// Correctness check
List<String> defResult = getUniquePrefixesDefective(codes);
List<String> fixResult = getUniquePrefixesFixed(codes);
Collections.sort(defResult);
Collections.sort(fixResult);
assert defResult.equals(fixResult)
: "Prefix list mismatch: defective=" + defResult.size() + " fixed=" + fixResult.size();
assert fixResult.size() == NUM_GROUPS : "Expected " + NUM_GROUPS + " unique prefixes";
// Warm up
for (int i = 0; i < 100; i++) {
getUniquePrefixesDefective(codes);
getUniquePrefixesFixed(codes);
}
// Benchmark
int ITER = 1000;
long t0 = System.nanoTime();
for (int i = 0; i < ITER; i++) getUniquePrefixesDefective(codes);
long defectNs = System.nanoTime() - t0;
t0 = System.nanoTime();
for (int i = 0; i < ITER; i++) getUniquePrefixesFixed(codes);
long fixedNs = System.nanoTime() - t0;
double ratio = (double) defectNs / fixedNs;
System.out.printf("GetCodeListUniquePrefixes N=%d codes (%d groups) defect=%.1fms fixed=%.1fms ratio=%.1fx%n",
N, NUM_GROUPS, defectNs / 1e6, fixedNs / 1e6, ratio);
assert ratio > 2.0 : "Expected >2x speedup, got " + ratio;
System.out.println("PASS");
}
}

View file

@ -0,0 +1,22 @@
# UNDF: UNDF-2026-XXXXXXXXX
--- a/src/core/cheats.cpp
+++ b/src/core/cheats.cpp
@@ -893,6 +893,8 @@ u32 Cheats::EnablePatches(const CheatCodeList& patches, const EnableCodeList& en
u32 Cheats::EnablePatches(const CheatCodeList& patches, const EnableCodeList& enable_list, const char* section,
bool hc_mode_active)
{
+ // Convert enable_list to a hash set for O(1) lookup instead of O(E) std::find
+ // per patch, eliminating O(P*E) behaviour when activating our patch/cheat lists.
+ const std::unordered_set<std::string> enable_set(enable_list.begin(), enable_list.end());
u32 count = 0;
for (const std::unique_ptr<CheatCode>& p : patches)
{
@@ -905,7 +907,7 @@ u32 Cheats::EnablePatches(const CheatCodeList& patches, const EnableCodeList& en
if (p->GetMetadata().disallow_for_achievements && hc_mode_active)
continue;
- if (std::find(enable_list.begin(), enable_list.end(), p->GetName()) == enable_list.end())
+ if (!enable_set.count(p->GetName()))
continue;
INFO_LOG("Enabled code from {}: {}", section, p->GetName());

View file

@ -0,0 +1,91 @@
import java.util.*;
/**
* Unit test for DuckStation duckstation-0002:
* Cheats::EnablePatches loops over all patch codes and calls std::find on
* enable_list (std::vector&lt;std::string&gt;) for each patch, giving O(P*E)
* behaviour where P = patch count and E = enabled names list length.
*
* PS1 GameShark/CodeBreaker patch databases can contain 100s of codes per game.
* Called once at game load for both patches and cheats subsections.
*
* Defect file: src/core/cheats.cpp line 909
* Pattern: for each patch, std::find(enable_list.begin(), enable_list.end(), name)
* Fix: build unordered_set from enable_list once, use count() for O(1) lookup.
*/
public class CheatsEnablePatchesTest {
// --- Defective: O(P*E) linear scan per patch ---
static int enablePatchesDefective(List<String> patches, List<String> enableList) {
int count = 0;
for (String name : patches) {
if (!name.isEmpty() && !enableList.contains(name)) { // O(E) scan per patch
continue;
}
count++;
}
return count;
}
// --- Fixed: O(P+E) with hash set ---
static int enablePatchesFixed(List<String> patches, List<String> enableList) {
Set<String> enableSet = new HashSet<>(enableList);
int count = 0;
for (String name : patches) {
if (!name.isEmpty() && !enableSet.contains(name)) { // O(1) per patch
continue;
}
count++;
}
return count;
}
public static void main(String[] args) {
int N = 500;
List<String> patches = new ArrayList<>(N);
List<String> enableList = new ArrayList<>(N);
for (int i = 0; i < N; i++) {
patches.add("Patch_" + i);
enableList.add("Patch_" + i);
}
// Correctness check
int defCount = enablePatchesDefective(patches, enableList);
int fixCount = enablePatchesFixed(patches, enableList);
assert defCount == fixCount
: "Count mismatch: defective=" + defCount + " fixed=" + fixCount;
assert fixCount == N : "Expected all " + N + " patches enabled";
// Partial enable list
List<String> partialEnable = enableList.subList(0, N / 2);
int defPartial = enablePatchesDefective(patches, partialEnable);
int fixPartial = enablePatchesFixed(patches, partialEnable);
assert defPartial == fixPartial
: "Partial count mismatch: defective=" + defPartial + " fixed=" + fixPartial;
// Warm up
for (int i = 0; i < 200; i++) {
enablePatchesDefective(patches, enableList);
enablePatchesFixed(patches, enableList);
}
// Benchmark
int ITER = 2000;
long t0 = System.nanoTime();
for (int i = 0; i < ITER; i++) enablePatchesDefective(patches, enableList);
long defectNs = System.nanoTime() - t0;
t0 = System.nanoTime();
for (int i = 0; i < ITER; i++) enablePatchesFixed(patches, enableList);
long fixedNs = System.nanoTime() - t0;
double ratio = (double) defectNs / fixedNs;
System.out.printf("EnablePatches N=%d defect=%.1fms fixed=%.1fms ratio=%.1fx%n",
N, defectNs / 1e6, fixedNs / 1e6, ratio);
assert ratio > 2.0 : "Expected >2x speedup, got " + ratio;
System.out.println("PASS");
}
}