langchain: 1 CWE-407 defect; forgejo: 2 new CWE-407 defects, MOADs 0002-0005 CLEAN

langchain-0001: MultiVectorRetriever._get_relevant_documents() dedup
  IDs from vectorstore sub_docs uses list.contains() inside loop, O(k^2).
  k is unbounded in production RAG pipelines (configurable via search_kwargs).
  Fix: track seen IDs in a set, keep list for order. 499.5x at k=1000.

forgejo-0002: LoadRepoConfig() license sort O(P*L) where L=776 licenses.
  Two SliceContainsString calls in back-to-back loops iterate full license
  list for each preferred license and vice versa. Fix: build lookup sets
  before loops. 19.5x at P=20 preferred licenses.

forgejo-0003: synchronizePublicKeys() three O(N*M) scans per LDAP sync.
  Dedup of providedKeys is O(K^2), plus two O(P*G) set-difference loops.
  Runs per user per sync cycle. Fix: use maps for O(1) membership. 178.6x
  at K=G=500.

forgejo-0001 (search.go RepoIDs) already patched in prior scan.
MOADs 0002-0005 CLEAN for both targets.
This commit is contained in:
russell@unturf.com 2026-03-31 20:12:14 -04:00
parent efe58242e0
commit cdff140a7c
9 changed files with 593 additions and 0 deletions

View file

@ -0,0 +1,27 @@
--- a/modules/repository/init.go
+++ b/modules/repository/init.go
@@ -104,13 +104,16 @@ func LoadRepoConfig() error {
// Filter out invalid names and promote preferred licenses.
+ // Build a set for O(1) lookups to avoid O(P*L) and O(L*P) scans.
+ licensesSet := make(map[string]struct{}, len(Licenses))
+ for _, name := range Licenses {
+ licensesSet[strings.ToLower(name)] = struct{}{}
+ }
+ preferredSet := make(map[string]struct{}, len(setting.Repository.PreferredLicenses))
+ for _, name := range setting.Repository.PreferredLicenses {
+ preferredSet[strings.ToLower(name)] = struct{}{}
+ }
sortedLicenses := make([]string, 0, len(Licenses))
for _, name := range setting.Repository.PreferredLicenses {
- if util.SliceContainsString(Licenses, name, true) {
+ if _, ok := licensesSet[strings.ToLower(name)]; ok {
sortedLicenses = append(sortedLicenses, name)
}
}
for _, name := range Licenses {
- if !util.SliceContainsString(setting.Repository.PreferredLicenses, name, true) {
+ if _, ok := preferredSet[strings.ToLower(name)]; !ok {
sortedLicenses = append(sortedLicenses, name)
}
}

Binary file not shown.

View file

@ -0,0 +1,157 @@
import java.util.*;
/**
* Unit test simulating Forgejo forgejo-0002:
* modules/repository/init.go LoadRepoConfig() license sort uses O(P*L) scans:
*
* for _, name := range setting.Repository.PreferredLicenses {
* if util.SliceContainsString(Licenses, name, true) { // O(L) scan
* sortedLicenses = append(sortedLicenses, name)
* }
* }
* for _, name := range Licenses {
* if !util.SliceContainsString(setting.Repository.PreferredLicenses, name, true) { // O(P) scan
* sortedLicenses = append(sortedLicenses, name)
* }
* }
*
* Forgejo ships 776 licenses. Any preferred list of P entries costs O(P*776 + 776*P).
* Fix: build sets for O(1) lookup before the loops.
*/
public class ForgejoLicenseSortTest {
// Defective: O(P*L + L*P)
static List<String> sortLicensesDefective(
List<String> allLicenses, List<String> preferred) {
List<String> sorted = new ArrayList<>(allLicenses.size());
for (String name : preferred) {
if (containsIgnoreCase(allLicenses, name)) {
sorted.add(name);
}
}
for (String name : allLicenses) {
if (!containsIgnoreCase(preferred, name)) {
sorted.add(name);
}
}
return sorted;
}
static boolean containsIgnoreCase(List<String> list, String target) {
String t = target.toLowerCase(Locale.ROOT);
for (String s : list) {
if (s.toLowerCase(Locale.ROOT).equals(t)) return true;
}
return false;
}
// Fixed: build sets first, then O(P + L)
static List<String> sortLicensesFixed(
List<String> allLicenses, List<String> preferred) {
Set<String> licenseSet = new HashSet<>(allLicenses.size() * 2);
for (String s : allLicenses) licenseSet.add(s.toLowerCase(Locale.ROOT));
Set<String> preferredSet = new HashSet<>(preferred.size() * 2);
for (String s : preferred) preferredSet.add(s.toLowerCase(Locale.ROOT));
List<String> sorted = new ArrayList<>(allLicenses.size());
for (String name : preferred) {
if (licenseSet.contains(name.toLowerCase(Locale.ROOT))) {
sorted.add(name);
}
}
for (String name : allLicenses) {
if (!preferredSet.contains(name.toLowerCase(Locale.ROOT))) {
sorted.add(name);
}
}
return sorted;
}
static long countDefectiveOps(List<String> allLicenses, List<String> preferred) {
long ops = 0;
// First loop: P * L
for (String name : preferred) {
ops += allLicenses.size();
}
// Second loop: L * P
for (String name : allLicenses) {
ops += preferred.size();
}
return ops;
}
static long countFixedOps(List<String> allLicenses, List<String> preferred) {
// Build sets: O(L + P)
long ops = allLicenses.size() + preferred.size();
// Two loops: O(P + L)
ops += preferred.size() + allLicenses.size();
return ops;
}
// Build a realistic license list like Forgejo's 776 licenses
static List<String> makeLicenses(int count) {
List<String> licenses = new ArrayList<>(count);
String[] templates = {
"MIT", "Apache-2.0", "GPL-2.0", "GPL-3.0", "LGPL-2.1",
"BSD-2-Clause", "BSD-3-Clause", "ISC", "MPL-2.0", "CDDL-1.0"
};
for (int i = 0; i < count; i++) {
licenses.add(templates[i % templates.length] + "-variant-" + i);
}
return licenses;
}
public static void main(String[] args) {
System.out.println("forgejo-0002: LoadRepoConfig license sort O(P*L) -> O(P+L)");
System.out.println("=".repeat(60));
// Correctness test with small data
List<String> licenses = Arrays.asList(
"MIT", "Apache-2.0", "GPL-2.0", "GPL-3.0", "BSD-2-Clause"
);
List<String> preferred = Arrays.asList("GPL-3.0", "MIT");
List<String> defective = sortLicensesDefective(licenses, preferred);
List<String> fixed = sortLicensesFixed(licenses, preferred);
// Both should start with preferred items (in preferred order), rest follow
if (!defective.get(0).equals("GPL-3.0") || !defective.get(1).equals("MIT")) {
throw new AssertionError("Defective sort wrong ordering: " + defective);
}
if (!fixed.get(0).equals("GPL-3.0") || !fixed.get(1).equals("MIT")) {
throw new AssertionError("Fixed sort wrong ordering: " + fixed);
}
if (defective.size() != fixed.size()) {
throw new AssertionError("Size mismatch: " + defective.size() + " vs " + fixed.size());
}
System.out.println("PASS: correctness verified");
// Benchmark: 776 licenses (realistic), various preferred counts
System.out.printf("%n%-10s %15s %12s %10s%n",
"licenses", "defective_ops", "fixed_ops", "ratio");
System.out.println("-".repeat(48));
int L = 776;
List<String> allLicenses = makeLicenses(L);
int[] prefCounts = {1, 5, 10, 20};
for (int P : prefCounts) {
List<String> pref = allLicenses.subList(0, P);
long def = countDefectiveOps(allLicenses, pref);
long fix = countFixedOps(allLicenses, pref);
double ratio = (double) def / fix;
System.out.printf("L=%-4d P=%-4d %15d %12d %9.1fx%n", L, P, def, fix, ratio);
}
long def776 = countDefectiveOps(allLicenses, allLicenses.subList(0, 5));
long fix776 = countFixedOps(allLicenses, allLicenses.subList(0, 5));
double ratio776 = (double) def776 / fix776;
if (ratio776 < 4) {
throw new AssertionError("Expected >4x ratio at L=776, P=5, got " + ratio776);
}
System.out.println("\nAll assertions PASS");
System.out.println("Fix: build licensesSet and preferredSet before loops for O(1) lookup");
}
}

View file

@ -0,0 +1,47 @@
--- a/models/asymkey/ssh_key.go
+++ b/models/asymkey/ssh_key.go
@@ -378,19 +378,22 @@ func synchronizePublicKeys(ctx context.Context, s *auth.Source, usr *user_model.
// Process the provided keys to remove duplicates and name part
- var providedKeys []string
+ providedKeysSet := make(map[string]struct{})
+ var providedKeys []string
for _, v := range sshPublicKeys {
sshKeySplit := strings.Split(v, " ")
if len(sshKeySplit) > 1 {
key := strings.Join(sshKeySplit[:2], " ")
- if !util.SliceContainsString(providedKeys, key) {
+ if _, exists := providedKeysSet[key]; !exists {
+ providedKeysSet[key] = struct{}{}
providedKeys = append(providedKeys, key)
}
}
}
// Check if Public Key sync is needed
@@ -399,14 +402,16 @@ func synchronizePublicKeys(ctx context.Context, s *auth.Source, usr *user_model.
// Add new Public SSH Keys that doesn't already exist in DB
+ giteaKeysSet := make(map[string]struct{}, len(giteaKeys))
+ for _, k := range giteaKeys {
+ giteaKeysSet[k] = struct{}{}
+ }
var newKeys []string
for _, key := range providedKeys {
- if !util.SliceContainsString(giteaKeys, key) {
+ if _, exists := giteaKeysSet[key]; !exists {
newKeys = append(newKeys, key)
}
}
if AddPublicKeysBySource(ctx, usr, s, newKeys) {
sshKeysNeedUpdate = true
}
// Mark keys from DB that no longer exist in the source for deletion
var giteaKeysToDelete []string
for _, giteaKey := range giteaKeys {
- if !util.SliceContainsString(providedKeys, giteaKey) {
+ if _, exists := providedKeysSet[giteaKey]; !exists {
log.Trace("synchronizePublicKeys[%s]: Marking Public SSH Key for deletion for user %s: %v", s.Name, usr.Name, giteaKey)
giteaKeysToDelete = append(giteaKeysToDelete, giteaKey)
}
}

Binary file not shown.

View file

@ -0,0 +1,179 @@
import java.util.*;
/**
* Unit test simulating Forgejo forgejo-0003:
* models/asymkey/ssh_key.go synchronizePublicKeys() has three O(N*M) scans:
*
* 1. Building providedKeys with dedup: O(K^2) where K = LDAP-provided key count
* for _, v := range sshPublicKeys {
* key = ...
* if !util.SliceContainsString(providedKeys, key) { // O(|providedKeys|) each time
*
* 2. Finding new keys: O(P * G) where P = provided, G = DB keys
* for _, key := range providedKeys {
* if !util.SliceContainsString(giteaKeys, key) { // O(G) each
*
* 3. Finding deleted keys: O(G * P)
* for _, giteaKey := range giteaKeys {
* if !util.SliceContainsString(providedKeys, giteaKey) { // O(P) each
*
* This runs per user per LDAP sync cycle. With many users or large key sets it accumulates.
* Fix: build maps for O(1) lookup before each loop.
*/
public class ForgejoSSHKeySyncTest {
// Defective: O(K^2) dedup + O(P*G) + O(G*P)
static long countDefectiveOps(int rawKeyCount, int giteaKeyCount) {
// Phase 1: dedup raw keys into providedKeys
// Assume half are unique: K/2 unique keys, K/2 duplicates
// Each insertion checks against growing list
long ops = 0;
int provided = 0;
for (int i = 0; i < rawKeyCount; i++) {
ops += provided; // SliceContainsString cost
if (i % 2 == 0) provided++; // ~half unique
}
// Phase 2: find new keys (provided not in gitea)
ops += (long) provided * giteaKeyCount;
// Phase 3: find keys to delete (gitea not in provided)
ops += (long) giteaKeyCount * provided;
return ops;
}
static long countFixedOps(int rawKeyCount, int giteaKeyCount) {
// Phase 1: dedup with set O(K) total
long ops = rawKeyCount;
int provided = rawKeyCount / 2; // ~half unique
// Build giteaKeysSet: O(G)
ops += giteaKeyCount;
// Phase 2: find new keys O(P)
ops += provided;
// Phase 3: find keys to delete O(G)
ops += giteaKeyCount;
return ops;
}
// Simulate the sync logic for correctness
static Map<String, List<String>> syncKeysDefective(
List<String> rawKeys, List<String> giteaKeys) {
// Dedup rawKeys
List<String> provided = new ArrayList<>();
for (String k : rawKeys) {
if (!provided.contains(k)) {
provided.add(k);
}
}
// Find new keys
List<String> toAdd = new ArrayList<>();
for (String k : provided) {
if (!giteaKeys.contains(k)) {
toAdd.add(k);
}
}
// Find keys to delete
List<String> toDelete = new ArrayList<>();
for (String k : giteaKeys) {
if (!provided.contains(k)) {
toDelete.add(k);
}
}
Map<String, List<String>> result = new HashMap<>();
result.put("toAdd", toAdd);
result.put("toDelete", toDelete);
return result;
}
static Map<String, List<String>> syncKeysFixed(
List<String> rawKeys, List<String> giteaKeys) {
// Dedup rawKeys with set
Set<String> providedSet = new LinkedHashSet<>();
List<String> provided = new ArrayList<>();
for (String k : rawKeys) {
if (providedSet.add(k)) {
provided.add(k);
}
}
// Build giteaKeys set
Set<String> giteaSet = new HashSet<>(giteaKeys);
// Find new keys
List<String> toAdd = new ArrayList<>();
for (String k : provided) {
if (!giteaSet.contains(k)) {
toAdd.add(k);
}
}
// Find keys to delete
List<String> toDelete = new ArrayList<>();
for (String k : giteaKeys) {
if (!providedSet.contains(k)) {
toDelete.add(k);
}
}
Map<String, List<String>> result = new HashMap<>();
result.put("toAdd", toAdd);
result.put("toDelete", toDelete);
return result;
}
public static void main(String[] args) {
System.out.println("forgejo-0003: synchronizePublicKeys() O(K^2+P*G) -> O(K+P+G)");
System.out.println("=".repeat(60));
// Correctness test
List<String> rawKeys = Arrays.asList(
"ssh-rsa AAAA1", "ssh-rsa AAAA2", "ssh-rsa AAAA1", // dup
"ssh-rsa AAAA3", "ssh-rsa AAAA2" // more dups
);
List<String> giteaKeys = Arrays.asList(
"ssh-rsa AAAA2", "ssh-rsa AAAA4"
);
Map<String, List<String>> def = syncKeysDefective(rawKeys, giteaKeys);
Map<String, List<String>> fix = syncKeysFixed(rawKeys, giteaKeys);
// Expected: add AAAA1 and AAAA3 (new from LDAP), delete AAAA4 (no longer in LDAP)
List<String> expectedAdd = Arrays.asList("ssh-rsa AAAA1", "ssh-rsa AAAA3");
List<String> expectedDelete = Arrays.asList("ssh-rsa AAAA4");
if (!def.get("toAdd").equals(expectedAdd)) {
throw new AssertionError("Defective toAdd wrong: " + def.get("toAdd"));
}
if (!def.get("toDelete").equals(expectedDelete)) {
throw new AssertionError("Defective toDelete wrong: " + def.get("toDelete"));
}
if (!fix.get("toAdd").equals(expectedAdd)) {
throw new AssertionError("Fixed toAdd wrong: " + fix.get("toAdd"));
}
if (!fix.get("toDelete").equals(expectedDelete)) {
throw new AssertionError("Fixed toDelete wrong: " + fix.get("toDelete"));
}
System.out.println("PASS: correctness verified");
// Performance comparison
System.out.printf("%n%-20s %15s %12s %10s%n",
"scenario", "defective_ops", "fixed_ops", "ratio");
System.out.println("-".repeat(58));
int[][] scenarios = {
{10, 10}, {50, 50}, {100, 100}, {500, 500}, {1000, 1000}
};
for (int[] sc : scenarios) {
int K = sc[0], G = sc[1];
long dOps = countDefectiveOps(K, G);
long fOps = countFixedOps(K, G);
double ratio = (double) dOps / fOps;
System.out.printf("K=%-4d G=%-4d %15d %12d %9.1fx%n",
K, G, dOps, fOps, ratio);
}
long dOps = countDefectiveOps(500, 500);
long fOps = countFixedOps(500, 500);
double ratio = (double) dOps / fOps;
if (ratio < 10) {
throw new AssertionError("Expected >10x ratio at K=500 G=500, got " + ratio);
}
System.out.println("\nAll assertions PASS");
System.out.println("Fix: use providedKeysSet(map) for O(1) dedup; giteaKeysSet for O(1) diff");
}
}

View file

@ -0,0 +1,32 @@
--- a/libs/langchain/langchain_classic/retrievers/multi_vector.py
+++ b/libs/langchain/langchain_classic/retrievers/multi_vector.py
@@ -105,9 +105,10 @@ class MultiVectorRetriever(BaseRetriever):
sub_docs = self.vectorstore.similarity_search(query, **self.search_kwargs)
# We do this to maintain the order of the IDs that are returned
- ids = []
+ seen_ids: set = set()
+ ids = []
for d in sub_docs:
- if self.id_key in d.metadata and d.metadata[self.id_key] not in ids:
+ if self.id_key in d.metadata and d.metadata[self.id_key] not in seen_ids:
+ seen_ids.add(d.metadata[self.id_key])
ids.append(d.metadata[self.id_key])
docs = self.docstore.mget(ids)
return [d for d in docs if d is not None]
@@ -147,9 +148,10 @@ class MultiVectorRetriever(BaseRetriever):
sub_docs = await self.vectorstore.asimilarity_search(
query, **self.search_kwargs
)
# We do this to maintain the order of the IDs that are returned
- ids = []
+ seen_ids_async: set = set()
+ ids = []
for d in sub_docs:
- if self.id_key in d.metadata and d.metadata[self.id_key] not in ids:
+ if self.id_key in d.metadata and d.metadata[self.id_key] not in seen_ids_async:
+ seen_ids_async.add(d.metadata[self.id_key])
ids.append(d.metadata[self.id_key])
docs = await self.docstore.amget(ids)
return [d for d in docs if d is not None]

View file

@ -0,0 +1,151 @@
import java.util.*;
/**
* Unit test simulating LangChain langchain-0001:
* MultiVectorRetriever._get_relevant_documents() uses a list for ID dedup:
* ids = []
* for d in sub_docs:
* if id_key in d.metadata and d.metadata[id_key] not in ids:
* ids.append(d.metadata[id_key])
*
* "id not in ids" is O(|ids|) per document, making the full dedup O(k^2)
* where k = number of sub-docs from vectorstore search.
* Fix: use a set for O(1) membership, keeping ids list for order preservation.
*/
public class LangChainMultiVectorDedupTest {
// Defective: O(k^2) list-based dedup
static List<String> dedupList(List<String> subDocIds) {
List<String> ids = new ArrayList<>();
for (String id : subDocIds) {
if (!ids.contains(id)) { // O(|ids|) scan
ids.add(id);
}
}
return ids;
}
// Fixed: O(k) set-assisted dedup, preserving order
static List<String> dedupSet(List<String> subDocIds) {
Set<String> seen = new LinkedHashSet<>();
List<String> ids = new ArrayList<>();
for (String id : subDocIds) {
if (seen.add(id)) { // O(1) amortized
ids.add(id);
}
}
return ids;
}
// Measure operation count for list-based dedup
static long countListOps(List<String> subDocIds) {
List<String> ids = new ArrayList<>();
long ops = 0;
for (String id : subDocIds) {
ops += ids.size(); // each contains() scans whole list
if (!ids.contains(id)) {
ids.add(id);
}
}
return ops;
}
// Measure operation count for set-based dedup
static long countSetOps(List<String> subDocIds) {
Set<String> seen = new HashSet<>();
long ops = 0;
for (String id : subDocIds) {
ops += 1; // O(1) hash lookup
seen.add(id);
}
return ops;
}
// Simulate sub_docs where each doc has an id pointing to a parent document
// k sub-docs may map to fewer unique parent ids (many-to-one)
static List<String> makeSubDocIds(int k, int uniqueParents) {
Random rng = new Random(42);
List<String> ids = new ArrayList<>(k);
for (int i = 0; i < k; i++) {
ids.add("parent-" + (rng.nextInt(uniqueParents)));
}
return ids;
}
static void append(List<String> list, String val) {
list.add(val);
}
// Override dedupList to use add not append
static List<String> dedupListFixed(List<String> subDocIds) {
List<String> ids = new ArrayList<>();
for (String id : subDocIds) {
if (!ids.contains(id)) {
ids.add(id);
}
}
return ids;
}
public static void main(String[] args) {
System.out.println("langchain-0001: MultiVectorRetriever ID dedup O(k^2) -> O(k)");
System.out.println("=".repeat(60));
// Test correctness
List<String> input = Arrays.asList(
"p1", "p2", "p1", "p3", "p2", "p4", "p1"
);
List<String> expected = Arrays.asList("p1", "p2", "p3", "p4");
List<String> listResult = dedupListFixed(input);
List<String> setResult = dedupSet(input);
if (!listResult.equals(expected)) {
throw new AssertionError("List dedup wrong: " + listResult);
}
if (!setResult.equals(expected)) {
throw new AssertionError("Set dedup wrong: " + setResult);
}
System.out.println("PASS: both produce correct ordered dedup");
// Benchmark comparison at various k values
System.out.printf("%n%-8s %12s %12s %10s%n",
"k", "list_ops", "set_ops", "ratio");
System.out.println("-".repeat(44));
int[] kValues = {10, 50, 100, 500, 1000};
for (int k : kValues) {
// Worst case: all unique ids (no duplicates) maximizes list scan ops
List<String> allUnique = new ArrayList<>(k);
for (int i = 0; i < k; i++) allUnique.add("p" + i);
long listOps = countListOps(allUnique);
long setOps = countSetOps(allUnique);
double ratio = (double) listOps / setOps;
System.out.printf("%-8d %12d %12d %9.1fx%n", k, listOps, setOps, ratio);
if (k >= 100 && ratio < 40) {
throw new AssertionError("Expected >40x ratio at k=" + k + ", got " + ratio);
}
}
// Realistic scenario: k=100 sub-docs mapping to 20 unique parents
int k = 100, parents = 20;
List<String> realistic = makeSubDocIds(k, parents);
long listOpsR = countListOps(realistic);
long setOpsR = countSetOps(realistic);
double ratioR = (double) listOpsR / setOpsR;
System.out.printf("%nRealistic k=%d, %d parents: list=%d set=%d ratio=%.1fx%n",
k, parents, listOpsR, setOpsR, ratioR);
if (ratioR < 5) {
throw new AssertionError("Expected >5x ratio in realistic case, got " + ratioR);
}
System.out.println("\nAll assertions PASS");
System.out.println("Fix: replace 'ids = []; id not in ids' with set-assisted dedup");
System.out.println(" seen_ids = set()");
System.out.println(" if id not in seen_ids: seen_ids.add(id); ids.append(id)");
}
}