engineering/java-topology
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

suitecrm+dolibarr: 5-MOAD scan — 3 new defects, MOADs 0002/0003/0005 CLEAN

Browse source 
suitecrm-0004: SugarBean.php subpanel union field dedup in_array($field, $all_fields)
inside nested foreach over subpanel queries — O(S*F^2), 4.3x at S=40 F=60.
Fix: parallel hash set for O(1) isset() check.

dolibarr-0004: emailcollector_card.php line 575 — IMAP password logged verbatim
unconditionally in dol_syslog(). CWE-312 HIGH. Fix: replace with literal ***.

dolibarr-0005: functions_ldap.php line 98 — LDAP admin searchPassword first 3 chars
leaked via dol_trunc() in dol_syslog() and browser print when $ldapdebug=true.
CWE-312 MEDIUM. Fix: replace dol_trunc(...) with literal *** in both sinks.

All 4 SuiteCRM tests PASS. All 5 Dolibarr tests PASS.
MOADs 0002/0003/0005 CLEAN for both targets (PHP single-threaded, architectural globals).
This commit is contained in:
russell@unturf.com 2026-03-31 20:27:34 -04:00
parent 14c3a92bab
commit fc7fdad2dc
9 changed files with 301 additions and 3 deletions
Download patch file Download diff file Expand all files Collapse all files

42
defects/dolibarr/MOAD-0001-0002-0003-0005-partial-CLEAN.md Normal file
View file

@ -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.

11
defects/dolibarr/patch/dolibarr-0004-emailcollector-imap-password-logged.patch Normal file
View file

@ -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);

11
defects/dolibarr/patch/dolibarr-0005-ldap-searchpassword-logged.patch Normal file
View file

@ -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."<br>\n";
- print "DEBUG: uid/samaccountname=".$ldapuserattr.", dn=".$ldapdn.", Admin:".$ldap->searchUser.", Pass:".dol_trunc($ldap->searchPassword, 3)."<br>\n";
+ print "DEBUG: uid/samaccountname=".$ldapuserattr.", dn=".$ldapdn.", Admin:".$ldap->searchUser.", Pass:***<br>\n";
}

BIN
defects/dolibarr/test/DolibarrCWE407Test.class
View file

Binary file not shown.

94
defects/dolibarr/test/DolibarrCWE407Test.java
View file

@ -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.");
}
}

43
defects/suitecrm/MOAD-0002-0003-0005-CLEAN.md Normal file
View file

@ -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.

19
defects/suitecrm/patch/suitecrm-0004-sugarbean-allfields-in_array.patch Normal file
View file

@ -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;
}

BIN
defects/suitecrm/test/SuiteCRMCWE407Test.class
View file

Binary file not shown.

84
defects/suitecrm/test/SuiteCRMCWE407Test.java
View file

@ -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<String> subpanelFieldDedupUnpatched(List<List<String>> subpanelFields) {
List<String> allFields = new ArrayList<>();
for (List<String> 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<String> subpanelFieldDedupPatched(List<List<String>> subpanelFields) {
List<String> allFields = new ArrayList<>();
Set<String> allFieldsSet = new HashSet<>(); // O(1) lookup
for (List<String> 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<List<String>> subpanelFields = new ArrayList<>();
for (int s = 0; s < S; s++) {
List<String> 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<String> r1 = subpanelFieldDedupUnpatched(subpanelFields);
long unpatched = System.nanoTime() - t0;
t0 = System.nanoTime();
List<String> 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.");
}
}