dosbox-x: 2 CWE-407 defects (jtbs/dbox O(N²) text select, bdlist O(N²) DBCS convert); higan: all 5 MOADs CLEAN
This commit is contained in:
parent
d668589698
commit
f797e6f0ff
7 changed files with 333 additions and 0 deletions
|
|
@ -0,0 +1,45 @@
|
|||
--- a/src/hardware/vga_draw.cpp
|
||||
+++ b/src/hardware/vga_draw.cpp
|
||||
@@ -2585 +2585 @@
|
||||
-std::vector<std::pair<int,int>> jtbs = {}, dbox = {};
|
||||
+std::unordered_set<uint32_t> jtbs_set, dbox_set;
|
||||
+static inline uint32_t encode_pos(int row, int col) { return ((uint32_t)(uint16_t)row << 16) | (uint32_t)(uint16_t)col; }
|
||||
@@ -2622,2 +2622,2 @@
|
||||
- if (!jtbs.empty()) jtbs.erase(std::remove_if(jtbs.begin(), jtbs.end(), first_equal(row)), jtbs.end());
|
||||
- if (!dbox.empty()) dbox.erase(std::remove_if(dbox.begin(), dbox.end(), first_equal(row)), dbox.end());
|
||||
+ for (auto it = jtbs_set.begin(); it != jtbs_set.end(); ) { if ((int)(*it >> 16) == row) it = jtbs_set.erase(it); else ++it; }
|
||||
+ for (auto it = dbox_set.begin(); it != dbox_set.end(); ) { if ((int)(*it >> 16) == row) it = dbox_set.erase(it); else ++it; }
|
||||
@@ -2686 +2686 @@
|
||||
- if (line == 1) dbox.push_back(std::make_pair(row, col));
|
||||
+ if (line == 1) dbox_set.insert(encode_pos(row, col));
|
||||
@@ -2722 +2722 @@
|
||||
- if (line == 1) jtbs.push_back(std::make_pair(row, col));
|
||||
+ if (line == 1) jtbs_set.insert(encode_pos(row, col));
|
||||
--- a/src/ints/mouse.cpp
|
||||
+++ b/src/ints/mouse.cpp
|
||||
@@ -995 +995 @@
|
||||
-extern std::vector<std::pair<int,int>> jtbs, dbox;
|
||||
+extern std::unordered_set<uint32_t> jtbs_set, dbox_set;
|
||||
+static inline bool jtbs_has(int row, int col) { return jtbs_set.count(((uint32_t)(uint16_t)row << 16) | (uint32_t)(uint16_t)col) != 0; }
|
||||
+static inline bool dbox_has(int row, int col) { return dbox_set.count(((uint32_t)(uint16_t)row << 16) | (uint32_t)(uint16_t)col) != 0; }
|
||||
@@ -1116 +1116 @@
|
||||
- && showdbcs) ? std::find(jtbs.begin(), jtbs.end(), std::make_pair(i,j)) != jtbs.end():false;
|
||||
+ && showdbcs) ? jtbs_has(i,j) : false;
|
||||
@@ -1130 +1130 @@
|
||||
- && showdbcs && std::find(dbox.begin(), dbox.end(), std::make_pair(i,j)) != dbox.end()) bdlist.push_back(len);
|
||||
+ && showdbcs && dbox_has(i,j)) bdlist.push_back(len);
|
||||
@@ -1138 +1138 @@
|
||||
- && showdbcs))&&j==c2&&c2<c-1&&std::find(jtbs.begin(), jtbs.end(), std::make_pair(i,j+1)) != jtbs.end()) {
|
||||
+ && showdbcs))&&j==c2&&c2<c-1&&jtbs_has(i,j+1)) {
|
||||
#
|
||||
# CWE-407: Mouse_GetSelected scans jtbs/dbox (vector<pair<int,int>>) with
|
||||
# std::find inside nested loops over screen rows (i) and columns (j).
|
||||
# For a full 80x25 DBCS screen, jtbs can hold ~2000 entries. Each of the
|
||||
# ~2000 (row, col) positions calls std::find → O(rows * cols * N_entries)
|
||||
# = O(N²) where N = number of DBCS characters on screen.
|
||||
# Fix: replace vector<pair<int,int>> with unordered_set<uint32_t> using
|
||||
# (row<<16)|col encoding. Membership test drops from O(N) to O(1).
|
||||
# The row-based erase loop replaces erase(remove_if) — same O(N) cost,
|
||||
# called only on scanline invalidation, not per text-selection character.
|
||||
# Severity: MEDIUM — triggered during mouse text selection/copy in DBCS
|
||||
# (Japanese/Chinese/Korean) codepage mode; scales with screen DBCS content.
|
||||
BIN
defects/dosbox-x-0003/test/DosboxXJtbsDboxTest.class
Normal file
BIN
defects/dosbox-x-0003/test/DosboxXJtbsDboxTest.class
Normal file
Binary file not shown.
115
defects/dosbox-x-0003/test/DosboxXJtbsDboxTest.java
Normal file
115
defects/dosbox-x-0003/test/DosboxXJtbsDboxTest.java
Normal file
|
|
@ -0,0 +1,115 @@
|
|||
import java.util.*;
|
||||
|
||||
/**
|
||||
* Unit test for dosbox-x-0003: jtbs/dbox vector<pair<int,int>> O(N^2) membership
|
||||
* in Mouse_GetSelected nested row/col loops.
|
||||
*
|
||||
* Models the DOSBox-X defect where std::find is called on a vector of (row,col)
|
||||
* pairs inside nested loops over all screen positions. For a full DBCS screen
|
||||
* (80 cols x 25 rows), jtbs can hold ~2000 entries; each position calls find
|
||||
* making the total O(rows * cols * N_entries) = O(N^2).
|
||||
*
|
||||
* The fix encodes (row,col) as a single int key and stores in a HashSet,
|
||||
* making each membership test O(1).
|
||||
*/
|
||||
public class DosboxXJtbsDboxTest {
|
||||
|
||||
// --- Defect: vector<pair<int,int>> with linear find ---
|
||||
static List<long[]> buildVectorList(int screenCols, int screenRows) {
|
||||
List<long[]> list = new ArrayList<>();
|
||||
// Simulate: every other column is a double-byte char (DBCS), producing (row,col) pairs
|
||||
for (int row = 0; row < screenRows; row++) {
|
||||
for (int col = 0; col < screenCols; col += 2) {
|
||||
list.add(new long[]{row, col});
|
||||
}
|
||||
}
|
||||
return list;
|
||||
}
|
||||
|
||||
static boolean vectorContains(List<long[]> list, int row, int col) {
|
||||
for (long[] pair : list) {
|
||||
if (pair[0] == row && pair[1] == col) return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
static int countHitsVector(List<long[]> jtbs, int r1, int r2, int c1, int c2) {
|
||||
int hits = 0;
|
||||
for (int i = r1; i <= r2; i++) {
|
||||
for (int j = c1; j <= c2; j++) {
|
||||
if (vectorContains(jtbs, i, j)) hits++;
|
||||
}
|
||||
}
|
||||
return hits;
|
||||
}
|
||||
|
||||
// --- Fix: unordered_set<uint32_t> encoded as (row<<16)|col ---
|
||||
static int encodePos(int row, int col) {
|
||||
return ((row & 0xFFFF) << 16) | (col & 0xFFFF);
|
||||
}
|
||||
|
||||
static Set<Integer> buildHashSet(int screenCols, int screenRows) {
|
||||
Set<Integer> set = new HashSet<>();
|
||||
for (int row = 0; row < screenRows; row++) {
|
||||
for (int col = 0; col < screenCols; col += 2) {
|
||||
set.add(encodePos(row, col));
|
||||
}
|
||||
}
|
||||
return set;
|
||||
}
|
||||
|
||||
static int countHitsHashSet(Set<Integer> jtbs, int r1, int r2, int c1, int c2) {
|
||||
int hits = 0;
|
||||
for (int i = r1; i <= r2; i++) {
|
||||
for (int j = c1; j <= c2; j++) {
|
||||
if (jtbs.contains(encodePos(i, j))) hits++;
|
||||
}
|
||||
}
|
||||
return hits;
|
||||
}
|
||||
|
||||
public static void main(String[] args) {
|
||||
// Small correctness test
|
||||
int smallCols = 10, smallRows = 5;
|
||||
List<long[]> vec = buildVectorList(smallCols, smallRows);
|
||||
Set<Integer> set = buildHashSet(smallCols, smallRows);
|
||||
|
||||
int vecHits = countHitsVector(vec, 0, smallRows - 1, 0, smallCols - 1);
|
||||
int setHits = countHitsHashSet(set, 0, smallRows - 1, 0, smallCols - 1);
|
||||
assert vecHits == setHits : "Correctness failed: vec=" + vecHits + " set=" + setHits;
|
||||
System.out.println("Correctness OK: " + vecHits + " hits in " + smallCols + "x" + smallRows + " grid");
|
||||
|
||||
// Performance test: 80x25 full DBCS screen
|
||||
int cols = 80, rows = 25;
|
||||
List<long[]> bigVec = buildVectorList(cols, rows);
|
||||
Set<Integer> bigSet = buildHashSet(cols, rows);
|
||||
|
||||
// Warmup
|
||||
for (int i = 0; i < 3; i++) {
|
||||
countHitsVector(bigVec, 0, rows - 1, 0, cols - 1);
|
||||
countHitsHashSet(bigSet, 0, rows - 1, 0, cols - 1);
|
||||
}
|
||||
|
||||
int reps = 200;
|
||||
long t0 = System.nanoTime();
|
||||
int totalVec = 0;
|
||||
for (int i = 0; i < reps; i++) totalVec += countHitsVector(bigVec, 0, rows - 1, 0, cols - 1);
|
||||
long vecNs = System.nanoTime() - t0;
|
||||
|
||||
long t1 = System.nanoTime();
|
||||
int totalSet = 0;
|
||||
for (int i = 0; i < reps; i++) totalSet += countHitsHashSet(bigSet, 0, rows - 1, 0, cols - 1);
|
||||
long setNs = System.nanoTime() - t1;
|
||||
|
||||
assert totalVec == totalSet : "Result mismatch";
|
||||
|
||||
double ratio = (double) vecNs / setNs;
|
||||
System.out.printf("80x25 DBCS text selection (%d reps):%n", reps);
|
||||
System.out.printf(" vector<pair> linear scan: %,d ns%n", vecNs);
|
||||
System.out.printf(" unordered_set O(1) lookup: %,d ns%n", setNs);
|
||||
System.out.printf(" speedup: %.1fx%n", ratio);
|
||||
|
||||
assert ratio > 3.0 : "Expected >3x speedup, got " + ratio;
|
||||
System.out.println("PASS");
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,43 @@
|
|||
--- a/src/dos/drive_local.cpp
|
||||
+++ b/src/dos/drive_local.cpp
|
||||
@@ -278 +278 @@
|
||||
-std::list<uint16_t> bdlist = {};
|
||||
+std::unordered_set<uint16_t> bdlist;
|
||||
@@ -290 +290 @@
|
||||
- ) && (std::find(bdlist.begin(), bdlist.end(), (uint16_t)(baselen + s - ss)) != bdlist.end() || (isKanji1(*s) && (!(*(s+1)) || !isKanji2(*(s+1)))))) {
|
||||
+ ) && (bdlist.count((uint16_t)(baselen + s - ss)) || (isKanji1(*s) && (!(*(s+1)) || !isKanji2(*(s+1)))))) {
|
||||
@@ -342 +342 @@
|
||||
- ) && (std::find(bdlist.begin(), bdlist.end(), (uint16_t)(baselen + s - ss)) != bdlist.end() || (isKanji1(*s) && (!(*(s+1)) || !isKanji2(*(s+1))))) && utf8_encode(&d,df,(uint32_t)cp437_to_unicode[(uint8_t)*s]) >= 0) {
|
||||
+ ) && (bdlist.count((uint16_t)(baselen + s - ss)) || (isKanji1(*s) && (!(*(s+1)) || !isKanji2(*(s+1))))) && utf8_encode(&d,df,(uint32_t)cp437_to_unicode[(uint8_t)*s]) >= 0) {
|
||||
--- a/src/ints/mouse.cpp
|
||||
+++ b/src/ints/mouse.cpp
|
||||
@@ -993 +993 @@
|
||||
-extern std::list<uint16_t> bdlist;
|
||||
+extern std::unordered_set<uint16_t> bdlist;
|
||||
@@ -1044 +1044 @@
|
||||
- if (curAC[rtl?ttf.cols-x-1:x].boxdraw||(!x&&curAC[rtl?ttf.cols-x:x+1].boxdraw)) bdlist.push_back(len);
|
||||
+ if (curAC[rtl?ttf.cols-x-1:x].boxdraw||(!x&&curAC[rtl?ttf.cols-x:x+1].boxdraw)) bdlist.insert(len);
|
||||
@@ -1123 +1123 @@
|
||||
- if (curAC[rtl?ttfcols-j-1:j].boxdraw||(!j&&curAC[rtl?ttf.cols-j:j+1].boxdraw)) bdlist.push_back(len);
|
||||
+ if (curAC[rtl?ttfcols-j-1:j].boxdraw||(!j&&curAC[rtl?ttf.cols-j:j+1].boxdraw)) bdlist.insert(len);
|
||||
@@ -1146 +1146 @@
|
||||
- while (len>0&&text[len-1]==32) {text[--len]=0;bdlist.remove(len);}
|
||||
+ while (len>0&&text[len-1]==32) {text[--len]=0;bdlist.erase(len);}
|
||||
--- a/src/misc/clipboard.cpp
|
||||
+++ b/src/misc/clipboard.cpp
|
||||
@@ -721 +721 @@
|
||||
-extern std::list<uint16_t> bdlist;
|
||||
+extern std::unordered_set<uint16_t> bdlist;
|
||||
#
|
||||
# CWE-407: String_DBCS_TO_HOST_UTF16 and String_DBCS_TO_HOST_UTF8 in
|
||||
# drive_local.cpp call std::find(bdlist.begin(), bdlist.end(), pos) for
|
||||
# every character in the input string while converting DBCS filenames to
|
||||
# host encoding. bdlist is a std::list<uint16_t> tracking box-draw character
|
||||
# byte positions; std::find is O(N) per character, making each conversion
|
||||
# O(string_length * bdlist_size) = O(N²) when both grow together.
|
||||
# CROSS_LEN = 512 so worst case: 512 * 512 = 262,144 comparisons per filename.
|
||||
# This path is hot during file open, FindFirst, GetFileAttr, etc. in DBCS TTF mode.
|
||||
# Fix: replace std::list<uint16_t> with std::unordered_set<uint16_t>; membership
|
||||
# test drops from O(N) to O(1). push_back → insert; remove(val) → erase(val).
|
||||
# Severity: MEDIUM — triggered on every file I/O in DBCS TTF mode; O(N²) cost
|
||||
# per file access scales with number of box-draw characters in filenames/paths.
|
||||
BIN
defects/dosbox-x-0004/test/DosboxXBdlistTest.class
Normal file
BIN
defects/dosbox-x-0004/test/DosboxXBdlistTest.class
Normal file
Binary file not shown.
101
defects/dosbox-x-0004/test/DosboxXBdlistTest.java
Normal file
101
defects/dosbox-x-0004/test/DosboxXBdlistTest.java
Normal file
|
|
@ -0,0 +1,101 @@
|
|||
import java.util.*;
|
||||
|
||||
/**
|
||||
* Unit test for dosbox-x-0004: bdlist std::list<uint16_t> O(N^2) membership
|
||||
* in String_DBCS_TO_HOST_UTF16/UTF8 per-character conversion loop.
|
||||
*
|
||||
* Models DOSBox-X's DBCS filename conversion where std::find(bdlist) is called
|
||||
* for every character in the input string. bdlist tracks byte positions of
|
||||
* box-draw characters; with CROSS_LEN=512, worst case is 512*512=262,144
|
||||
* comparisons per filename conversion.
|
||||
*
|
||||
* The fix replaces std::list<uint16_t> with std::unordered_set<uint16_t>,
|
||||
* making each membership test O(1).
|
||||
*/
|
||||
public class DosboxXBdlistTest {
|
||||
|
||||
static final int CROSS_LEN = 512;
|
||||
|
||||
// --- Defect: std::list<uint16_t> with std::find per character ---
|
||||
static boolean listContains(List<Integer> bdlist, int pos) {
|
||||
for (int v : bdlist) {
|
||||
if (v == pos) return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
static int convertWithList(int[] stringBytes, List<Integer> bdlist) {
|
||||
int converted = 0;
|
||||
for (int i = 0; i < stringBytes.length; i++) {
|
||||
if (listContains(bdlist, i)) {
|
||||
converted++; // box-draw path
|
||||
} else {
|
||||
converted++; // normal DBCS path
|
||||
}
|
||||
}
|
||||
return converted;
|
||||
}
|
||||
|
||||
// --- Fix: std::unordered_set<uint16_t> with O(1) count ---
|
||||
static int convertWithSet(int[] stringBytes, Set<Integer> bdset) {
|
||||
int converted = 0;
|
||||
for (int i = 0; i < stringBytes.length; i++) {
|
||||
if (bdset.contains(i)) {
|
||||
converted++; // box-draw path
|
||||
} else {
|
||||
converted++; // normal DBCS path
|
||||
}
|
||||
}
|
||||
return converted;
|
||||
}
|
||||
|
||||
public static void main(String[] args) {
|
||||
// Build a worst-case string: CROSS_LEN chars, half are box-draw positions
|
||||
int[] str = new int[CROSS_LEN];
|
||||
Arrays.fill(str, 0xA1); // Kanji-range bytes
|
||||
|
||||
List<Integer> bdlist = new ArrayList<>();
|
||||
Set<Integer> bdset = new HashSet<>();
|
||||
for (int i = 0; i < CROSS_LEN; i += 2) {
|
||||
bdlist.add(i);
|
||||
bdset.add(i);
|
||||
}
|
||||
|
||||
// Correctness check
|
||||
int listResult = convertWithList(str, bdlist);
|
||||
int setResult = convertWithSet(str, bdset);
|
||||
assert listResult == setResult : "Correctness failed: list=" + listResult + " set=" + setResult;
|
||||
System.out.println("Correctness OK: both produce " + listResult + " converted chars");
|
||||
|
||||
// Performance benchmark
|
||||
int reps = 5000;
|
||||
|
||||
// Warmup
|
||||
for (int i = 0; i < 10; i++) {
|
||||
convertWithList(str, bdlist);
|
||||
convertWithSet(str, bdset);
|
||||
}
|
||||
|
||||
long t0 = System.nanoTime();
|
||||
long totalList = 0;
|
||||
for (int i = 0; i < reps; i++) totalList += convertWithList(str, bdlist);
|
||||
long listNs = System.nanoTime() - t0;
|
||||
|
||||
long t1 = System.nanoTime();
|
||||
long totalSet = 0;
|
||||
for (int i = 0; i < reps; i++) totalSet += convertWithSet(str, bdset);
|
||||
long setNs = System.nanoTime() - t1;
|
||||
|
||||
assert totalList == totalSet : "Result mismatch";
|
||||
|
||||
double ratio = (double) listNs / setNs;
|
||||
System.out.printf("DBCS filename conversion CROSS_LEN=%d, bdlist=%d entries (%d reps):%n",
|
||||
CROSS_LEN, bdlist.size(), reps);
|
||||
System.out.printf(" std::list<uint16_t> linear find: %,d ns%n", listNs);
|
||||
System.out.printf(" unordered_set<uint16_t> O(1): %,d ns%n", setNs);
|
||||
System.out.printf(" speedup: %.1fx%n", ratio);
|
||||
|
||||
assert ratio > 5.0 : "Expected >5x speedup, got " + ratio;
|
||||
System.out.println("PASS");
|
||||
}
|
||||
}
|
||||
29
defects/higan/scan
Normal file
29
defects/higan/scan
Normal file
|
|
@ -0,0 +1,29 @@
|
|||
CLEAN
|
||||
|
||||
MOAD-0001 (CWE-407): CLEAN
|
||||
- Emulation cores (SFC/SNES, FC/NES, GBA, WonderSwan, NGP, MD, PCE, MS)
|
||||
use fixed-size arrays indexed by sprite/object number for per-scanline
|
||||
and per-pixel paths. No list membership checks in hot render loops.
|
||||
- nall vector::find (linear O(N)) is called only in setup/attach paths
|
||||
(Screen::attach, Scheduler::append, node::remove) — bounded by small
|
||||
constant counts (sprites ~8, scheduler threads ~4). Not O(N^2).
|
||||
- icarus ROM database lookup: O(N_games) linear scan per ROM import — not
|
||||
O(N^2), just a single pass through 1162 SNES entries or ~500 GBA entries.
|
||||
- GBA heuristic scan: 7 identifiers x ROM_bytes x list.find (max 7 entries)
|
||||
= O(7 * D * 7). list stays bounded to num identifiers. Not O(N^2).
|
||||
|
||||
MOAD-0002 (Intertangle): CLEAN
|
||||
- higan uses global CPU/PPU/APU/Bus objects by design for emulation accuracy.
|
||||
This is intentional coupling, not an accidental god-object defect.
|
||||
|
||||
MOAD-0003 (Leaked Context): CLEAN
|
||||
- No thread_local or TLS usage found. higan uses cooperative coroutines
|
||||
(libco) for emulator threads — no OS thread-local context leakage.
|
||||
|
||||
MOAD-0004 (CWE-312): CLEAN
|
||||
- No credentials, tokens, passwords, or secrets are logged or persisted.
|
||||
higan handles only ROM data and save states — no network authentication.
|
||||
|
||||
MOAD-0005 (Thundering Herd): CLEAN
|
||||
- Single-threaded cooperative scheduler. No concurrent cache access patterns.
|
||||
All state mutation is deterministic and synchronous within scheduler turns.
|
||||
Loading…
Add table
Add a link
Reference in a new issue