diff --git a/defects/dolibarr/MOAD-0001-0002-0003-0005-partial-CLEAN.md b/defects/dolibarr/MOAD-0001-0002-0003-0005-partial-CLEAN.md new file mode 100644 index 000000000..ba8934536 --- /dev/null +++ b/defects/dolibarr/MOAD-0001-0002-0003-0005-partial-CLEAN.md @@ -0,0 +1,42 @@ +# Dolibarr — MOAD scan summary + +**Date:** 2026-03-31 +**Target:** https://github.com/Dolibarr/dolibarr (PHP ERP) +**Scanner:** Agent Blackops 5-MOAD sweep +**Note:** dolibarr-0001 through dolibarr-0005 patched. dolibarr-0004 and dolibarr-0005 are new CWE-312 defects. + +## MOAD-0001 (CWE-407): 3 defects found (pre-existing) + +- **dolibarr-0001**: `filecheck.php` — `in_array($tmprelativefilename, $file_list['insignature'])` — O(S*I), 83.5x +- **dolibarr-0002**: `datapolicycron.class.php` — `in_array($obj->rowid, $processedIds)` growing dedup — O(N^2), 23.8x +- **dolibarr-0003**: `bookkeeping.class.php` — `in_array($bookKeeping->piece_num, $alreadyExtourneT)` — O(P*E*A), 39.9x + +No new MOAD-0001 defects found in product/invoice/contact core classes. All remaining +`in_array` calls in hot paths operate on small UI selection arrays (`$arrayofselected` +from POST) or fixed-size type lists — not algorithmic hot paths. + +## MOAD-0002 (Intertangle): CLEAN (architectural PHP pattern) + +Dolibarr uses `global $conf, $db, $langs, $user` throughout. This is the framework-level +architecture — all modules share these globals intentionally. No single discrete patchable +coupling found beyond the intentional design. + +## MOAD-0003 (Leaked Context): CLEAN (standard PHP session pattern) + +User identity stored in `$_SESSION` and loaded into `global $user`. Standard PHP architecture. +No ThreadLocal misuse; PHP is single-threaded per request. + +## MOAD-0004 (CWE-312): 2 defects found + +- **dolibarr-0004**: `htdocs/admin/emailcollector_card.php` line 575 — IMAP password logged verbatim + unconditionally: `dol_syslog("... password=".$object->password...)`. Severity: HIGH. Fix: replace + `$object->password` with literal `***` in log string. +- **dolibarr-0005**: `htdocs/core/login/functions_ldap.php` line 98 — LDAP admin `searchPassword` + first 3 chars logged via `dol_trunc($ldap->searchPassword, 3)`, gated on `$ldapdebug` flag AND + also printed to browser output. Severity: MEDIUM (debug-gated). Fix: replace `dol_trunc(...)` with + literal `***` in both `dol_syslog()` and `print` calls. + +## MOAD-0005 (Thundering Herd): CLEAN + +Dolibarr uses PHP in-process caching (`$conf->cache_*` arrays). PHP single-threaded per +request. No distributed atomic cache patterns needed. No get+null+compute+set race possible. diff --git a/defects/dolibarr/patch/dolibarr-0004-emailcollector-imap-password-logged.patch b/defects/dolibarr/patch/dolibarr-0004-emailcollector-imap-password-logged.patch new file mode 100644 index 000000000..4ce5a9bb9 --- /dev/null +++ b/defects/dolibarr/patch/dolibarr-0004-emailcollector-imap-password-logged.patch @@ -0,0 +1,11 @@ +--- a/htdocs/admin/emailcollector_card.php ++++ b/htdocs/admin/emailcollector_card.php +@@ -572,7 +572,7 @@ + $timeoutconnect = getDolGlobalInt('MAIN_USE_CONNECT_TIMEOUT', 5); + $timeoutread = getDolGlobalInt('MAIN_USE_RESPONSE_TIMEOUT', 20); + +- dol_syslog("imap_open connectstring=".$connectstringsource." login=".$object->login." password=".$object->password." timeoutconnect=".$timeoutconnect." timeoutread=".$timeoutread); ++ dol_syslog("imap_open connectstring=".$connectstringsource." login=".$object->login." password=*** timeoutconnect=".$timeoutconnect." timeoutread=".$timeoutread); + + $result1 = imap_timeout(IMAP_OPENTIMEOUT, $timeoutconnect); // timeout seems ignored with ssl connect + $result2 = imap_timeout(IMAP_READTIMEOUT, $timeoutread); diff --git a/defects/dolibarr/patch/dolibarr-0005-ldap-searchpassword-logged.patch b/defects/dolibarr/patch/dolibarr-0005-ldap-searchpassword-logged.patch new file mode 100644 index 000000000..125934e35 --- /dev/null +++ b/defects/dolibarr/patch/dolibarr-0005-ldap-searchpassword-logged.patch @@ -0,0 +1,11 @@ +--- a/htdocs/core/login/functions_ldap.php ++++ b/htdocs/core/login/functions_ldap.php +@@ -95,9 +95,9 @@ + if ($ldapdebug) { + dol_syslog("functions_ldap::check_user_password_ldap Server:".implode(',', $ldap->server).", Port:".$ldap->serverPort.", Protocol:".$ldap->ldapProtocolVersion.", Type:".$ldap->serverType); +- dol_syslog("functions_ldap::check_user_password_ldap uid/samaccountname=".$ldapuserattr.", dn=".$ldapdn.", Admin:".$ldap->searchUser.", Pass:".dol_trunc($ldap->searchPassword, 3)); ++ dol_syslog("functions_ldap::check_user_password_ldap uid/samaccountname=".$ldapuserattr.", dn=".$ldapdn.", Admin:".$ldap->searchUser.", Pass:***"); + print "DEBUG: Server:".implode(',', $ldap->server).", Port:".$ldap->serverPort.", Protocol:".$ldap->ldapProtocolVersion.", Type:".$ldap->serverType."
\n"; +- print "DEBUG: uid/samaccountname=".$ldapuserattr.", dn=".$ldapdn.", Admin:".$ldap->searchUser.", Pass:".dol_trunc($ldap->searchPassword, 3)."
\n"; ++ print "DEBUG: uid/samaccountname=".$ldapuserattr.", dn=".$ldapdn.", Admin:".$ldap->searchUser.", Pass:***
\n"; + } diff --git a/defects/dolibarr/test/DolibarrCWE407Test.class b/defects/dolibarr/test/DolibarrCWE407Test.class index daaa80331..5eafc5cb0 100644 Binary files a/defects/dolibarr/test/DolibarrCWE407Test.class and b/defects/dolibarr/test/DolibarrCWE407Test.class differ diff --git a/defects/dolibarr/test/DolibarrCWE407Test.java b/defects/dolibarr/test/DolibarrCWE407Test.java index d3ead1afb..0227012a6 100644 --- a/defects/dolibarr/test/DolibarrCWE407Test.java +++ b/defects/dolibarr/test/DolibarrCWE407Test.java @@ -1,7 +1,7 @@ import java.util.*; /** - * CWE-407 unit tests for Dolibarr defects. + * Unit tests for Dolibarr defects (CWE-407 and CWE-312). * * dolibarr-0001: filecheck.php in_array($tmprelativefilename, $file_list['insignature']) * — O(S*I) file membership check during integrity verification @@ -9,6 +9,10 @@ import java.util.*; * — O(N^2) dedup accumulator in GDPR data policy cron * dolibarr-0003: bookkeeping.class.php in_array($bookKeeping->piece_num, $alreadyExtourneT) * — O(P*E*A) reversal check in accounting journal extourne + * dolibarr-0004: emailcollector_card.php dol_syslog(...password=$object->password...) + * — CWE-312: IMAP password logged verbatim to system log + * dolibarr-0005: functions_ldap.php dol_syslog(...Pass:dol_trunc($ldap->searchPassword, 3)) + * — CWE-312: LDAP admin searchPassword first 3 chars logged (debug-gated but still leaks) */ public class DolibarrCWE407Test { @@ -208,10 +212,96 @@ public class DolibarrCWE407Test { System.out.println(" PASS"); } + // ---- dolibarr-0004: IMAP password logged verbatim (CWE-312) ---- + + /** + * Models the log message builder for emailcollector_card.php line 575. + * Unpatched: concatenates $object->password verbatim into the log string. + * Patched: replaces password with literal "***". + */ + static String imapLogUnpatched(String connectstring, String login, String password) { + return "imap_open connectstring=" + connectstring + + " login=" + login + + " password=" + password; + } + + static String imapLogPatched(String connectstring, String login, String password) { + return "imap_open connectstring=" + connectstring + + " login=" + login + + " password=***"; + } + + // ---- dolibarr-0005: LDAP searchPassword first 3 chars logged (CWE-312) ---- + + /** Simulates dol_trunc(password, 3): returns first 3 chars + "..." */ + static String dolTrunc(String s, int n) { + if (s == null || s.length() <= n) return s; + return s.substring(0, n) + "..."; + } + + static String ldapLogUnpatched(String searchUser, String searchPassword) { + return "Admin:" + searchUser + ", Pass:" + dolTrunc(searchPassword, 3); + } + + static String ldapLogPatched(String searchUser, String searchPassword) { + return "Admin:" + searchUser + ", Pass:***"; + } + + static void testDolibarr0004() { + System.out.println("=== dolibarr-0004: IMAP password verbatim in log (CWE-312) ==="); + String connectstring = "{imap.example.com:993/imap/ssl}INBOX"; + String login = "user@example.com"; + String password = "S3cr3tP@ss!"; + + String unpatched = imapLogUnpatched(connectstring, login, password); + String patched = imapLogPatched(connectstring, login, password); + + // Unpatched log contains the real password + assert unpatched.contains(password) : "Expected unpatched log to contain password"; + + // Patched log must NOT contain the real password + assert !patched.contains(password) : "Patched log must not contain password"; + assert patched.contains("***") : "Patched log must contain redaction marker"; + + // Both logs contain the login (not a secret) + assert unpatched.contains(login) : "Log must contain login"; + assert patched.contains(login) : "Log must contain login"; + + System.out.println(" unpatched log: " + unpatched); + System.out.println(" patched log: " + patched); + System.out.println(" PASS"); + } + + static void testDolibarr0005() { + System.out.println("=== dolibarr-0005: LDAP searchPassword partial leak in log (CWE-312) ==="); + String searchUser = "cn=ldapadmin,dc=example,dc=com"; + String searchPassword = "S3cr3tAdminPass"; + + String unpatched = ldapLogUnpatched(searchUser, searchPassword); + String patched = ldapLogPatched(searchUser, searchPassword); + + // Unpatched log leaks first 3 chars of password + String leaked = searchPassword.substring(0, 3); + assert unpatched.contains(leaked) : "Unpatched log should contain first 3 chars: " + leaked; + + // Patched log must NOT contain any prefix of the password + assert !patched.contains(leaked) : "Patched log must not leak any password chars"; + assert patched.contains("***") : "Patched log must contain redaction marker"; + + // Both logs contain the admin DN (not a secret per se, but identity) + assert patched.contains(searchUser) : "Log should still contain admin user DN"; + + System.out.println(" unpatched log: " + unpatched); + System.out.println(" patched log: " + patched); + System.out.println(" PASS"); + } + public static void main(String[] args) { testDolibarr0001(); testDolibarr0002(); testDolibarr0003(); - System.out.println("\nAll 3 Dolibarr CWE-407 tests PASSED."); + testDolibarr0004(); + testDolibarr0005(); + System.out.println("\nAll 5 Dolibarr tests PASSED."); } } diff --git a/defects/suitecrm/MOAD-0002-0003-0005-CLEAN.md b/defects/suitecrm/MOAD-0002-0003-0005-CLEAN.md new file mode 100644 index 000000000..4a905a199 --- /dev/null +++ b/defects/suitecrm/MOAD-0002-0003-0005-CLEAN.md @@ -0,0 +1,43 @@ +# SuiteCRM — MOADs 0002, 0003, 0005 CLEAN + +**Date:** 2026-03-31 +**Target:** https://github.com/salesagility/SuiteCRM (PHP CRM) +**Scanner:** Agent Blackops 5-MOAD sweep +**Note:** suitecrm-0001 through suitecrm-0004 are CWE-407 patches. suitecrm-0004 is new (SugarBean subpanel field dedup). + +## MOAD-0001 (CWE-407): 4 defects found + +- **suitecrm-0001**: `InboundEmail.php` — `in_array($overview->imap_uid, $uids)` — O(I*U) +- **suitecrm-0002**: `jjwg_Maps/controller.php` — `in_array($display['id'], $records)` — O(D*R) +- **suitecrm-0003**: `Email.php` — `in_array(trim($match[0]), $knownEmails)` growing dedup — O(N^2) +- **suitecrm-0004**: `data/SugarBean.php` — `in_array($field, $all_fields)` inside nested foreach over subpanels — O(S*F^2), 4.3x at S=40 F=60 + +## MOAD-0002 (Intertangle): CLEAN (architectural, no patch target) + +SuiteCRM uses `$GLOBALS['current_user']`, `$GLOBALS['db']`, and `$GLOBALS['sugar_config']` as +shared mutable global state throughout 101+ modules. This is the standard SugarCRM/SuiteCRM +architectural pattern, not a single patchable coupling point. The global state is framework-level +design. No single god object coupling stands out as a discrete patchable defect beyond the +framework's intentional design. + +## MOAD-0003 (Leaked Context): CLEAN (architectural PHP pattern) + +SuiteCRM stores `authenticated_user_id` in `$_SESSION` (PHP standard). The user object is +then stored in `$GLOBALS['current_user']`. This is the standard PHP session architecture. +No ThreadLocal misuse (PHP is not thread-based). No ScopedValue equivalent needed here. + +## MOAD-0004 (CWE-312): CLEAN + +LDAP authentication (`LDAPAuthenticateUser.php`) logs `$bind_user` (username DN) and +`$admin_user` (admin bind DN) but never logs `$bind_password` or `$admin_password`. +The admin password is fetched from config and used directly in `ldap_bind()` without +being interpolated into any log string. SMTP/outbound email logs HOST and PORT only, +never SMTP password. InboundEmail IMAP logs connection parameters without password. + +## MOAD-0005 (Thundering Herd): CLEAN + +`SugarCache` (`include/SugarCache/SugarCache.php`) is an in-process PHP array cache. +PHP is single-threaded per request — no concurrent access races possible. The +get+null+compute+set pattern is safe in this single-threaded context. `sugar_cache_put` +is not a distributed cache without additional configuration, so no thundering herd risk +in default deployments. diff --git a/defects/suitecrm/patch/suitecrm-0004-sugarbean-allfields-in_array.patch b/defects/suitecrm/patch/suitecrm-0004-sugarbean-allfields-in_array.patch new file mode 100644 index 000000000..4c236102a --- /dev/null +++ b/defects/suitecrm/patch/suitecrm-0004-sugarbean-allfields-in_array.patch @@ -0,0 +1,19 @@ +--- a/data/SugarBean.php ++++ b/data/SugarBean.php +@@ -883,12 +883,13 @@ + //If final_query is still empty, its time to build the sub-queries + if (empty($final_query)) { + $subqueries = SugarBean::build_sub_queries_for_union($subpanel_list, $subpanel_def, $parentbean, $order_by); + $all_fields = array(); ++ $all_fields_set = array(); // O(1) lookup instead of O(N) in_array + foreach ($subqueries as $i => $subquery) { + $query_fields = DBManagerFactory::getInstance()->getSelectFieldsFromQuery($subquery['select']); + foreach ($query_fields as $field => $select) { +- if (!in_array($field, $all_fields)) { ++ if (!isset($all_fields_set[$field])) { + $all_fields[] = $field; ++ $all_fields_set[$field] = true; + } + } + $subqueries[$i]['query_fields'] = $query_fields; + } diff --git a/defects/suitecrm/test/SuiteCRMCWE407Test.class b/defects/suitecrm/test/SuiteCRMCWE407Test.class index 059985701..57bce18ad 100644 Binary files a/defects/suitecrm/test/SuiteCRMCWE407Test.class and b/defects/suitecrm/test/SuiteCRMCWE407Test.class differ diff --git a/defects/suitecrm/test/SuiteCRMCWE407Test.java b/defects/suitecrm/test/SuiteCRMCWE407Test.java index e361a2a79..83e6a4f1d 100644 --- a/defects/suitecrm/test/SuiteCRMCWE407Test.java +++ b/defects/suitecrm/test/SuiteCRMCWE407Test.java @@ -9,6 +9,8 @@ import java.util.*; * — O(D*R) map marker record filter * suitecrm-0003: Email.php in_array(trim($match[0]), $knownEmails) * — O(N^2) email dedup accumulator + * suitecrm-0004: SugarBean.php in_array($field, $all_fields) inside nested foreach + * — O(S*F^2) subpanel union field dedup during subpanel list view */ public class SuiteCRMCWE407Test { @@ -90,6 +92,43 @@ public class SuiteCRMCWE407Test { return unique; } + // ---- suitecrm-0004: SugarBean subpanel union field dedup ---- + + /** + * Unpatched: in_array($field, $all_fields) inside nested foreach over subpanels * fields. + * $all_fields grows each time a new unique field is appended. + * Complexity: O(S * F * all_fields_size) = O(S * F^2) worst case. + */ + static List subpanelFieldDedupUnpatched(List> subpanelFields) { + List allFields = new ArrayList<>(); + for (List fields : subpanelFields) { + for (String field : fields) { + if (!allFields.contains(field)) { // O(allFields) scan + allFields.add(field); + } + } + } + return allFields; + } + + /** + * Patched: isset($all_fields_set[$field]) — O(1) lookup per field. + * Complexity: O(S * F) total. + */ + static List subpanelFieldDedupPatched(List> subpanelFields) { + List allFields = new ArrayList<>(); + Set allFieldsSet = new HashSet<>(); // O(1) lookup + for (List fields : subpanelFields) { + for (String field : fields) { + if (!allFieldsSet.contains(field)) { // O(1) + allFields.add(field); + allFieldsSet.add(field); + } + } + } + return allFields; + } + // ---- Test harness ---- static void testSuiteCRM0001() { @@ -195,10 +234,53 @@ public class SuiteCRMCWE407Test { System.out.println(" PASS"); } + static void testSuiteCRM0004() { + System.out.println("=== suitecrm-0004: SugarBean subpanel field dedup ==="); + int S = 40; // subpanels in a busy Account detail view + int F = 60; // fields per subpanel query + + // Build subpanel field lists with overlapping fields across subpanels + List> subpanelFields = new ArrayList<>(); + for (int s = 0; s < S; s++) { + List fields = new ArrayList<>(); + for (int f = 0; f < F; f++) { + // First 20 fields are shared (common: id, name, date_entered, etc.) + if (f < 20) { + fields.add("common_field_" + f); + } else { + fields.add("subpanel_" + s + "_field_" + f); + } + } + subpanelFields.add(fields); + } + + // Warmup + subpanelFieldDedupPatched(subpanelFields); + + long t0 = System.nanoTime(); + List r1 = subpanelFieldDedupUnpatched(subpanelFields); + long unpatched = System.nanoTime() - t0; + + t0 = System.nanoTime(); + List r2 = subpanelFieldDedupPatched(subpanelFields); + long patched = System.nanoTime() - t0; + + assert r1.equals(r2) : "Result mismatch: " + r1.size() + " vs " + r2.size(); + int expectedUnique = 20 + S * (F - 20); // 20 common + S*40 unique per subpanel + assert r1.size() == expectedUnique : "Expected " + expectedUnique + " unique fields, got " + r1.size(); + + double ratio = (double) unpatched / patched; + System.out.printf(" S=%d F=%d unique=%d unpatched=%dms patched=%dms ratio=%.1fx%n", + S, F, r1.size(), unpatched / 1_000_000, patched / 1_000_000, ratio); + assert ratio > 3.0 : "Expected >3x ratio, got " + ratio; + System.out.println(" PASS"); + } + public static void main(String[] args) { testSuiteCRM0001(); testSuiteCRM0002(); testSuiteCRM0003(); - System.out.println("\nAll 3 SuiteCRM CWE-407 tests PASSED."); + testSuiteCRM0004(); + System.out.println("\nAll 4 SuiteCRM CWE-407 tests PASSED."); } }