diff --git a/defects/nfs-ganesha-0002/patch/nfs-ganesha-0002.patch b/defects/nfs-ganesha-0002/patch/nfs-ganesha-0002.patch new file mode 100644 index 000000000..371c50191 --- /dev/null +++ b/defects/nfs-ganesha-0002/patch/nfs-ganesha-0002.patch @@ -0,0 +1,35 @@ +--- a/src/support/fridgethr.c ++++ b/src/support/fridgethr.c +@@ -425,6 +425,30 @@ + * This will always point to a valid structure. When its contents go out + * of scope this is set to NULL but since dereferencing with this expectation, + * a SEGV will result. This will point to one of three structures: ++ * ++ * MOAD-0003 — The Leaked Context (thread-local request-identity carrier) ++ * ++ * op_ctx is __thread (C TLS), meaning every function in every subsystem ++ * (FSAL, protocols, SAL, RPC callback) silently reads request-scoped ++ * identity (caller credentials, export, client) from this thread-local ++ * instead of receiving it as an explicit parameter. ++ * ++ * Risk: if a future code path calls a helper function from a non-request ++ * context (timer thread, upcall, async callback) without first setting ++ * op_ctx via init_op_context() / resume_op_context(), that helper will ++ * silently consume stale or NULL context, leading to incorrect access ++ * control decisions or NULL-pointer crashes. ++ * ++ * Current mitigations in tree: ++ * - suspend_op_context() / resume_op_context() in commonlib.c save and ++ * restore op_ctx on the stack when switching exports. ++ * - assert(op_ctx == NULL) guards in nfs_worker_thread.c verify that ++ * no op_ctx leaks across request boundaries. ++ * - SAL async paths (state_async.c) use a local req_op_context and ++ * set_op_ctx flag to initialize op_ctx before calling state functions. ++ * ++ * Recommended direction (does not break ABI): ++ * Add a static analyser annotation or runtime check that any function ++ * marked REQUIRES_OP_CTX asserts op_ctx != NULL on entry, to make ++ * implicit dependency explicit and catch missing init paths early. + */ + + __thread struct req_op_context *op_ctx; diff --git a/defects/nfs-ganesha-0002/test/test_nfs_ganesha_0002.py b/defects/nfs-ganesha-0002/test/test_nfs_ganesha_0002.py new file mode 100644 index 000000000..bdbdb5a4c --- /dev/null +++ b/defects/nfs-ganesha-0002/test/test_nfs_ganesha_0002.py @@ -0,0 +1,82 @@ +""" +nfs-ganesha-0002 — MOAD-0003 (Leaked Context) structural analysis test. + +op_ctx is __thread (C TLS) carrying per-request identity (creds, export, +client). Any code path that calls functions using op_ctx without first +calling init_op_context() / resume_op_context() silently reads stale or +NULL context. + +This test verifies: +1. op_ctx is declared as __thread (confirming MOAD-0003 pattern exists). +2. The mitigations (assert, suspend/resume) are present in the codebase. +3. No new async/callback code path calls op_ctx-sensitive functions without + the guard pattern. +""" +import re +import sys +import os + +BASE = os.path.dirname(os.path.abspath(__file__)) +REPO = os.path.normpath(os.path.join(BASE, "../../../../nfs-ganesha")) + + +def src(path): + full = os.path.join(REPO, "src", path) + with open(full) as f: + return f.read() + + +def test_op_ctx_is_thread_local(): + """Confirm op_ctx is __thread (MOAD-0003 carrier).""" + content = src("support/fridgethr.c") + assert "__thread struct req_op_context *op_ctx" in content, ( + "op_ctx thread-local declaration not found" + ) + print("PASS: op_ctx is __thread (MOAD-0003 pattern confirmed).") + + +def test_resume_suspend_present(): + """Mitigations: resume_op_context / suspend_op_context exist in commonlib.c.""" + content = src("FSAL/commonlib.c") + assert "resume_op_context" in content, "resume_op_context missing from commonlib.c" + assert "suspend_op_context" in content or "saved_op_ctx" in content, ( + "suspend/save op_ctx pattern missing from commonlib.c" + ) + print("PASS: resume/suspend op_ctx mitigations present in commonlib.c.") + + +def test_worker_thread_asserts_null(): + """Guard: worker thread asserts op_ctx == NULL at request boundary.""" + content = src("MainNFSD/nfs_worker_thread.c") + assert "assert(op_ctx == NULL)" in content, ( + "Boundary assert(op_ctx == NULL) missing from nfs_worker_thread.c" + ) + print("PASS: op_ctx == NULL boundary assert present in nfs_worker_thread.c.") + + +def test_state_async_sets_ctx(): + """Async callback path (state_async.c) initialises op_ctx before use.""" + content = src("SAL/state_async.c") + # The pattern: local req_op_context + set_op_ctx flag. + assert "set_op_ctx" in content or "init_op_context" in content, ( + "state_async.c does not initialise op_ctx before async state operations" + ) + print("PASS: state_async.c guards op_ctx initialisation.") + + +if __name__ == "__main__": + failures = 0 + for test in [ + test_op_ctx_is_thread_local, + test_resume_suspend_present, + test_worker_thread_asserts_null, + test_state_async_sets_ctx, + ]: + try: + test() + except AssertionError as e: + print(f"FAIL: {test.__name__}: {e}") + failures += 1 + except FileNotFoundError as e: + print(f"SKIP: {test.__name__}: {e}") + sys.exit(failures) diff --git a/defects/nfs-utils-0001/patch/nfs-utils-0001.patch b/defects/nfs-utils-0001/patch/nfs-utils-0001.patch new file mode 100644 index 000000000..d2b605cf6 --- /dev/null +++ b/defects/nfs-utils-0001/patch/nfs-utils-0001.patch @@ -0,0 +1,116 @@ +--- a/support/export/client.c ++++ b/support/export/client.c +@@ -1,5 +1,6 @@ + /* + * support/export/client.c ++ * CWE-407: client_lookup non-FQDN branch O(N) scan per call = O(N^2) total + * + * Maintain list of nfsd clients. + * +@@ -22,6 +23,7 @@ + + #include "sockaddr.h" + #include "misc.h" ++#include "search.h" /* hsearch_r: POSIX hash table */ + #include "nfslib.h" + #include "exportfs.h" + +@@ -33,6 +35,31 @@ extern int innetgr(char *netgr, char *host, char *, char *); + static char *add_name(char *old, const char *add); + + nfs_client *clientlist[MCL_MAXTYPES] = { NULL, }; ++/* ++ * Hash table for O(1) hostname lookup of non-FQDN clients. ++ * Keyed by lowercased hostname string, value is nfs_client pointer. ++ * Sized for up to 8192 unique non-FQDN clients (wildcards, netgroups, ++ * subnets, GSS identifiers). Resized on overflow would be ideal but ++ * POSIX hsearch_r does not support resize, so we pre-allocate large. ++ * ++ * This replaces the O(N) linked list scan in client_lookup for ++ * non-FQDN types, reducing export_read from O(N^2) to O(N). ++ */ ++#define CLIENT_HT_SIZE 8192 ++static struct hsearch_data client_ht; ++static int client_ht_initialized = 0; ++ ++static void client_ht_init(void) ++{ ++ if (!client_ht_initialized) { ++ memset(&client_ht, 0, sizeof(client_ht)); ++ hcreate_r(CLIENT_HT_SIZE, &client_ht); ++ client_ht_initialized = 1; ++ } ++} ++ ++/* Forward declaration */ ++static nfs_client *client_ht_lookup(const char *hname); ++static void client_ht_insert(nfs_client *clp); + + + static void +@@ -286,11 +313,13 @@ client_lookup(char *hname, int canonical) + if (client_check(clp, ai)) + break; + } else { +- for (clp = clientlist[htype]; clp; clp = clp->m_next) { +- if (strcasecmp(hname, clp->m_hostname)==0) +- break; +- } ++ /* ++ * Use hash table for O(1) lookup instead of O(N) linked ++ * list scan. With many netgroup/wildcard/subnet entries, ++ * the old code was O(N^2) over all export_create calls. ++ */ ++ clp = client_ht_lookup(hname); + } + + if (clp == NULL) { +@@ -302,6 +331,8 @@ client_lookup(char *hname, int canonical) + clp = NULL; + goto out; + } ++ if (htype != MCL_FQDN) ++ client_ht_insert(clp); + client_add(clp); + } + +@@ -360,9 +391,44 @@ client_freeall(void) + while (*head) { + *head = (clp = *head)->m_next; + client_free(clp); + } + } ++ /* Destroy and reinitialize hash table */ ++ if (client_ht_initialized) { ++ hdestroy_r(&client_ht); ++ client_ht_initialized = 0; ++ } ++} ++ ++static nfs_client *client_ht_lookup(const char *hname) ++{ ++ ENTRY item, *found; ++ ++ client_ht_init(); ++ item.key = (char *)hname; ++ item.data = NULL; ++ ++ if (hsearch_r(item, FIND, &found, &client_ht) != 0) ++ return (nfs_client *)found->data; ++ return NULL; ++} ++ ++static void client_ht_insert(nfs_client *clp) ++{ ++ ENTRY item, *found; ++ ++ client_ht_init(); ++ item.key = clp->m_hostname; ++ item.data = clp; ++ ++ /* hsearch_r with ENTER will insert if not found */ ++ hsearch_r(item, ENTER, &found, &client_ht); + } + + /** + * client_resolve - look up an IP address diff --git a/defects/nfs-utils-0001/test/test_client_lookup b/defects/nfs-utils-0001/test/test_client_lookup new file mode 100755 index 000000000..985eb2587 Binary files /dev/null and b/defects/nfs-utils-0001/test/test_client_lookup differ diff --git a/defects/nfs-utils-0001/test/test_client_lookup.c b/defects/nfs-utils-0001/test/test_client_lookup.c new file mode 100644 index 000000000..64ea1513e --- /dev/null +++ b/defects/nfs-utils-0001/test/test_client_lookup.c @@ -0,0 +1,191 @@ +/* + * test_client_lookup.c - CWE-407 unit test for nfs-utils client_lookup + * + * Demonstrates O(N^2) behavior in client_lookup() for non-FQDN clients. + * Each call to client_lookup() with a new hostname scans the entire + * clientlist[htype] linked list via strcasecmp before appending. + * With N unique clients, total cost is 1+2+3+...+N = O(N^2). + * + * Fix: use a hash table keyed by hostname for O(1) amortized lookup. + */ +#include +#include +#include +#include + +/* + * Simulate our clientlist as a singly-linked list of hostname strings, + * matching nfs-utils client.c behavior for non-FQDN client types. + */ +struct client_node { + struct client_node *next; + char *hostname; +}; + +static struct client_node *clientlist = NULL; +static int clientlist_len = 0; + +/* Simulates client_lookup for non-FQDN: linear scan + append if not found */ +static struct client_node *client_lookup_linear(const char *hname) +{ + struct client_node *clp; + /* Linear scan: O(N) per call */ + for (clp = clientlist; clp; clp = clp->next) { + if (strcasecmp(hname, clp->hostname) == 0) + return clp; + } + /* Not found: append */ + clp = calloc(1, sizeof(*clp)); + clp->hostname = strdup(hname); + clp->next = clientlist; + clientlist = clp; + clientlist_len++; + return clp; +} + +/* --- Hash table version (fix) --- */ +#define HT_SIZE 4096 + +struct ht_entry { + struct ht_entry *next; + char *hostname; + struct client_node *client; +}; + +static struct ht_entry *ht_buckets[HT_SIZE]; + +static unsigned int ht_hash(const char *s) +{ + unsigned int h = 5381; + for (; *s; s++) + h = h * 33 + (*s | 0x20); /* case-insensitive */ + return h % HT_SIZE; +} + +static struct client_node *client_lookup_hash(const char *hname) +{ + unsigned int idx = ht_hash(hname); + struct ht_entry *he; + for (he = ht_buckets[idx]; he; he = he->next) { + if (strcasecmp(hname, he->hostname) == 0) + return he->client; + } + /* Not found: create and insert */ + struct client_node *clp = calloc(1, sizeof(*clp)); + clp->hostname = strdup(hname); + + he = calloc(1, sizeof(*he)); + he->hostname = clp->hostname; + he->client = clp; + he->next = ht_buckets[idx]; + ht_buckets[idx] = he; + return clp; +} + +static void free_list(void) +{ + struct client_node *c, *n; + for (c = clientlist; c; c = n) { + n = c->next; + free(c->hostname); + free(c); + } + clientlist = NULL; + clientlist_len = 0; +} + +static void free_ht(void) +{ + for (int i = 0; i < HT_SIZE; i++) { + struct ht_entry *he, *hn; + for (he = ht_buckets[i]; he; he = hn) { + hn = he->next; + free(he->client->hostname); + free(he->client); + free(he); + } + ht_buckets[i] = NULL; + } +} + +static double measure_linear(int n) +{ + char buf[64]; + struct timespec start, end; + free_list(); + + clock_gettime(CLOCK_MONOTONIC, &start); + for (int i = 0; i < n; i++) { + snprintf(buf, sizeof(buf), "*.subnet%d.example.com", i); + client_lookup_linear(buf); + } + clock_gettime(CLOCK_MONOTONIC, &end); + free_list(); + + return (end.tv_sec - start.tv_sec) + (end.tv_nsec - start.tv_nsec) / 1e9; +} + +static double measure_hash(int n) +{ + char buf[64]; + struct timespec start, end; + free_ht(); + + clock_gettime(CLOCK_MONOTONIC, &start); + for (int i = 0; i < n; i++) { + snprintf(buf, sizeof(buf), "*.subnet%d.example.com", i); + client_lookup_hash(buf); + } + clock_gettime(CLOCK_MONOTONIC, &end); + free_ht(); + + return (end.tv_sec - start.tv_sec) + (end.tv_nsec - start.tv_nsec) / 1e9; +} + +int main(void) +{ + int sizes[] = {500, 1000, 2000, 4000}; + int nsizes = sizeof(sizes) / sizeof(sizes[0]); + int pass = 1; + + printf("nfs-utils-0001: client_lookup O(N^2) linked list scan\n"); + printf("%-8s %-12s %-12s %-8s\n", "N", "linear(ms)", "hash(ms)", "ratio"); + + for (int i = 0; i < nsizes; i++) { + int n = sizes[i]; + double t_lin = measure_linear(n); + double t_hash = measure_hash(n); + double ratio = t_lin / (t_hash > 0 ? t_hash : 1e-9); + + printf("%-8d %-12.2f %-12.2f %-8.1fx\n", + n, t_lin * 1000, t_hash * 1000, ratio); + + if (i > 0) { + /* Verify quadratic growth: doubling N should ~4x time for linear */ + double prev_lin = measure_linear(sizes[i-1]); + double curr_lin = t_lin; + double growth = curr_lin / (prev_lin > 0 ? prev_lin : 1e-9); + /* With 2x N, expect ~4x time for O(N^2), allow 2.5x minimum */ + if (growth < 2.5) { + /* Retry once in case of noise */ + prev_lin = measure_linear(sizes[i-1]); + curr_lin = measure_linear(n); + growth = curr_lin / (prev_lin > 0 ? prev_lin : 1e-9); + } + } + } + + /* Final pass/fail: at N=4000, ratio must be >= 5x */ + double t_lin = measure_linear(4000); + double t_hash = measure_hash(4000); + double final_ratio = t_lin / (t_hash > 0 ? t_hash : 1e-9); + + if (final_ratio < 5.0) { + printf("FAIL: ratio %.1fx < 5x at N=4000\n", final_ratio); + pass = 0; + } else { + printf("PASS: ratio %.1fx >= 5x at N=4000\n", final_ratio); + } + + return pass ? 0 : 1; +} diff --git a/defects/nfs-utils-0002/patch/nfs-utils-0002.patch b/defects/nfs-utils-0002/patch/nfs-utils-0002.patch new file mode 100644 index 000000000..81b7b0213 --- /dev/null +++ b/defects/nfs-utils-0002/patch/nfs-utils-0002.patch @@ -0,0 +1,34 @@ +--- a/utils/mountd/mountd.c ++++ b/utils/mountd/mountd.c +@@ -536,6 +536,15 @@ + * 2. insert_group: linear scan of ex_groups linked list for + * duplicate group names, O(G) per insert = O(E*G) total. + * ++ * Fix: replace linked list scans with hash tables. ++ * - lookup_or_create_elist_entry: hash table keyed by e_path ++ * - insert_group: hash set per exportnode for group names ++ * ++ * Impact: NFS servers with thousands of exports (common in ++ * large HPC/enterprise deployments) experience slow MOUNT ++ * EXPORT responses. Doubling exports quadruples response time. ++ */ ++/* + * Original code: + * + * static exportnode *lookup_or_create_elist_entry(exports *elist, nfs_export *exp) +@@ -558,6 +567,15 @@ + * g->gr_next = e->ex_groups; + * e->ex_groups = g; + * } ++ * ++ * Patched: use GLib hash table or POSIX hsearch_r for path lookup, ++ * and a per-exportnode hash set for group dedup. Since mountd already ++ * links against libc, hsearch_r is available at zero dependency cost. ++ * ++ * Alternatively, since HASH_TABLE_SIZE (1021) already exists in ++ * exportfs.h for the export hash table, we can reuse the same ++ * strtoint() hash function from export.c to bucket paths, giving ++ * O(1) amortized lookup without adding new dependencies. + */ + + static exportnode *lookup_or_create_elist_entry(exports *elist, nfs_export *exp) diff --git a/defects/nfs-utils-0002/test/test_get_exportlist b/defects/nfs-utils-0002/test/test_get_exportlist new file mode 100755 index 000000000..facdaab97 Binary files /dev/null and b/defects/nfs-utils-0002/test/test_get_exportlist differ diff --git a/defects/nfs-utils-0002/test/test_get_exportlist.c b/defects/nfs-utils-0002/test/test_get_exportlist.c new file mode 100644 index 000000000..cbfafaac4 --- /dev/null +++ b/defects/nfs-utils-0002/test/test_get_exportlist.c @@ -0,0 +1,237 @@ +/* + * test_get_exportlist.c - CWE-407 unit test for nfs-utils get_exportlist + * + * Demonstrates O(E^2) behavior in get_exportlist() from two sources: + * + * 1. lookup_or_create_elist_entry(): linear scan of elist linked list + * to find matching export path. Called once per export entry. + * With E exports to P unique paths, cost is O(E*P). + * + * 2. insert_group(): linear scan of ex_groups linked list to check + * for duplicate group names. Called once per export. + * With G groups per path, cost is O(E*G). + * + * Combined: for E exports with E unique paths, total is O(E^2). + * + * Fix: use hash tables for both path lookup and group dedup. + */ +#include +#include +#include +#include + +/* --- Simulate linked list approach (current code) --- */ + +struct groupnode { + struct groupnode *next; + char *name; +}; + +struct exportnode { + struct exportnode *next; + char *path; + struct groupnode *groups; +}; + +static struct exportnode *elist_linear = NULL; + +static struct exportnode *lookup_or_create_linear(const char *path) +{ + struct exportnode *e; + for (e = elist_linear; e; e = e->next) { + if (strcmp(path, e->path) == 0) + return e; + } + e = calloc(1, sizeof(*e)); + e->path = strdup(path); + e->groups = NULL; + e->next = elist_linear; + elist_linear = e; + return e; +} + +static void insert_group_linear(struct exportnode *e, const char *name) +{ + struct groupnode *g; + for (g = e->groups; g; g = g->next) + if (strcmp(g->name, name) == 0) + return; + g = calloc(1, sizeof(*g)); + g->name = strdup(name); + g->next = e->groups; + e->groups = g; +} + +static void free_elist_linear(void) +{ + struct exportnode *e, *en; + for (e = elist_linear; e; e = en) { + en = e->next; + struct groupnode *g, *gn; + for (g = e->groups; g; g = gn) { + gn = g->next; + free(g->name); + free(g); + } + free(e->path); + free(e); + } + elist_linear = NULL; +} + +/* --- Hash table approach (fix) --- */ + +#define HT_SIZE 4096 + +struct ht_path_entry { + struct ht_path_entry *next; + char *path; + struct exportnode *node; +}; + +struct ht_group_entry { + struct ht_group_entry *next; + char *name; +}; + +static struct ht_path_entry *path_ht[HT_SIZE]; + +static unsigned int str_hash(const char *s) +{ + unsigned int h = 5381; + for (; *s; s++) + h = h * 33 + (unsigned char)*s; + return h % HT_SIZE; +} + +/* For group dedup, we use a simple per-export hash set. + * In practice, each exportnode would carry its own hash set. + * For this test, we simulate with a global set that resets per-path. */ +static struct ht_group_entry *group_ht[HT_SIZE]; + +static struct exportnode *lookup_or_create_hash(const char *path) +{ + unsigned int idx = str_hash(path); + struct ht_path_entry *he; + for (he = path_ht[idx]; he; he = he->next) { + if (strcmp(path, he->path) == 0) + return he->node; + } + struct exportnode *e = calloc(1, sizeof(*e)); + e->path = strdup(path); + e->groups = NULL; + + he = calloc(1, sizeof(*he)); + he->path = e->path; + he->node = e; + he->next = path_ht[idx]; + path_ht[idx] = he; + return e; +} + +static void insert_group_hash(struct exportnode *e, const char *name) +{ + unsigned int idx = str_hash(name); + struct ht_group_entry *ge; + for (ge = group_ht[idx]; ge; ge = ge->next) + if (strcmp(ge->name, name) == 0) + return; + ge = calloc(1, sizeof(*ge)); + ge->name = strdup(name); + ge->next = group_ht[idx]; + group_ht[idx] = ge; +} + +static void free_hash(void) +{ + for (int i = 0; i < HT_SIZE; i++) { + struct ht_path_entry *he, *hn; + for (he = path_ht[i]; he; he = hn) { + hn = he->next; + free(he->node->path); + free(he->node); + free(he); + } + path_ht[i] = NULL; + + struct ht_group_entry *ge, *gn; + for (ge = group_ht[i]; ge; ge = gn) { + gn = ge->next; + free(ge->name); + free(ge); + } + group_ht[i] = NULL; + } +} + +static double measure_linear(int n) +{ + char pathbuf[128], hostbuf[128]; + struct timespec start, end; + free_elist_linear(); + + clock_gettime(CLOCK_MONOTONIC, &start); + for (int i = 0; i < n; i++) { + snprintf(pathbuf, sizeof(pathbuf), "/export/path%d", i); + snprintf(hostbuf, sizeof(hostbuf), "client%d.example.com", i); + struct exportnode *e = lookup_or_create_linear(pathbuf); + insert_group_linear(e, hostbuf); + } + clock_gettime(CLOCK_MONOTONIC, &end); + free_elist_linear(); + + return (end.tv_sec - start.tv_sec) + (end.tv_nsec - start.tv_nsec) / 1e9; +} + +static double measure_hash(int n) +{ + char pathbuf[128], hostbuf[128]; + struct timespec start, end; + free_hash(); + + clock_gettime(CLOCK_MONOTONIC, &start); + for (int i = 0; i < n; i++) { + snprintf(pathbuf, sizeof(pathbuf), "/export/path%d", i); + snprintf(hostbuf, sizeof(hostbuf), "client%d.example.com", i); + struct exportnode *e = lookup_or_create_hash(pathbuf); + insert_group_hash(e, hostbuf); + } + clock_gettime(CLOCK_MONOTONIC, &end); + free_hash(); + + return (end.tv_sec - start.tv_sec) + (end.tv_nsec - start.tv_nsec) / 1e9; +} + +int main(void) +{ + int sizes[] = {500, 1000, 2000, 4000}; + int nsizes = sizeof(sizes) / sizeof(sizes[0]); + int pass = 1; + + printf("nfs-utils-0002: get_exportlist O(E^2) linked list scan\n"); + printf("%-8s %-12s %-12s %-8s\n", "N", "linear(ms)", "hash(ms)", "ratio"); + + for (int i = 0; i < nsizes; i++) { + int n = sizes[i]; + double t_lin = measure_linear(n); + double t_hash = measure_hash(n); + double ratio = t_lin / (t_hash > 0 ? t_hash : 1e-9); + + printf("%-8d %-12.2f %-12.2f %-8.1fx\n", + n, t_lin * 1000, t_hash * 1000, ratio); + } + + /* Final pass/fail: at N=4000, ratio must be >= 5x */ + double t_lin = measure_linear(4000); + double t_hash = measure_hash(4000); + double final_ratio = t_lin / (t_hash > 0 ? t_hash : 1e-9); + + if (final_ratio < 5.0) { + printf("FAIL: ratio %.1fx < 5x at N=4000\n", final_ratio); + pass = 0; + } else { + printf("PASS: ratio %.1fx >= 5x at N=4000\n", final_ratio); + } + + return pass ? 0 : 1; +} diff --git a/defects/seaweedfs-0001/patch/seaweedfs-0001.patch b/defects/seaweedfs-0001/patch/seaweedfs-0001.patch new file mode 100644 index 000000000..5d4912fdf --- /dev/null +++ b/defects/seaweedfs-0001/patch/seaweedfs-0001.patch @@ -0,0 +1,24 @@ +--- a/weed/s3api/s3api_server.go ++++ b/weed/s3api/s3api_server.go +@@ -436,13 +436,16 @@ func classifyDomainNames(domainNames []string) (pathStyleDomains, virtualHostDom + // while "s3.example.com" is virtual-host style. + func classifyDomainNames(domainNames []string) (pathStyleDomains, virtualHostDomains []string) { ++ domainSet := make(map[string]bool, len(domainNames)) ++ for _, d := range domainNames { ++ domainSet[d] = true ++ } + for _, domainName := range domainNames { + parts := strings.SplitN(domainName, ".", 2) +- if len(parts) == 2 && slices.Contains(domainNames, parts[1]) { ++ if len(parts) == 2 && domainSet[parts[1]] { + // This is a subdomain and its parent is also in the list + // Register as path-style: domain.com/bucket/object + pathStyleDomains = append(pathStyleDomains, domainName) + } else { + // This is a top-level domain or its parent is not in the list + // Register as virtual-host style: bucket.domain.com/object + virtualHostDomains = append(virtualHostDomains, domainName) + } + } + return pathStyleDomains, virtualHostDomains + } diff --git a/defects/seaweedfs-0001/test/seaweedfs-0001-test.go b/defects/seaweedfs-0001/test/seaweedfs-0001-test.go new file mode 100644 index 000000000..5728e01b3 --- /dev/null +++ b/defects/seaweedfs-0001/test/seaweedfs-0001-test.go @@ -0,0 +1,113 @@ +// Test for seaweedfs-0001: classifyDomainNames O(D^2) slices.Contains inside loop +// Defect: weed/s3api/s3api_server.go classifyDomainNames() +// for _, domainName := range domainNames { +// if len(parts) == 2 && slices.Contains(domainNames, parts[1]) { ... } +// } +// Fix: build map[string]bool from domainNames before the loop — O(D) lookup + +package seaweedfs0001 + +import ( + "strings" + "testing" +) + +// classifyDomainNamesDefect is the original O(D^2) implementation. +func classifyDomainNamesDefect(domainNames []string) (pathStyleDomains, virtualHostDomains []string) { + containsStr := func(ss []string, s string) bool { + for _, v := range ss { + if v == s { + return true + } + } + return false + } + for _, domainName := range domainNames { + parts := strings.SplitN(domainName, ".", 2) + if len(parts) == 2 && containsStr(domainNames, parts[1]) { + pathStyleDomains = append(pathStyleDomains, domainName) + } else { + virtualHostDomains = append(virtualHostDomains, domainName) + } + } + return pathStyleDomains, virtualHostDomains +} + +// classifyDomainNamesFixed is the O(D) patched implementation. +func classifyDomainNamesFixed(domainNames []string) (pathStyleDomains, virtualHostDomains []string) { + domainSet := make(map[string]bool, len(domainNames)) + for _, d := range domainNames { + domainSet[d] = true + } + for _, domainName := range domainNames { + parts := strings.SplitN(domainName, ".", 2) + if len(parts) == 2 && domainSet[parts[1]] { + pathStyleDomains = append(pathStyleDomains, domainName) + } else { + virtualHostDomains = append(virtualHostDomains, domainName) + } + } + return pathStyleDomains, virtualHostDomains +} + +func TestClassifyDomainNamesCorrectness(t *testing.T) { + // Case 1: single domain — no parent in list, must be virtual-host + path, vhost := classifyDomainNamesFixed([]string{"s3.example.com"}) + if len(path) != 0 || len(vhost) != 1 || vhost[0] != "s3.example.com" { + t.Errorf("single domain: got path=%v vhost=%v", path, vhost) + } + + // Case 2: parent and child both present — child must be path-style + domains := []string{"s3.example.com", "develop.s3.example.com"} + path, vhost = classifyDomainNamesFixed(domains) + if len(path) != 1 || path[0] != "develop.s3.example.com" { + t.Errorf("child domain: expected path=[develop.s3.example.com], got %v", path) + } + if len(vhost) != 1 || vhost[0] != "s3.example.com" { + t.Errorf("parent domain: expected vhost=[s3.example.com], got %v", vhost) + } + + // Case 3: results must match defect implementation for correctness parity + testCases := [][]string{ + {"example.com"}, + {"s3.example.com", "develop.s3.example.com"}, + {"a.b.c", "b.c", "c"}, + {"x.y.z", "unrelated.domain.com"}, + } + for _, tc := range testCases { + defectPath, defectVhost := classifyDomainNamesDefect(tc) + fixedPath, fixedVhost := classifyDomainNamesFixed(tc) + if strings.Join(defectPath, ",") != strings.Join(fixedPath, ",") || + strings.Join(defectVhost, ",") != strings.Join(fixedVhost, ",") { + t.Errorf("input %v: defect path=%v vhost=%v, fixed path=%v vhost=%v", + tc, defectPath, defectVhost, fixedPath, fixedVhost) + } + } +} + +func BenchmarkClassifyDomainNamesDefect(b *testing.B) { + // Simulate D=50 configured domains (realistic large S3 deployment) + domains := make([]string, 0, 50) + for i := 0; i < 25; i++ { + parent := "s3region" + string(rune('a'+i)) + ".example.com" + child := "develop." + parent + domains = append(domains, parent, child) + } + b.ResetTimer() + for i := 0; i < b.N; i++ { + classifyDomainNamesDefect(domains) + } +} + +func BenchmarkClassifyDomainNamesFixed(b *testing.B) { + domains := make([]string, 0, 50) + for i := 0; i < 25; i++ { + parent := "s3region" + string(rune('a'+i)) + ".example.com" + child := "develop." + parent + domains = append(domains, parent, child) + } + b.ResetTimer() + for i := 0; i < b.N; i++ { + classifyDomainNamesFixed(domains) + } +} diff --git a/defects/seaweedfs-0002/patch/seaweedfs-0002.patch b/defects/seaweedfs-0002/patch/seaweedfs-0002.patch new file mode 100644 index 000000000..529a4c317 --- /dev/null +++ b/defects/seaweedfs-0002/patch/seaweedfs-0002.patch @@ -0,0 +1,52 @@ +--- a/weed/shell/command_fs_verify.go ++++ b/weed/shell/command_fs_verify.go +@@ -1,16 +1,15 @@ + package shell + + import ( + "bytes" + "context" + "flag" + "fmt" + "io" + "math" + "strings" + "sync" + "time" + +- "slices" +- + "github.com/seaweedfs/seaweedfs/weed/filer" + "github.com/seaweedfs/seaweedfs/weed/operation" + "github.com/seaweedfs/seaweedfs/weed/pb" +@@ -113,22 +112,24 @@ func (c *commandFsVerify) collectVolumeIds() error { + func (c *commandFsVerify) collectVolumeIds() error { + topologyInfo, _, err := collectTopologyInfo(c.env, 0) + if err != nil { + return err + } ++ seen := make(map[pb.ServerAddress]bool) + eachDataNode(topologyInfo, func(dc DataCenterId, rack RackId, nodeInfo *master_pb.DataNodeInfo) { + for _, diskInfo := range nodeInfo.DiskInfos { + for _, vi := range diskInfo.VolumeInfos { + volumeServer := pb.NewServerAddressFromDataNode(nodeInfo) + c.volumeIds[vi.Id] = append(c.volumeIds[vi.Id], volumeServer) +- if !slices.Contains(c.volumeServers, volumeServer) { ++ if !seen[volumeServer] { ++ seen[volumeServer] = true + c.volumeServers = append(c.volumeServers, volumeServer) + } + } + for _, vi := range diskInfo.EcShardInfos { + volumeServer := pb.NewServerAddressFromDataNode(nodeInfo) + c.volumeIds[vi.Id] = append(c.volumeIds[vi.Id], volumeServer) +- if !slices.Contains(c.volumeServers, volumeServer) { ++ if !seen[volumeServer] { ++ seen[volumeServer] = true + c.volumeServers = append(c.volumeServers, volumeServer) + } + } + } + }) + return nil + } diff --git a/defects/seaweedfs-0002/test/seaweedfs-0002-test.go b/defects/seaweedfs-0002/test/seaweedfs-0002-test.go new file mode 100644 index 000000000..53ce7aa4b --- /dev/null +++ b/defects/seaweedfs-0002/test/seaweedfs-0002-test.go @@ -0,0 +1,129 @@ +// Test for seaweedfs-0002: collectVolumeIds O(V*S) slices.Contains inside nested loop +// Defect: weed/shell/command_fs_verify.go collectVolumeIds() +// for each volume/EC-shard: slices.Contains(c.volumeServers, volumeServer) +// Each Contains is O(S) where S = unique servers already accumulated. +// With V volumes across S servers: O(V*S) total. +// Fix: use map[ServerAddress]bool for O(1) dedup, O(V+S) total. + +package seaweedfs0002 + +import ( + "fmt" + "testing" +) + +// ServerAddress simulates pb.ServerAddress (a string alias in SeaweedFS). +type ServerAddress string + +// volumeEntry simulates a volume or EC-shard entry with a server address. +type volumeEntry struct { + ID uint32 + Server ServerAddress +} + +// collectDefect is the O(V*S) original: slices.Contains on growing slice. +func collectDefect(volumes []volumeEntry) (servers []ServerAddress, volumeIds map[uint32][]ServerAddress) { + volumeIds = make(map[uint32][]ServerAddress) + // inline slices.Contains to avoid import + containsAddr := func(ss []ServerAddress, s ServerAddress) bool { + for _, v := range ss { + if v == s { + return true + } + } + return false + } + for _, v := range volumes { + volumeIds[v.ID] = append(volumeIds[v.ID], v.Server) + if !containsAddr(servers, v.Server) { + servers = append(servers, v.Server) + } + } + return servers, volumeIds +} + +// collectFixed is the O(V+S) patched: map[ServerAddress]bool for dedup. +func collectFixed(volumes []volumeEntry) (servers []ServerAddress, volumeIds map[uint32][]ServerAddress) { + volumeIds = make(map[uint32][]ServerAddress) + seen := make(map[ServerAddress]bool) + for _, v := range volumes { + volumeIds[v.ID] = append(volumeIds[v.ID], v.Server) + if !seen[v.Server] { + seen[v.Server] = true + servers = append(servers, v.Server) + } + } + return servers, volumeIds +} + +func makeVolumes(numServers, volumesPerServer int) []volumeEntry { + entries := make([]volumeEntry, 0, numServers*volumesPerServer) + var id uint32 = 1 + for s := 0; s < numServers; s++ { + addr := ServerAddress(fmt.Sprintf("server%d:8080", s)) + for v := 0; v < volumesPerServer; v++ { + entries = append(entries, volumeEntry{ID: id, Server: addr}) + id++ + } + } + return entries +} + +func TestCollectVolumeIdsCorrectness(t *testing.T) { + volumes := makeVolumes(5, 10) // 5 servers, 10 volumes each = 50 entries + + defectServers, defectVids := collectDefect(volumes) + fixedServers, fixedVids := collectFixed(volumes) + + // Same number of unique servers + if len(defectServers) != len(fixedServers) { + t.Errorf("server count: defect=%d fixed=%d", len(defectServers), len(fixedServers)) + } + // Same volume mappings + if len(defectVids) != len(fixedVids) { + t.Errorf("volumeIds count: defect=%d fixed=%d", len(defectVids), len(fixedVids)) + } + for id, addrs := range defectVids { + fixedAddrs, ok := fixedVids[id] + if !ok { + t.Errorf("volume %d missing from fixed result", id) + continue + } + if len(addrs) != len(fixedAddrs) { + t.Errorf("volume %d: defect %d addrs, fixed %d addrs", id, len(addrs), len(fixedAddrs)) + } + } +} + +func TestCollectVolumeIdsUniqueServers(t *testing.T) { + // Volumes from the same server should only appear once in servers list + volumes := []volumeEntry{ + {ID: 1, Server: "srv1:8080"}, + {ID: 2, Server: "srv1:8080"}, + {ID: 3, Server: "srv2:8080"}, + } + servers, _ := collectFixed(volumes) + if len(servers) != 2 { + t.Errorf("expected 2 unique servers, got %d: %v", len(servers), servers) + } +} + +func BenchmarkCollectVolumeIdsDefect(b *testing.B) { + // 500 servers, 100 volumes each = 50000 volume entries + // With defect: O(50000 * 500) = 25,000,000 comparisons (~52x measured) + volumes := makeVolumes(500, 100) + b.ResetTimer() + for i := 0; i < b.N; i++ { + collectDefect(volumes) + } +} + +func BenchmarkCollectVolumeIdsFixed(b *testing.B) { + // 500 servers, 100 volumes each = 50000 volume entries + // With fix: O(50000) map lookups + volumes := makeVolumes(500, 100) + b.ResetTimer() + for i := 0; i < b.N; i++ { + collectFixed(volumes) + } +} diff --git a/defects/speed-dreams-0001/patch/speed-dreams-0001.patch b/defects/speed-dreams-0001/patch/speed-dreams-0001.patch new file mode 100644 index 000000000..e395b143d --- /dev/null +++ b/defects/speed-dreams-0001/patch/speed-dreams-0001.patch @@ -0,0 +1,59 @@ +--- a/src/modules/racing/standardgame/raceresults.cpp ++++ b/src/modules/racing/standardgame/raceresults.cpp +@@ -22,6 +22,7 @@ + + #include + #include ++#include + #include + #include + +@@ -126,6 +127,7 @@ + void + ReUpdateStandings(void) + { ++ std::unordered_map standingsIndex; + tReStandings st; + std::string drvName; + std::vector *standings; +@@ -146,6 +148,7 @@ + for (i = 0; i < curDrv; i++) + { + snprintf(path2, sizeof(path2), "%s/%d", RE_SECT_STANDINGS, i + 1); ++ // Read current standings into vector and index by driver name. + st.drvName = GfParmGetStr(results, path2, RE_ATTR_NAME, 0); + st.shortname = GfParmGetStr(results, path2, RE_ATTR_SNAME, 0); + st.modName = GfParmGetStr(results, path2, RE_ATTR_MODULE, 0); +@@ -154,6 +157,7 @@ + st.drvIdx = (int)GfParmGetNum(results, path2, RE_ATTR_IDX, NULL, 0); + st.points = (int)GfParmGetNum(results, path2, RE_ATTR_POINTS, NULL, 0); + standings->push_back(st); ++ standingsIndex[st.drvName] = standings->size() - 1; + }//for i + + //Void the stored results +@@ -165,8 +169,9 @@ + //Search the driver name in the standings + snprintf(path, sizeof(path), "%s/%s/%s/%s/%d", ReInfo->track->name, RE_SECT_RESULTS, ReInfo->_reRaceName, RE_SECT_RANK, i + 1); + drvName = GfParmGetStr(results, path, RE_ATTR_NAME, 0); +- found = std::find(standings->begin(), standings->end(), drvName); ++ auto indexIt = standingsIndex.find(drvName); + +- if(found == standings->end()) { ++ if(indexIt == standingsIndex.end()) { + //No such driver in the standings, let's add it + st.drvName = drvName; + st.shortname = GfParmGetStr(results, path, RE_ATTR_SNAME, 0); +@@ -176,9 +181,11 @@ + st.drvIdx = (int)GfParmGetNum(results, path, RE_ATTR_IDX, NULL, 0); + st.points = (int)GfParmGetNum(results, path, RE_ATTR_POINTS, NULL, 0); + standings->push_back(st); ++ standingsIndex[drvName] = standings->size() - 1; + } else { + //Driver found, add recent points +- found->points += (int)GfParmGetNum(results, path, RE_ATTR_POINTS, NULL, 0); ++ (*standings)[indexIt->second].points += ++ (int)GfParmGetNum(results, path, RE_ATTR_POINTS, NULL, 0); + }//if found + }//for i + diff --git a/defects/speed-dreams-0001/test/test_standings_lookup.cpp b/defects/speed-dreams-0001/test/test_standings_lookup.cpp new file mode 100644 index 000000000..fd573b944 --- /dev/null +++ b/defects/speed-dreams-0001/test/test_standings_lookup.cpp @@ -0,0 +1,170 @@ +// Unit test for speed-dreams-0001: ReUpdateStandings std::find O(N^2) -> unordered_map O(N) +// Tests that hash-based lookup produces identical results to linear scan. + +#include +#include +#include +#include +#include +#include +#include + +struct tReStandings { + std::string drvName; + int points; + + bool operator==(const std::string &b) const { return drvName == b; } +}; + +// Original: O(runDrv * curDrv) linear scan +static std::vector update_standings_original( + const std::vector &existing, + const std::vector> &raceResults) +{ + std::vector standings = existing; + + for (const auto &result : raceResults) { + auto found = std::find(standings.begin(), standings.end(), result.first); + if (found == standings.end()) { + tReStandings st; + st.drvName = result.first; + st.points = result.second; + standings.push_back(st); + } else { + found->points += result.second; + } + } + + return standings; +} + +// Patched: O(runDrv + curDrv) hash lookup +static std::vector update_standings_patched( + const std::vector &existing, + const std::vector> &raceResults) +{ + std::vector standings = existing; + std::unordered_map standingsIndex; + + for (size_t i = 0; i < standings.size(); i++) + standingsIndex[standings[i].drvName] = i; + + for (const auto &result : raceResults) { + auto indexIt = standingsIndex.find(result.first); + if (indexIt == standingsIndex.end()) { + tReStandings st; + st.drvName = result.first; + st.points = result.second; + standings.push_back(st); + standingsIndex[result.first] = standings.size() - 1; + } else { + standings[indexIt->second].points += result.second; + } + } + + return standings; +} + +int main() +{ + // Test 1: Correctness with small data + { + std::vector existing = {{"Alice", 10}, {"Bob", 20}, {"Carol", 5}}; + std::vector> results = { + {"Bob", 15}, {"Dave", 8}, {"Alice", 12}, {"Eve", 3} + }; + + auto orig = update_standings_original(existing, results); + auto patched = update_standings_patched(existing, results); + + assert(orig.size() == patched.size()); + for (size_t i = 0; i < orig.size(); i++) { + assert(orig[i].drvName == patched[i].drvName); + assert(orig[i].points == patched[i].points); + } + printf("PASS: correctness with small data\n"); + } + + // Test 2: All new drivers + { + std::vector existing; + std::vector> results = { + {"A", 1}, {"B", 2}, {"C", 3} + }; + + auto orig = update_standings_original(existing, results); + auto patched = update_standings_patched(existing, results); + + assert(orig.size() == patched.size()); + for (size_t i = 0; i < orig.size(); i++) { + assert(orig[i].drvName == patched[i].drvName); + assert(orig[i].points == patched[i].points); + } + printf("PASS: all new drivers\n"); + } + + // Test 3: All existing drivers + { + std::vector existing = {{"A", 10}, {"B", 20}}; + std::vector> results = { + {"A", 5}, {"B", 10} + }; + + auto orig = update_standings_original(existing, results); + auto patched = update_standings_patched(existing, results); + + assert(orig.size() == 2); + assert(orig[0].points == 15); + assert(orig[1].points == 30); + assert(orig.size() == patched.size()); + for (size_t i = 0; i < orig.size(); i++) { + assert(orig[i].drvName == patched[i].drvName); + assert(orig[i].points == patched[i].points); + } + printf("PASS: all existing drivers\n"); + } + + // Test 4: Performance at scale (N=500 drivers, R=200 race results) + { + const int N = 500; + const int R = 200; + + std::vector existing; + for (int i = 0; i < N; i++) { + tReStandings st; + st.drvName = "Driver_" + std::to_string(i); + st.points = i * 10; + existing.push_back(st); + } + + std::vector> results; + for (int i = 0; i < R; i++) + results.push_back({"Driver_" + std::to_string(i % N), 5}); + + // Warm up + update_standings_original(existing, results); + update_standings_patched(existing, results); + + const int ITERS = 500; + + auto t0 = std::chrono::high_resolution_clock::now(); + for (int i = 0; i < ITERS; i++) + update_standings_original(existing, results); + auto t1 = std::chrono::high_resolution_clock::now(); + for (int i = 0; i < ITERS; i++) + update_standings_patched(existing, results); + auto t2 = std::chrono::high_resolution_clock::now(); + + double orig_us = std::chrono::duration_cast(t1 - t0).count(); + double patched_us = std::chrono::duration_cast(t2 - t1).count(); + double ratio = orig_us / patched_us; + + printf("PASS: performance N=%d R=%d: original=%.0fus patched=%.0fus ratio=%.1fx\n", + N, R, orig_us, patched_us, ratio); + + assert(ratio > 2.0); + } + + printf("ALL TESTS PASSED\n"); + return 0; +} diff --git a/defects/speed-dreams-0002/patch/speed-dreams-0002.patch b/defects/speed-dreams-0002/patch/speed-dreams-0002.patch new file mode 100644 index 000000000..45146258e --- /dev/null +++ b/defects/speed-dreams-0002/patch/speed-dreams-0002.patch @@ -0,0 +1,20 @@ +--- a/src/modules/userinterface/legacymenu/racescreens/driverselect.cpp ++++ b/src/modules/userinterface/legacymenu/racescreens/driverselect.cpp +@@ -1164,13 +1164,17 @@ + + // d) Keep only drivers accepted by the race and not already among competitors + // (but don't reject humans with the wrong car category : they must be able to change it). ++ // Build a set for O(1) competitor lookup instead of O(N) linear scan. ++ std::set setCompetitors(vecCompetitors.begin(), vecCompetitors.end()); ++ + std::vector::const_iterator itCandidate; + for (itCandidate = vecCandidates.begin(); itCandidate != vecCandidates.end(); ++itCandidate) + { +- if (std::find(vecCompetitors.begin(), vecCompetitors.end(), *itCandidate) +- == vecCompetitors.end() ++ if (setCompetitors.find(*itCandidate) == setCompetitors.end() + && MenuData->pRace->acceptsDriverType((*itCandidate)->getType()) + && (strCarModel == AnyCarModel + || (*itCandidate)->getCar()->getId() == strCarModel) + && ((*itCandidate)->isHuman() + || MenuData->pRace->acceptsCarCategory((*itCandidate)->getCar()->getCategoryId()))) diff --git a/defects/speed-dreams-0002/test/test_driver_filter.cpp b/defects/speed-dreams-0002/test/test_driver_filter.cpp new file mode 100644 index 000000000..d3760ff1b --- /dev/null +++ b/defects/speed-dreams-0002/test/test_driver_filter.cpp @@ -0,0 +1,92 @@ +// Unit test for speed-dreams-0002: driverselect.cpp competitor filter +// std::find O(candidates * competitors) -> std::set O(candidates * log(competitors)) + +#include +#include +#include +#include +#include +#include + +// Simulated driver pointer (just an int address) +using Driver = int; + +// Original: O(C * N) linear scan +static std::vector filter_original( + const std::vector &candidates, + const std::vector &competitors) +{ + std::vector result; + for (auto c : candidates) { + if (std::find(competitors.begin(), competitors.end(), c) == competitors.end()) + result.push_back(c); + } + return result; +} + +// Patched: O(C * log N) set lookup +static std::vector filter_patched( + const std::vector &candidates, + const std::vector &competitors) +{ + std::set setCompetitors(competitors.begin(), competitors.end()); + std::vector result; + for (auto c : candidates) { + if (setCompetitors.find(c) == setCompetitors.end()) + result.push_back(c); + } + return result; +} + +int main() +{ + // Allocate driver objects + const int TOTAL = 500; + const int COMP = 200; + std::vector pool(TOTAL); + std::vector candidates, competitors; + + for (int i = 0; i < TOTAL; i++) + candidates.push_back(&pool[i]); + + for (int i = 0; i < COMP; i++) + competitors.push_back(&pool[i * 2]); // every other driver + + // Test 1: Correctness + { + auto orig = filter_original(candidates, competitors); + auto patched = filter_patched(candidates, competitors); + + assert(orig.size() == patched.size()); + for (size_t i = 0; i < orig.size(); i++) + assert(orig[i] == patched[i]); + + printf("PASS: correctness (kept %zu of %d candidates, %d competitors)\n", + orig.size(), TOTAL, COMP); + } + + // Test 2: Performance + { + const int ITERS = 2000; + + auto t0 = std::chrono::high_resolution_clock::now(); + for (int i = 0; i < ITERS; i++) + filter_original(candidates, competitors); + auto t1 = std::chrono::high_resolution_clock::now(); + for (int i = 0; i < ITERS; i++) + filter_patched(candidates, competitors); + auto t2 = std::chrono::high_resolution_clock::now(); + + double orig_us = std::chrono::duration_cast(t1 - t0).count(); + double patched_us = std::chrono::duration_cast(t2 - t1).count(); + double ratio = orig_us / patched_us; + + printf("PASS: performance C=%d N=%d: original=%.0fus patched=%.0fus ratio=%.1fx\n", + TOTAL, COMP, orig_us, patched_us, ratio); + + assert(ratio > 1.5); + } + + printf("ALL TESTS PASSED\n"); + return 0; +} diff --git a/defects/speed-dreams-0003/patch/speed-dreams-0003.patch b/defects/speed-dreams-0003/patch/speed-dreams-0003.patch new file mode 100644 index 000000000..95dc349e6 --- /dev/null +++ b/defects/speed-dreams-0003/patch/speed-dreams-0003.patch @@ -0,0 +1,71 @@ +--- a/src/libs/tgfdata/racemanagers.cpp ++++ b/src/libs/tgfdata/racemanagers.cpp +@@ -245,6 +245,8 @@ + GfRaceManager::GfRaceManager(const std::string& strId, void* hparmHandle) + { + _strId = strId; ++ std::set seenDriverTypes; ++ std::set seenCarCategories; + + // Load constant properties (never changed afterwards). + // 1) Name, type, sub-type and priority (ordering the buttons in the race select menu). +@@ -261,9 +263,9 @@ + std::string strDrvType; + while(std::getline(ssAcceptDrvTypes, strDrvType, cFilterSeparator)) + { +- std::vector::iterator itDrvType = +- std::find(_vecAcceptedDriverTypes.begin(), _vecAcceptedDriverTypes.end(), strDrvType); +- if (itDrvType == _vecAcceptedDriverTypes.end()) // Not already there => store it. ++ if (seenDriverTypes.find(strDrvType) == seenDriverTypes.end()) // Not already there => store it. ++ { ++ seenDriverTypes.insert(strDrvType); + _vecAcceptedDriverTypes.push_back(strDrvType); ++ } + } + +@@ -278,9 +280,10 @@ + std::stringstream ssRejectDrvTypes(pszRejectDrvTypes); + while(std::getline(ssRejectDrvTypes, strDrvType, cFilterSeparator)) + { +- std::vector::iterator itDrvType = +- std::find(_vecAcceptedDriverTypes.begin(), _vecAcceptedDriverTypes.end(), strDrvType); +- if (itDrvType != _vecAcceptedDriverTypes.end()) // Accepted til now => now rejected. ++ auto itDrvType = std::find(_vecAcceptedDriverTypes.begin(), ++ _vecAcceptedDriverTypes.end(), strDrvType); ++ if (itDrvType != _vecAcceptedDriverTypes.end()) ++ { ++ seenDriverTypes.erase(strDrvType); + _vecAcceptedDriverTypes.erase(itDrvType); ++ } + } + +@@ -293,9 +296,9 @@ + std::string strCarCat; + while(std::getline(ssAcceptCarCats, strCarCat, cFilterSeparator)) + { +- std::vector::iterator itCarCat = +- std::find(_vecAcceptedCarCategoryIds.begin(), _vecAcceptedCarCategoryIds.end(), strCarCat); +- if (itCarCat == _vecAcceptedCarCategoryIds.end()) // Not already there => store it. ++ if (seenCarCategories.find(strCarCat) == seenCarCategories.end()) ++ { ++ seenCarCategories.insert(strCarCat); + _vecAcceptedCarCategoryIds.push_back(strCarCat); ++ } + } + +@@ -310,9 +313,10 @@ + std::stringstream ssRejectCarCats(pszRejectCarCats); + while(std::getline(ssRejectCarCats, strCarCat, cFilterSeparator)) + { +- std::vector::iterator itCarCat = +- std::find(_vecAcceptedCarCategoryIds.begin(), _vecAcceptedCarCategoryIds.end(), strCarCat); +- if (itCarCat != _vecAcceptedCarCategoryIds.end()) // Accepted til now => now rejected. ++ auto itCarCat = std::find(_vecAcceptedCarCategoryIds.begin(), ++ _vecAcceptedCarCategoryIds.end(), strCarCat); ++ if (itCarCat != _vecAcceptedCarCategoryIds.end()) ++ { ++ seenCarCategories.erase(strCarCat); + _vecAcceptedCarCategoryIds.erase(itCarCat); ++ } + } + diff --git a/defects/speed-dreams-0003/test/test_racemanager_dedup.cpp b/defects/speed-dreams-0003/test/test_racemanager_dedup.cpp new file mode 100644 index 000000000..7c393ef3c --- /dev/null +++ b/defects/speed-dreams-0003/test/test_racemanager_dedup.cpp @@ -0,0 +1,108 @@ +// Unit test for speed-dreams-0003: GfRaceManager constructor dedup +// std::find on vector for dedup O(N^2) -> std::set O(N log N) + +#include +#include +#include +#include +#include +#include +#include + +// Original: O(N^2) dedup with linear scan +static std::vector dedup_original(const std::vector &input) { + std::vector result; + for (const auto &s : input) { + if (std::find(result.begin(), result.end(), s) == result.end()) + result.push_back(s); + } + return result; +} + +// Patched: O(N log N) dedup with set +static std::vector dedup_patched(const std::vector &input) { + std::vector result; + std::set seen; + for (const auto &s : input) { + if (seen.find(s) == seen.end()) { + seen.insert(s); + result.push_back(s); + } + } + return result; +} + +int main() +{ + // Test 1: Basic correctness + { + std::vector input = {"a", "b", "a", "c", "b", "d", "a"}; + auto orig = dedup_original(input); + auto patched = dedup_patched(input); + + assert(orig.size() == patched.size()); + for (size_t i = 0; i < orig.size(); i++) + assert(orig[i] == patched[i]); + + assert(orig.size() == 4); + assert(orig[0] == "a"); + assert(orig[1] == "b"); + assert(orig[2] == "c"); + assert(orig[3] == "d"); + printf("PASS: basic correctness\n"); + } + + // Test 2: No duplicates + { + std::vector input = {"x", "y", "z"}; + auto orig = dedup_original(input); + auto patched = dedup_patched(input); + assert(orig.size() == 3); + assert(orig.size() == patched.size()); + printf("PASS: no duplicates\n"); + } + + // Test 3: All duplicates + { + std::vector input = {"a", "a", "a", "a"}; + auto orig = dedup_original(input); + auto patched = dedup_patched(input); + assert(orig.size() == 1); + assert(orig.size() == patched.size()); + printf("PASS: all duplicates\n"); + } + + // Test 4: Performance at scale N=1000 with 50% duplicates + { + const int N = 1000; + std::vector input; + for (int i = 0; i < N; i++) + input.push_back("type_" + std::to_string(i % (N / 2))); + + auto orig = dedup_original(input); + auto patched = dedup_patched(input); + assert(orig.size() == patched.size()); + + const int ITERS = 500; + + auto t0 = std::chrono::high_resolution_clock::now(); + for (int i = 0; i < ITERS; i++) + dedup_original(input); + auto t1 = std::chrono::high_resolution_clock::now(); + for (int i = 0; i < ITERS; i++) + dedup_patched(input); + auto t2 = std::chrono::high_resolution_clock::now(); + + double orig_us = std::chrono::duration_cast(t1 - t0).count(); + double patched_us = std::chrono::duration_cast(t2 - t1).count(); + double ratio = orig_us / patched_us; + + printf("PASS: performance N=%d: original=%.0fus patched=%.0fus ratio=%.1fx\n", + N, orig_us, patched_us, ratio); + + assert(ratio > 2.0); + } + + printf("ALL TESTS PASSED\n"); + return 0; +} diff --git a/defects/speed-dreams-0004/patch/speed-dreams-0004.patch b/defects/speed-dreams-0004/patch/speed-dreams-0004.patch new file mode 100644 index 000000000..388e91e92 --- /dev/null +++ b/defects/speed-dreams-0004/patch/speed-dreams-0004.patch @@ -0,0 +1,20 @@ +--- a/src/modules/userinterface/legacymenu/racescreens/humanselect.cpp ++++ b/src/modules/userinterface/legacymenu/racescreens/humanselect.cpp +@@ -224,12 +224,15 @@ + std::vector drivers = + GfDrivers::self()->getDriversWithTypeAndCategory(ROB_VAL_HUMAN); + ++ // Build a set from staging for O(1) lookup instead of O(S) linear scan per driver. ++ std::unordered_set stagingSet(staging.cbegin(), staging.cend()); ++ + i = 0; + + for (auto driver : drivers) + { + const char *name = driver->getName().c_str(); + +- if (std::find(staging.cbegin(), staging.cend(), name) == staging.end()) ++ if (stagingSet.find(name) == stagingSet.end()) + GfuiScrollListInsertElement(hscr, available, name, i++, (void *)name); + } + diff --git a/defects/speed-dreams-0004/test/test_humanselect_staging.cpp b/defects/speed-dreams-0004/test/test_humanselect_staging.cpp new file mode 100644 index 000000000..2e5de2333 --- /dev/null +++ b/defects/speed-dreams-0004/test/test_humanselect_staging.cpp @@ -0,0 +1,117 @@ +// Unit test for speed-dreams-0004: humanselect.cpp update_ui staging lookup +// std::find on staging vector O(D*S) -> unordered_set O(D) + +#include +#include +#include +#include +#include +#include +#include + +// Original: O(D * S) linear scan of staging per driver +static std::vector filter_available_original( + const std::vector &drivers, + const std::vector &staging) +{ + std::vector result; + for (const auto &name : drivers) { + if (std::find(staging.cbegin(), staging.cend(), name) == staging.cend()) + result.push_back(name); + } + return result; +} + +// Patched: O(D + S) hash set lookup +static std::vector filter_available_patched( + const std::vector &drivers, + const std::vector &staging) +{ + std::unordered_set stagingSet(staging.cbegin(), staging.cend()); + std::vector result; + for (const auto &name : drivers) { + if (stagingSet.find(name) == stagingSet.end()) + result.push_back(name); + } + return result; +} + +int main() +{ + // Test 1: Correctness + { + std::vector drivers = {"Alice", "Bob", "Carol", "Dave", "Eve"}; + std::vector staging = {"Bob", "Dave"}; + + auto orig = filter_available_original(drivers, staging); + auto patched = filter_available_patched(drivers, staging); + + assert(orig.size() == 3); + assert(orig.size() == patched.size()); + for (size_t i = 0; i < orig.size(); i++) + assert(orig[i] == patched[i]); + + printf("PASS: correctness\n"); + } + + // Test 2: Empty staging + { + std::vector drivers = {"A", "B"}; + std::vector staging; + + auto orig = filter_available_original(drivers, staging); + auto patched = filter_available_patched(drivers, staging); + assert(orig.size() == 2); + assert(orig.size() == patched.size()); + printf("PASS: empty staging\n"); + } + + // Test 3: All staged + { + std::vector drivers = {"A", "B"}; + std::vector staging = {"A", "B"}; + + auto orig = filter_available_original(drivers, staging); + auto patched = filter_available_patched(drivers, staging); + assert(orig.empty()); + assert(patched.empty()); + printf("PASS: all staged\n"); + } + + // Test 4: Performance D=500 S=200 + { + const int D = 500, S = 200; + std::vector drivers, staging; + + for (int i = 0; i < D; i++) + drivers.push_back("Driver_" + std::to_string(i)); + for (int i = 0; i < S; i++) + staging.push_back("Driver_" + std::to_string(i * 2)); + + auto orig = filter_available_original(drivers, staging); + auto patched = filter_available_patched(drivers, staging); + assert(orig.size() == patched.size()); + + const int ITERS = 2000; + + auto t0 = std::chrono::high_resolution_clock::now(); + for (int i = 0; i < ITERS; i++) + filter_available_original(drivers, staging); + auto t1 = std::chrono::high_resolution_clock::now(); + for (int i = 0; i < ITERS; i++) + filter_available_patched(drivers, staging); + auto t2 = std::chrono::high_resolution_clock::now(); + + double orig_us = std::chrono::duration_cast(t1 - t0).count(); + double patched_us = std::chrono::duration_cast(t2 - t1).count(); + double ratio = orig_us / patched_us; + + printf("PASS: performance D=%d S=%d: original=%.0fus patched=%.0fus ratio=%.1fx\n", + D, S, orig_us, patched_us, ratio); + + assert(ratio > 2.0); + } + + printf("ALL TESTS PASSED\n"); + return 0; +} diff --git a/defects/speed-dreams-0005/patch/speed-dreams-0005.patch b/defects/speed-dreams-0005/patch/speed-dreams-0005.patch new file mode 100644 index 000000000..6940f005d --- /dev/null +++ b/defects/speed-dreams-0005/patch/speed-dreams-0005.patch @@ -0,0 +1,45 @@ +--- a/src/libs/tgfdata/cars.cpp ++++ b/src/libs/tgfdata/cars.cpp +@@ -82,6 +82,7 @@ + void GfCars::list(const std::string &path) + { + tFList* lstFolders = GfDirGetList(path.c_str()); ++ std::set seenCatIds(_pPrivate->vecCatIds.begin(), _pPrivate->vecCatIds.end()); + + if (!lstFolders) + { +@@ -146,9 +147,11 @@ + // Update the GfCars singleton. + _pPrivate->vecCars.push_back(pCar); + _pPrivate->mapCarsById[pszCarId] = pCar; +- if (std::find(_pPrivate->vecCatIds.begin(), _pPrivate->vecCatIds.end(), strCatId) +- == _pPrivate->vecCatIds.end()) ++ if (seenCatIds.find(strCatId) == seenCatIds.end()) + { ++ seenCatIds.insert(strCatId); + _pPrivate->vecCatIds.push_back(strCatId); + _pPrivate->vecCatNames.push_back(strCatName); + } +--- a/src/libs/tgfdata/drivers.cpp ++++ b/src/libs/tgfdata/drivers.cpp +@@ -560,6 +560,7 @@ + int GfDrivers::load() + { ++ std::set seenCarCategoryIds; + + // List the robot shared libraries in the standard folder. + tModList *lstDriverModules = NULL; +@@ -633,8 +634,10 @@ + _pPrivate->mapDriversByKey[driverKey] = pDriver; + +- if (std::find(_pPrivate->vecCarCategoryIds.begin(), _pPrivate->vecCarCategoryIds.end(), +- pDriver->getCar()->getCategoryId()) == _pPrivate->vecCarCategoryIds.end()) ++ const std::string &catId = pDriver->getCar()->getCategoryId(); ++ if (seenCarCategoryIds.find(catId) == seenCarCategoryIds.end()) ++ { ++ seenCarCategoryIds.insert(catId); + _pPrivate->vecCarCategoryIds.push_back(pDriver->getCar()->getCategoryId()); ++ } + } + else + diff --git a/defects/speed-dreams-0005/test/test_cars_catids.py b/defects/speed-dreams-0005/test/test_cars_catids.py new file mode 100644 index 000000000..77ca167ec --- /dev/null +++ b/defects/speed-dreams-0005/test/test_cars_catids.py @@ -0,0 +1,55 @@ +""" +speed-dreams-0005 — MOAD-0001 (CWE-407) test +GfCars::list() calls std::find on vecCatIds for every car loaded, O(C*N). +Fix: seed a std::set at the start of list() and use set::find O(log C). +""" +import re +import sys +import os + +BASE = os.path.dirname(os.path.abspath(__file__)) +CARS_CPP = os.path.normpath( + os.path.join(BASE, "../../../../speed-dreams/src/libs/tgfdata/cars.cpp") +) + + +def read_source(): + with open(CARS_CPP) as f: + return f.read() + + +def test_no_raw_std_find_in_list(): + """ + After patch, the list() function must not call std::find on vecCatIds + without a set guard. We confirm the set is seeded before the loop. + """ + src = read_source() + list_start = src.find("void GfCars::list(") + assert list_start != -1, "GfCars::list not found" + list_body = src[list_start: list_start + 2000] + + # Patched: seenCatIds set is constructed before the do-while loop. + has_set = "seenCatIds" in list_body or "set" in list_body or \ + "std::set" in list_body + # Unpatched: direct std::find on vecCatIds inside the loop. + raw_find = bool(re.search( + r'std::find\s*\(\s*_pPrivate->vecCatIds', list_body + )) + assert has_set or not raw_find, ( + "DEFECT PRESENT: GfCars::list() still uses std::find on vecCatIds " + "without a set guard — O(C*N) category dedup." + ) + print("PASS: GfCars::list() category dedup is guarded (set or no raw find).") + + +if __name__ == "__main__": + failures = 0 + for test in [test_no_raw_std_find_in_list]: + try: + test() + except AssertionError as e: + print(f"FAIL: {test.__name__}: {e}") + failures += 1 + except FileNotFoundError: + print(f"SKIP: {test.__name__}: source not found at {CARS_CPP}") + sys.exit(failures) diff --git a/defects/speed-dreams-0006/patch/speed-dreams-0006.patch b/defects/speed-dreams-0006/patch/speed-dreams-0006.patch new file mode 100644 index 000000000..44b883e6a --- /dev/null +++ b/defects/speed-dreams-0006/patch/speed-dreams-0006.patch @@ -0,0 +1,21 @@ +diff --git a/src/libs/tgfclient/webserver.cpp b/src/libs/tgfclient/webserver.cpp +index 8863236..26d73f7 100644 +--- a/src/libs/tgfclient/webserver.cpp ++++ b/src/libs/tgfclient/webserver.cpp +@@ -296,7 +296,15 @@ int WebServer::updateAsyncStatus() + + int WebServer::addAsyncRequest(const std::string &data) + { +- GfLogInfo("WebServer: Performing ASYNC request:\n%s\n", data.c_str()); ++ // CWE-312: Never log request bodies verbatim — they may contain ++ // credentials. The login request embeds and in ++ // plain XML, so logging data.c_str() exposes them in the game log file. ++ // Log only the request size instead. ++ GfLogInfo("WebServer: Performing ASYNC request (%zu bytes)\n", data.size()); ++#ifdef SD_WEBSERVER_DEBUG_VERBOSE ++ // Only compiled in explicit debug builds — never in release. ++ GfLogDebug("WebServer: ASYNC request body:\n%s\n", data.c_str()); ++#endif + + //read the webserver configuration + this->readConfiguration(); diff --git a/defects/speed-dreams-0006/test/test_speed_dreams_0006.py b/defects/speed-dreams-0006/test/test_speed_dreams_0006.py new file mode 100644 index 000000000..54a70284a --- /dev/null +++ b/defects/speed-dreams-0006/test/test_speed_dreams_0006.py @@ -0,0 +1,81 @@ +""" +speed-dreams-0006 — MOAD-0004 (CWE-312) test +WebServer::addAsyncRequest logs full XML request body, exposing plaintext +username and password in the game log file. + +This test checks the source directly: the patched line must not pass +data.c_str() to any logging macro; only data.size() is permitted. +""" +import re +import sys +import os + +WEBSERVER_CPP = os.path.join( + os.path.dirname(__file__), + "../../../..", # repo root of speed-dreams clone + "src/libs/tgfclient/webserver.cpp", +) + +# Resolve relative to this test file's location. +BASE = os.path.dirname(os.path.abspath(__file__)) +WEBSERVER_CPP = os.path.normpath( + os.path.join(BASE, "../../../../speed-dreams/src/libs/tgfclient/webserver.cpp") +) + + +def read_source(): + with open(WEBSERVER_CPP, "r") as f: + return f.read() + + +def test_no_plain_data_log(): + """After patch: addAsyncRequest must not log data.c_str() unconditionally.""" + src = read_source() + + # Find the addAsyncRequest function body. + # We look for GfLogInfo / GfLogTrace / GfLogDebug outside of #ifdef guard. + func_start = src.find("int WebServer::addAsyncRequest") + assert func_start != -1, "addAsyncRequest not found in source" + # Grab up to next top-level function (simple heuristic: next blank line + "int ") + func_body = src[func_start: func_start + 800] + + # The vulnerable pattern: unconditional GfLog* call with data.c_str() + vulnerable = re.search( + r'GfLog(?:Info|Trace|Debug|Warn)\s*\([^)]*data\.c_str\(\)', + func_body, + ) + if vulnerable: + # Check it is NOT inside an #ifdef SD_WEBSERVER_DEBUG_VERBOSE guard. + context = func_body[max(0, vulnerable.start() - 200): vulnerable.start()] + assert "#ifdef SD_WEBSERVER_DEBUG_VERBOSE" in context, ( + "DEFECT PRESENT: addAsyncRequest logs data.c_str() unconditionally.\n" + "This exposes username/password in XML tag to the log file.\n" + f"Matched: {vulnerable.group()}" + ) + # If no match, patch is applied and test passes. + print("PASS: addAsyncRequest does not log data.c_str() unconditionally.") + + +def test_size_log_present(): + """After patch: addAsyncRequest should log data.size() for traceability.""" + src = read_source() + func_start = src.find("int WebServer::addAsyncRequest") + assert func_start != -1 + func_body = src[func_start: func_start + 800] + assert "data.size()" in func_body, ( + "Expected data.size() log after patch is not present." + ) + print("PASS: addAsyncRequest logs data.size() (non-sensitive).") + + +if __name__ == "__main__": + failures = 0 + for test in [test_no_plain_data_log, test_size_log_present]: + try: + test() + except AssertionError as e: + print(f"FAIL: {test.__name__}: {e}") + failures += 1 + except FileNotFoundError: + print(f"SKIP: {test.__name__}: speed-dreams source not found at {WEBSERVER_CPP}") + sys.exit(failures)