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

thunderbird: 2 defects (0007 O(N²) folder tree init, 0008 CWE-312 OAuth2 token logging); MOADs 0002/0003/0005 CLEAN

Browse source
This commit is contained in:
russell@unturf.com 2026-03-31 20:02:17 -04:00
parent 097829af0a
commit 62305ef339
6 changed files with 378 additions and 0 deletions
Download patch file Download diff file Expand all files Collapse all files

49
defects/thunderbird-0007/patch/thunderbird-0007_about3Pane_initServer_existingURIs_ON2.patch Normal file
View file

@ -0,0 +1,49 @@
--- a/mail/base/content/about3Pane.js
+++ b/mail/base/content/about3Pane.js
@@ -1241,20 +1241,21 @@ MOAD-0001 CWE-407: about3Pane.js SmartServerPane.initServer() O(N²) folder dedup
# Severity: MEDIUM-HIGH
# Ratio: O(F²) where F = number of folders in a server (e.g. 500 IMAP folders → 250,000 ops)
#
# Root cause: initServer() builds existingURIs as an Array, then calls existingURIs.includes()
# inside a do-while loop over remainingFolderURIs. After each addFolder() call the array is
# fully rebuilt from DOM (Array.from(existingRows, li => li.uri)). Every includes() call is
# O(F) and there are F iterations → O(F²) total.
#
# Fix: use a Set<string> for existingURIs. Seed it once from DOM; when addFolder() adds a row
# update the Set directly instead of re-scanning the entire live NodeList.
initServer(server) {
// Find all folders in this server, and display the ones that aren't
// currently displayed.
const descendants = new Map(
server.rootFolder.descendants.map(d => [d.URI, d])
);
if (!descendants.size) {
return;
}
const remainingFolderURIs = Array.from(descendants.keys());
// Get a list of folders that already exist in the folder tree.
const existingRows = this.containerList.getElementsByTagName("li");
- let existingURIs = Array.from(existingRows, li => li.uri);
+ // PATCH thunderbird-0007: use a Set so membership checks are O(1).
+ const existingURIs = new Set(Array.from(existingRows, li => li.uri));
do {
const folderURI = remainingFolderURIs.shift();
- if (existingURIs.includes(folderURI)) {
+ if (existingURIs.has(folderURI)) {
continue;
}
const folder = descendants.get(folderURI);
if (folderPane._isGmailFolder(folder)) {
continue;
}
this.addFolder(folderPane._getNonGmailParent(folder), folder);
- // Update the list of existing folders. `existingRows` is a live
- // list, so we don't need to call `getElementsByTagName` again.
- existingURIs = Array.from(existingRows, li => li.uri);
+ // Update the Set so subsequent iterations see the newly added row.
+ // `existingRows` is a live NodeList — we just add the new URI.
+ existingURIs.add(folderURI);
} while (remainingFolderURIs.length);
},

BIN
defects/thunderbird-0007/test/ThunderbirdInitServerTest.class Normal file
View file

Binary file not shown.

146
defects/thunderbird-0007/test/ThunderbirdInitServerTest.java Normal file
View file

@ -0,0 +1,146 @@
import java.util.*;
/**
* Unit test for thunderbird-0007: about3Pane.js SmartServerPane.initServer()
* existingURIs Array.includes() O(N²) in do-while loop.
*
* Models the JavaScript pattern:
* let existingURIs = Array.from(existingRows, li => li.uri); // re-built each iteration
* do {
* const folderURI = remainingFolderURIs.shift();
* if (existingURIs.includes(folderURI)) continue; // O(F) per check
* this.addFolder(...);
* existingURIs = Array.from(existingRows, li => li.uri); // full rebuild!
* } while (remainingFolderURIs.length);
*
* Fix: replace Array with Set so membership checks are O(1).
*/
public class ThunderbirdInitServerTest {
// --- Defective implementation (Array + linear includes + full rebuild) ---
static int initServerDefective(List<String> remainingFolderURIs,
List<String> initialExistingURIs) {
List<String> existingRows = new ArrayList<>(initialExistingURIs);
int ops = 0;
List<String> remaining = new ArrayList<>(remainingFolderURIs);
List<String> existingURIs = new ArrayList<>(existingRows);
while (!remaining.isEmpty()) {
String folderURI = remaining.remove(0);
// O(F) scan — the defect
ops++;
boolean found = existingURIs.contains(folderURI);
if (found) continue;
// addFolder: mutates existingRows
existingRows.add(folderURI);
// Full rebuild — another O(F) scan hidden here
existingURIs = new ArrayList<>(existingRows);
ops += existingRows.size(); // cost of rebuild
}
return ops;
}
// --- Fixed implementation (Set + O(1) has()) ---
static int initServerFixed(List<String> remainingFolderURIs,
List<String> initialExistingURIs) {
List<String> existingRows = new ArrayList<>(initialExistingURIs);
int ops = 0;
List<String> remaining = new ArrayList<>(remainingFolderURIs);
Set<String> existingURIs = new HashSet<>(existingRows);
while (!remaining.isEmpty()) {
String folderURI = remaining.remove(0);
ops++;
if (existingURIs.contains(folderURI)) continue;
// addFolder: just add to Set, no rebuild
existingRows.add(folderURI);
existingURIs.add(folderURI);
}
return ops;
}
// --- Helper ---
static List<String> makeFolderURIs(int count, String prefix) {
List<String> uris = new ArrayList<>();
for (int i = 0; i < count; i++) {
uris.add("imap://user@server/mailbox/" + prefix + i);
}
return uris;
}
public static void main(String[] args) {
System.out.println("=== thunderbird-0007: initServer existingURIs O(N²) ===\n");
// Test 1: correctness - no duplicates processed
{
List<String> existing = makeFolderURIs(3, "existing-");
List<String> remaining = makeFolderURIs(5, "new-");
// Add overlap: one of the new folders is already in existing
remaining.add(existing.get(0)); // duplicate
// Both implementations should accept the same non-duplicate count
// (just verify they run without error and give non-negative ops)
int opsD = initServerDefective(new ArrayList<>(remaining), new ArrayList<>(existing));
int opsF = initServerFixed(new ArrayList<>(remaining), new ArrayList<>(existing));
assert opsD > 0 : "FAIL: defective returned 0 ops";
assert opsF > 0 : "FAIL: fixed returned 0 ops";
System.out.println("PASS correctness: defective=" + opsD + " ops, fixed=" + opsF + " ops");
}
// Test 2: small N — verify both give same logical result
{
for (int n : new int[]{10, 50, 100}) {
List<String> existing = makeFolderURIs(2, "e-");
List<String> remaining = makeFolderURIs(n, "r-");
int opsD = initServerDefective(new ArrayList<>(remaining), new ArrayList<>(existing));
int opsF = initServerFixed(new ArrayList<>(remaining), new ArrayList<>(existing));
// Fixed should always use fewer ops than defective for N > small constant
System.out.printf("N=%-4d defective=%6d ops fixed=%4d ops ratio=%.1fx%n",
n, opsD, opsF, (double) opsD / opsF);
}
}
// Test 3: ratio benchmark at F=500 (large IMAP account)
{
int F = 500;
List<String> existing = makeFolderURIs(10, "existing-");
List<String> remaining = makeFolderURIs(F, "folder-");
long t0 = System.nanoTime();
int opsD = initServerDefective(new ArrayList<>(remaining), new ArrayList<>(existing));
long t1 = System.nanoTime();
int opsF = initServerFixed(new ArrayList<>(remaining), new ArrayList<>(existing));
long t2 = System.nanoTime();
double ratio = (double) opsD / opsF;
System.out.printf("%nF=%-4d defective=%8d ops fixed=%6d ops op-ratio=%.1fx%n",
F, opsD, opsF, ratio);
System.out.printf(" defective=%6d µs fixed=%5d µs%n",
(t1 - t0) / 1000, (t2 - t1) / 1000);
assert ratio > 5.0 : "FAIL: expected ratio > 5x at F=500, got " + ratio;
System.out.println("PASS: ratio > 5x confirmed");
}
// Test 4: all-duplicate case (everything already in tree)
{
List<String> existing = makeFolderURIs(100, "folder-");
List<String> remaining = new ArrayList<>(existing); // 100% overlap
int opsD = initServerDefective(new ArrayList<>(remaining), new ArrayList<>(existing));
int opsF = initServerFixed(new ArrayList<>(remaining), new ArrayList<>(existing));
// Both should still handle this correctly (skip all)
assert opsD > 0 : "FAIL: defective should still do ops on duplicate check";
System.out.println("PASS all-duplicate: defective=" + opsD + " fixed=" + opsF);
}
System.out.println("\nAll tests PASS");
}
}

38
defects/thunderbird-0008/patch/thunderbird-0008_OAuth2_accessToken_logged_CWE312.patch Normal file
View file

@ -0,0 +1,38 @@
--- a/mailnews/base/src/OAuth2.sys.mjs
+++ b/mailnews/base/src/OAuth2.sys.mjs
@@ -318,16 +318,25 @@ MOAD-0004 CWE-312: OAuth2.sys.mjs access_token and refresh_token logged verbatim
# Severity: HIGH
# CVE class: CWE-312 Cleartext Storage of Sensitive Information
#
# Root cause: requestAccessToken() receives the full JSON response from the OAuth2
# authorization server, serializes it with JSON.stringify(result) → resultStr, then
# logs it verbatim at log.info level:
#
# line 333: log.info(`Error response details: ${resultStr}`)
# line 358: log.info(`Successful response from the authorization server: ${resultStr}`)
#
# A successful OAuth2 response always contains "access_token" and frequently
# "refresh_token". When a user or developer sets mailnews.oauth.loglevel to
# "All", "Debug", or "Info" (common during troubleshooting), these tokens are
# written to the application log and/or browser console.
#
# access_token grants full mailbox access (Gmail, Microsoft 365, Fastmail).
# refresh_token is long-lived and survives account password changes.
# Logging either constitutes CWE-312: credentials at rest in plaintext log files.
#
# Fix: strip sensitive fields before logging. Redact access_token, refresh_token,
# and id_token from the result object before calling JSON.stringify.
.then(result => {
- const resultStr = JSON.stringify(result);
+ // PATCH thunderbird-0008: redact credential fields before logging (CWE-312).
+ const safeResult = Object.assign({}, result);
+ for (const field of ["access_token", "refresh_token", "id_token"]) {
+ if (field in safeResult) {
+ safeResult[field] = "[redacted]";
+ }
+ }
+ const resultStr = JSON.stringify(safeResult);
if ("error" in result) {
// RFC 6749 section 5.2. Error Response
let err = result.error;

BIN
defects/thunderbird-0008/test/ThunderbirdOAuth2TokenLogTest.class Normal file
View file

Binary file not shown.

145
defects/thunderbird-0008/test/ThunderbirdOAuth2TokenLogTest.java Normal file
View file

@ -0,0 +1,145 @@
import java.util.*;
/**
* Unit test for thunderbird-0008: OAuth2.sys.mjs access_token and refresh_token
* logged verbatim in successful and error response paths (CWE-312).
*
* Models the JavaScript pattern:
* const resultStr = JSON.stringify(result);
* log.info(`Successful response from the authorization server: ${resultStr}`);
*
* A typical successful OAuth2 response:
* { "access_token": "ya29.XYZ...", "refresh_token": "1//0g...", "expires_in": 3600 }
*
* resultStr is logged with full token values. When mailnews.oauth.loglevel is
* set to "Info" (common during user/developer troubleshooting), tokens appear
* in the Thunderbird error console and application log.
*
* Fix: shallow-copy result, replace sensitive fields with "[redacted]" before stringify.
*/
public class ThunderbirdOAuth2TokenLogTest {
// --- Simulate defective logging ---
static String buildLogMessageDefective(Map<String, String> oauthResponse) {
// Mirrors: const resultStr = JSON.stringify(result);
// log.info(`Successful response from the authorization server: ${resultStr}`);
StringBuilder sb = new StringBuilder("{");
boolean first = true;
for (Map.Entry<String, String> e : oauthResponse.entrySet()) {
if (!first) sb.append(", ");
sb.append("\"").append(e.getKey()).append("\": \"").append(e.getValue()).append("\"");
first = false;
}
sb.append("}");
return "Successful response from the authorization server: " + sb;
}
// --- Simulate fixed logging ---
static String buildLogMessageFixed(Map<String, String> oauthResponse) {
// Mirrors the patch: redact sensitive fields before stringify
Map<String, String> safe = new LinkedHashMap<>(oauthResponse);
for (String field : new String[]{"access_token", "refresh_token", "id_token"}) {
if (safe.containsKey(field)) {
safe.put(field, "[redacted]");
}
}
StringBuilder sb = new StringBuilder("{");
boolean first = true;
for (Map.Entry<String, String> e : safe.entrySet()) {
if (!first) sb.append(", ");
sb.append("\"").append(e.getKey()).append("\": \"").append(e.getValue()).append("\"");
first = false;
}
sb.append("}");
return "Successful response from the authorization server: " + sb;
}
public static void main(String[] args) {
System.out.println("=== thunderbird-0008: OAuth2 token logging CWE-312 ===\n");
// Simulate a real OAuth2 successful response
Map<String, String> oauthResponse = new LinkedHashMap<>();
oauthResponse.put("access_token", "ya29.A0ARrdaM9_SENSITIVE_ACCESS_TOKEN_XYZ");
oauthResponse.put("refresh_token", "1//0gSENSITIVE_REFRESH_TOKEN_ABC");
oauthResponse.put("token_type", "Bearer");
oauthResponse.put("expires_in", "3600");
oauthResponse.put("scope", "https://mail.google.com/");
// Test 1: defective path leaks tokens in log
{
String logMsg = buildLogMessageDefective(oauthResponse);
boolean leaksAccess = logMsg.contains("ya29.A0ARrdaM9_SENSITIVE_ACCESS_TOKEN_XYZ");
boolean leaksRefresh = logMsg.contains("1//0gSENSITIVE_REFRESH_TOKEN_ABC");
assert leaksAccess : "FAIL: expected defective path to contain access_token";
assert leaksRefresh : "FAIL: expected defective path to contain refresh_token";
System.out.println("CONFIRMED defective: access_token in log = " + leaksAccess);
System.out.println("CONFIRMED defective: refresh_token in log = " + leaksRefresh);
}
// Test 2: fixed path redacts tokens
{
String logMsg = buildLogMessageFixed(oauthResponse);
boolean hiddenAccess = !logMsg.contains("ya29.A0ARrdaM9_SENSITIVE_ACCESS_TOKEN_XYZ");
boolean hiddenRefresh = !logMsg.contains("1//0gSENSITIVE_REFRESH_TOKEN_ABC");
boolean hasRedacted = logMsg.contains("[redacted]");
assert hiddenAccess : "FAIL: fixed path should not contain access_token";
assert hiddenRefresh : "FAIL: fixed path should not contain refresh_token";
assert hasRedacted : "FAIL: fixed path should contain [redacted] marker";
System.out.println("PASS fixed: access_token hidden = " + hiddenAccess);
System.out.println("PASS fixed: refresh_token hidden = " + hiddenRefresh);
System.out.println("PASS fixed: [redacted] present = " + hasRedacted);
}
// Test 3: fixed path preserves non-sensitive fields (useful diagnostic info)
{
String logMsg = buildLogMessageFixed(oauthResponse);
assert logMsg.contains("Bearer") : "FAIL: token_type should still be logged";
assert logMsg.contains("3600") : "FAIL: expires_in should still be logged";
assert logMsg.contains("mail.google.com") : "FAIL: scope should still be logged";
System.out.println("PASS: non-sensitive fields preserved in fixed log message");
}
// Test 4: response without refresh_token (PKCE flow) — only access_token redacted
{
Map<String, String> pkceResponse = new LinkedHashMap<>();
pkceResponse.put("access_token", "ya29.PKCE_ACCESS_TOKEN");
pkceResponse.put("token_type", "Bearer");
pkceResponse.put("expires_in", "3600");
String logMsg = buildLogMessageFixed(pkceResponse);
assert !logMsg.contains("ya29.PKCE_ACCESS_TOKEN") : "FAIL: PKCE access_token should be redacted";
assert logMsg.contains("[redacted]") : "FAIL: [redacted] marker expected";
System.out.println("PASS: PKCE (no refresh_token) access_token redacted correctly");
}
// Test 5: id_token (OpenID Connect) also redacted
{
Map<String, String> oidcResponse = new LinkedHashMap<>();
oidcResponse.put("access_token", "AT_SENSITIVE");
oidcResponse.put("id_token", "eyJhbGciOiJSU0I_SENSITIVE_ID_TOKEN");
oidcResponse.put("token_type", "Bearer");
String logMsg = buildLogMessageFixed(oidcResponse);
assert !logMsg.contains("eyJhbGciOiJSU0I_SENSITIVE_ID_TOKEN") : "FAIL: id_token should be redacted";
System.out.println("PASS: id_token (OpenID Connect) also redacted");
}
// Test 6: error response — ensure resultStr does not leak any partial token
// Error case at line 333: log.info(`Error response details: ${resultStr}`)
// Some providers return token data in error responses too (e.g. invalid_scope with partial grant)
{
Map<String, String> errorResponse = new LinkedHashMap<>();
errorResponse.put("error", "invalid_scope");
errorResponse.put("access_token", "ya29.PARTIAL_TOKEN_ON_ERROR");
errorResponse.put("error_description", "Requested scope not allowed");
String logMsg = buildLogMessageFixed(errorResponse);
assert !logMsg.contains("ya29.PARTIAL_TOKEN_ON_ERROR") : "FAIL: partial token on error should be redacted";
System.out.println("PASS: error response partial token also redacted");
}
System.out.println("\nAll tests PASS");
}
}