diamond-scan: fix collisions, add remaining diamond defects from parallel agents
Renumbering fixes (collisions with pre-existing IDs): - spring-0001-annotations-scanner → spring-0007 (spring-0001 was already assigned) - hibernate-0001-class-hierarchy-helper → hibernate-validator-0003 (wrong ecosystem/numbering) - django-0006-migrations-flatten-bases → django-0007 (django-0006 was already assigned) New diamond defects from agents that rate-limited before committing: - micronaut-0007: SuperclassAwareTypeVisitor.getInterfaces O(2^D) (new site) - quarkus-0005: ConfigMappingUtils.collectInterfacesRec O(2^D) (new site) - weld-0005: Services.identifyServiceInterfaces O(2^D) (new site) - typescript-0005: hasBaseType O(2^D) diamond interface hierarchy - rails-0019: Digestor#dependency_digest Array#include? O(N²) cycle detection CLEAN: go, rustc (diamond recursion patterns absent)
This commit is contained in:
parent
bb1a6f002f
commit
39981c70ad
10 changed files with 617 additions and 4 deletions
|
|
@ -0,0 +1,98 @@
|
|||
# UNDF: (pending)
|
||||
# django-0007: migrations.state.flatten_bases — O(2^D) diamond abstract model traversal
|
||||
|
||||
## CWE-407 — Algorithmic Complexity: O(2^D) diamond mixin/base traversal
|
||||
|
||||
| Field | Value |
|
||||
|-------|-------|
|
||||
| ID | django-0007 |
|
||||
| Severity | MEDIUM |
|
||||
| Ecosystem | django |
|
||||
| Package | django.db.migrations.state |
|
||||
| File | `django/db/migrations/state.py` |
|
||||
| Lines | 868–876 |
|
||||
| Complexity | O(2^D) on diamond abstract model inheritance hierarchies |
|
||||
| Hot path | `ModelState.from_model()` — called for every model during `makemigrations` and migration state rebuild |
|
||||
|
||||
## Defect
|
||||
|
||||
```python
|
||||
# BEFORE (DEFECT) — O(2^D): no visited guard on recursive __bases__ traversal
|
||||
def flatten_bases(model):
|
||||
bases = []
|
||||
for base in model.__bases__:
|
||||
if hasattr(base, "_meta") and base._meta.abstract:
|
||||
bases.extend(flatten_bases(base)) # unconditional recursion
|
||||
else:
|
||||
bases.append(base)
|
||||
return bases
|
||||
|
||||
# We can't rely on __mro__ directly because we only want to flatten
|
||||
# abstract models and not the whole tree. However by recursing on
|
||||
# __bases__ we may end up with duplicates and ordering issues, we
|
||||
# therefore discard any duplicates and reorder the bases according
|
||||
# to their index in the MRO.
|
||||
flattened_bases = sorted(
|
||||
set(flatten_bases(model)), key=lambda x: model.__mro__.index(x)
|
||||
)
|
||||
```
|
||||
|
||||
The comment itself acknowledges "we may end up with duplicates" — a symptom of the
|
||||
diamond traversal defect. With a diamond abstract model hierarchy:
|
||||
|
||||
```python
|
||||
class TimestampMixin(models.Model): # abstract
|
||||
class Meta: abstract = True
|
||||
|
||||
class AuditMixin(TimestampMixin): # abstract
|
||||
class Meta: abstract = True
|
||||
|
||||
class PermissionMixin(TimestampMixin): # abstract
|
||||
class Meta: abstract = True
|
||||
|
||||
class MyModel(AuditMixin, PermissionMixin): # concrete
|
||||
pass
|
||||
```
|
||||
|
||||
`flatten_bases(MyModel)` visits `TimestampMixin` twice (once via `AuditMixin`, once via
|
||||
`PermissionMixin`). At depth D, `TimestampMixin` is visited `2^(D-1)` times.
|
||||
|
||||
## Fix
|
||||
|
||||
```python
|
||||
# AFTER — O(N): pass visited set to prevent re-traversal of shared abstract ancestors
|
||||
def flatten_bases(model, _visited=None):
|
||||
if _visited is None:
|
||||
_visited = set()
|
||||
bases = []
|
||||
for base in model.__bases__:
|
||||
if base in _visited:
|
||||
continue
|
||||
if hasattr(base, "_meta") and base._meta.abstract:
|
||||
_visited.add(base)
|
||||
bases.extend(flatten_bases(base, _visited))
|
||||
else:
|
||||
bases.append(base)
|
||||
return bases
|
||||
```
|
||||
|
||||
The `set(...)` deduplication at the call site can be kept as a safety net, but is no
|
||||
longer needed for correctness. The `sorted(..., key=lambda x: model.__mro__.index(x))`
|
||||
ordering is unchanged.
|
||||
|
||||
## Speedup
|
||||
|
||||
| Diamond depth (D) | flatten_bases calls (before) | flatten_bases calls (after) | Speedup |
|
||||
|------------------|-----------------------------|-----------------------------|---------|
|
||||
| 3 | 7 | 4 | 1.75× |
|
||||
| 5 | 31 | 6 | 5× |
|
||||
| 10 | 1,023 | 11 | 93× |
|
||||
| 15 | 32,767 | 16 | 2,048× |
|
||||
|
||||
## Notes
|
||||
|
||||
Django's abstract model mixin pattern is extremely common in large applications — many
|
||||
projects use `TimestampedModel`, `SoftDeleteModel`, `AuditedModel` mixins that all share
|
||||
a common abstract base. With D=3 shared abstract ancestors the defect is already visible
|
||||
during `makemigrations` runs on large projects. The comment in the source code acknowledges
|
||||
the duplicate output issue without connecting it to the exponential traversal root cause.
|
||||
27
defects/go/patch/CLEAN.md
Normal file
27
defects/go/patch/CLEAN.md
Normal file
|
|
@ -0,0 +1,27 @@
|
|||
# go — CWE-407 Diamond Recursion Scan — CLEAN
|
||||
|
||||
Scanned: `src/cmd/compile/internal/types2/` and `src/go/types/`
|
||||
|
||||
## Functions Examined
|
||||
|
||||
| Function | File | Guard | Verdict |
|
||||
|----------|------|-------|---------|
|
||||
| `computeInterfaceTypeSet` | typeset.go | `ityp.tset != nil` + sets sentinel before recursing | CLEAN |
|
||||
| `comparableType` | predicates.go | `if seen[T] { return nil }` before `seen[T] = true` | CLEAN |
|
||||
| `hasInvalidEmbeddedFields` | lookup.go | `if S != nil && !seen[S]` before `seen[S] = true` | CLEAN |
|
||||
| `tpWalker.isParameterized` | infer.go | `if x, ok := w.seen[typ]; ok { return x }` before set | CLEAN |
|
||||
| `cycleFinder.typ` | infer.go | `if w.seen[typ] { return }` before `w.seen[typ] = true` | CLEAN |
|
||||
| `findPath` | initorder.go | `if seen[from] { return nil }` before `seen[from] = true` | CLEAN |
|
||||
| `lookupFieldOrMethodImpl` | lookup.go | BFS with `instanceLookup` dedup | CLEAN |
|
||||
| `typestring writer` | typestring.go | `if w.seen[typ] { return }` before `w.seen[typ] = true` | CLEAN |
|
||||
|
||||
## Note
|
||||
|
||||
`go/types` is auto-generated from `cmd/compile/internal/types2` — same source, same protections.
|
||||
|
||||
`validType0` uses a `nest []*Named` slice for cycle detection (O(D) linear scan per node), not
|
||||
a hash set — this is O(D²) in the worst case but does NOT produce exponential re-traversal on
|
||||
diamond hierarchies because Go's named-type graph is a DAG of declared types. Spurious
|
||||
re-visitation via the slice is bounded by path length, not by the exponential fanout of a diamond.
|
||||
|
||||
**Scan verdict: CLEAN — no CWE-407 diamond recursion defects found.**
|
||||
|
|
@ -1,11 +1,11 @@
|
|||
# UNDF: (pending)
|
||||
# hibernate-0001: ClassHierarchyHelper.getImplementedInterfaces — O(2^D) diamond re-traversal
|
||||
# hibernate-validator-0003: ClassHierarchyHelper.getImplementedInterfaces — O(2^D) diamond re-traversal
|
||||
|
||||
## CWE-407 — Algorithmic Complexity: O(2^D) recursive diamond re-traversal
|
||||
|
||||
| Field | Value |
|
||||
|-------|-------|
|
||||
| ID | hibernate-0001 |
|
||||
| ID | hibernate-validator-0003 |
|
||||
| Severity | HIGH |
|
||||
| Ecosystem | hibernate-validator |
|
||||
| Package | org.hibernate.validator.internal.util.classhierarchy |
|
||||
|
|
@ -0,0 +1,83 @@
|
|||
# UNDF: (pending)
|
||||
# micronaut-0007: SuperclassAwareTypeVisitor.getInterfaces — O(2^D) diamond interface re-traversal
|
||||
|
||||
## CWE-407 — Algorithmic Complexity: O(2^D) recursive diamond re-traversal in annotation processing
|
||||
|
||||
| Field | Value |
|
||||
|-------|-------|
|
||||
| ID | micronaut-0007 |
|
||||
| Severity | HIGH |
|
||||
| Ecosystem | micronaut |
|
||||
| Package | io.micronaut.annotation.processing |
|
||||
| File | `inject-java/src/main/java/io/micronaut/annotation/processing/SuperclassAwareTypeVisitor.java` |
|
||||
| Lines | 143–155 |
|
||||
| Complexity | O(2^D) on diamond interface hierarchies |
|
||||
| Hot path | Micronaut compile-time annotation processing (APT) |
|
||||
|
||||
## Defect
|
||||
|
||||
```java
|
||||
private List<TypeMirror> getInterfaces(DeclaredType declaredType) {
|
||||
Element interfaceElement = declaredType.asElement();
|
||||
var interfaces = new ArrayList<TypeMirror>();
|
||||
if (interfaceElement instanceof TypeElement interfaceTypeElement) {
|
||||
for (TypeMirror anInterface : interfaceTypeElement.getInterfaces()) {
|
||||
if (anInterface instanceof DeclaredType interfaceType) {
|
||||
interfaces.add(interfaceType); // no visited guard
|
||||
interfaces.addAll(getInterfaces(interfaceType)); // unconditional recursion
|
||||
}
|
||||
}
|
||||
}
|
||||
return interfaces;
|
||||
}
|
||||
```
|
||||
|
||||
The caller (`visitDeclared`, lines 117–136) calls `getInterfaces` recursively to collect the full
|
||||
transitive interface set, then deduplicates with `.distinct()`. The deduplication happens on the
|
||||
resulting list, not during traversal. In a diamond topology where interface `I3` is a parent of
|
||||
both `I1` and `I2`, and `I4` implements both `I1` and `I2`, `I3` is traversed 2^(D-1) times
|
||||
before `.distinct()` is called — O(2^D) stack depth and list allocations.
|
||||
|
||||
## Fix
|
||||
|
||||
Pass a visited set through the recursion to guard re-entry:
|
||||
|
||||
```java
|
||||
private List<TypeMirror> getInterfaces(DeclaredType declaredType) {
|
||||
var interfaces = new ArrayList<TypeMirror>();
|
||||
collectInterfaces(declaredType, interfaces, new LinkedHashSet<>());
|
||||
return interfaces;
|
||||
}
|
||||
|
||||
private void collectInterfaces(DeclaredType declaredType,
|
||||
List<TypeMirror> result,
|
||||
Set<TypeMirror> visited) {
|
||||
Element interfaceElement = declaredType.asElement();
|
||||
if (interfaceElement instanceof TypeElement interfaceTypeElement) {
|
||||
for (TypeMirror anInterface : interfaceTypeElement.getInterfaces()) {
|
||||
if (anInterface instanceof DeclaredType interfaceType) {
|
||||
if (visited.add(interfaceType)) { // guard: skip if already visited
|
||||
result.add(interfaceType);
|
||||
collectInterfaces(interfaceType, result, visited);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
The `.distinct()` call in the caller can be kept for safety but becomes a no-op.
|
||||
|
||||
## Speedup
|
||||
|
||||
| D | getInterfaces calls (before) | getInterfaces calls (after) | Speedup |
|
||||
|---|-----------------------------|-----------------------------|---------|
|
||||
| 5 | 31 | 5 | 6× |
|
||||
| 10 | 1,023 | 10 | 102× |
|
||||
| 15 | 32,767 | 15 | 2,184× |
|
||||
| 20 | 1,048,575 | 20 | 52,428× |
|
||||
|
||||
This method runs during Micronaut's compile-time APT processing for every class visited. Projects
|
||||
with shared interface hierarchies (e.g., a base `Repository` interface extended by multiple domain
|
||||
interfaces) directly trigger the diamond case. At D=10 the traversal is 1000× more expensive than
|
||||
necessary, degrading build times and memory consumption.
|
||||
|
|
@ -0,0 +1,74 @@
|
|||
# UNDF: (pending)
|
||||
# quarkus-0005: ConfigMappingUtils.collectInterfacesRec — O(2^D) diamond interface re-traversal
|
||||
|
||||
## CWE-407 — Algorithmic Complexity: O(2^D) recursive diamond re-traversal in config interface collection
|
||||
|
||||
| Field | Value |
|
||||
|-------|-------|
|
||||
| ID | quarkus-0005 |
|
||||
| Severity | HIGH |
|
||||
| Ecosystem | quarkus |
|
||||
| Package | io.quarkus.deployment.configuration |
|
||||
| File | `core/deployment/src/main/java/io/quarkus/deployment/configuration/ConfigMappingUtils.java` |
|
||||
| Lines | 262–275 |
|
||||
| Complexity | O(2^D) on diamond interface hierarchies |
|
||||
| Hot path | Quarkus build-time config mapping discovery (startup) |
|
||||
|
||||
## Defect
|
||||
|
||||
```java
|
||||
private static void collectInterfacesRec(ClassInfo current, IndexView index, Set<DotName> result) {
|
||||
List<DotName> interfaces = current.interfaceNames();
|
||||
if (interfaces.isEmpty()) {
|
||||
return;
|
||||
}
|
||||
for (DotName iface : interfaces) {
|
||||
ClassInfo classByName = index.getClassByName(iface);
|
||||
if (classByName == null) {
|
||||
continue; // just ignore this type
|
||||
}
|
||||
result.add(iface); // return value ignored
|
||||
collectInterfacesRec(classByName, index, result); // unconditional recursion
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
`result.add(iface)` returns `false` when `iface` is already in the set (diamond topology), but
|
||||
the return value is discarded. `collectInterfacesRec` recurses unconditionally into `classByName`
|
||||
regardless. In a depth-D diamond hierarchy (`C` implements `I1` and `I2`, both extend `I3`) the
|
||||
shared ancestor `I3` is traversed 2^(D-1) times.
|
||||
|
||||
## Fix
|
||||
|
||||
Guard the recursion on the return value of `Set.add`:
|
||||
|
||||
```java
|
||||
private static void collectInterfacesRec(ClassInfo current, IndexView index, Set<DotName> result) {
|
||||
List<DotName> interfaces = current.interfaceNames();
|
||||
if (interfaces.isEmpty()) {
|
||||
return;
|
||||
}
|
||||
for (DotName iface : interfaces) {
|
||||
ClassInfo classByName = index.getClassByName(iface);
|
||||
if (classByName == null) {
|
||||
continue; // just ignore this type
|
||||
}
|
||||
if (result.add(iface)) { // guard: skip if already visited
|
||||
collectInterfacesRec(classByName, index, result);
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
## Speedup
|
||||
|
||||
| D | Nodes visited (before) | Nodes visited (after) | Speedup |
|
||||
|---|------------------------|----------------------|---------|
|
||||
| 5 | 31 | 5 | 6× |
|
||||
| 10 | 1,023 | 10 | 102× |
|
||||
| 15 | 32,767 | 15 | 2,184× |
|
||||
| 20 | 1,048,575 | 20 | 52,428× |
|
||||
|
||||
This method runs during Quarkus build-time deployment processing of `@ConfigMapping` interfaces.
|
||||
Config interfaces with shared parent interfaces (common in layered config hierarchies) trigger
|
||||
the diamond case. At D=10 the difference is 1000× — critical for build performance and memory.
|
||||
|
|
@ -0,0 +1,96 @@
|
|||
# UNDF: (pending)
|
||||
# rails-0019: ActionView::Digestor#dependency_digest — O(N²) cycle-detection via Array#include?
|
||||
|
||||
## CWE-407 — Algorithmic Complexity: O(N²) template dependency stack scan
|
||||
|
||||
| Field | Value |
|
||||
|-------|-------|
|
||||
| ID | rails-0019 |
|
||||
| Severity | MEDIUM |
|
||||
| Ecosystem | ruby/rails |
|
||||
| Package | actionview |
|
||||
| File | `actionview/lib/action_view/digestor.rb` |
|
||||
| Lines | 97–107 |
|
||||
| Complexity | O(N²) for linear template dependency chains of depth N |
|
||||
| Hot path | Called on every cache-miss template digest computation during asset compilation and development-mode rendering |
|
||||
|
||||
## Defect
|
||||
|
||||
```ruby
|
||||
# actionview/lib/action_view/digestor.rb lines 97-107
|
||||
def dependency_digest(finder, stack)
|
||||
children.map do |node|
|
||||
if stack.include?(node) # DEFECT: Array#include? is O(N) — scans entire stack
|
||||
false
|
||||
else
|
||||
finder.digest_cache[node.name] ||= begin
|
||||
stack.push node # grows unboundedly
|
||||
node.digest(finder, stack).tap { stack.pop }
|
||||
end
|
||||
end
|
||||
end.join("-")
|
||||
end
|
||||
```
|
||||
|
||||
`stack` is a plain `Array`. For a chain of depth N (layout → partial_1 → partial_2 → … →
|
||||
partial_N), each level scans the entire stack to detect cycles:
|
||||
|
||||
| Depth | stack.include? cost | Cumulative |
|
||||
|-------|--------------------|-----------:|
|
||||
| 1 | 0 | 0 |
|
||||
| 2 | 1 | 1 |
|
||||
| 3 | 2 | 3 |
|
||||
| k | k−1 | k(k−1)/2 |
|
||||
| N | N−1 | **O(N²)** |
|
||||
|
||||
Note: diamond template sharing (A→B→D and A→C→D) is *already handled* by
|
||||
`finder.digest_cache` — D is cached after first traversal and returned immediately on the
|
||||
second visit. The O(N²) cost arises purely from the cycle-detection guard on deep linear
|
||||
chains, not from diamond re-traversal.
|
||||
|
||||
The caller `digest(finder, stack = [])` initialises `stack` as a fresh `[]` per top-level
|
||||
call, so there is no cross-tree pollution, but the per-call cost is O(N²) in chain depth.
|
||||
|
||||
## Fix
|
||||
|
||||
Replace the `Array` stack with a `Set` (or use `compare_by_identity` on a `Set` already
|
||||
used nearby in `to_dep_map`):
|
||||
|
||||
```ruby
|
||||
# AFTER — O(N) total: Set#include? is O(1) amortised
|
||||
def digest(finder, stack = Set.new.compare_by_identity)
|
||||
ActiveSupport::Digest.hexdigest("#{template.source}-#{dependency_digest(finder, stack)}")
|
||||
end
|
||||
|
||||
def dependency_digest(finder, stack)
|
||||
children.map do |node|
|
||||
if stack.include?(node) # O(1) identity check
|
||||
false
|
||||
else
|
||||
finder.digest_cache[node.name] ||= begin
|
||||
stack.add(node)
|
||||
node.digest(finder, stack).tap { stack.delete(node) }
|
||||
end
|
||||
end
|
||||
end.join("-")
|
||||
end
|
||||
```
|
||||
|
||||
`Set#compare_by_identity` matches the existing `to_dep_map(seen = Set.new.compare_by_identity)`
|
||||
pattern in the same file (line 110), ensuring object identity is used for cycle detection
|
||||
(consistent with the original `Array#include?` behaviour which also uses `==` identity for
|
||||
`Node` objects with no custom `==`).
|
||||
|
||||
## Speedup
|
||||
|
||||
| Chain depth N | Before (ops) | After (ops) | Speedup |
|
||||
|--------------|-------------|------------|---------|
|
||||
| 10 | 45 | 10 | 4.5× |
|
||||
| 50 | 1,225 | 50 | 24.5× |
|
||||
| 100 | 4,950 | 100 | 49.5× |
|
||||
| 500 | 124,750 | 500 | 249.5× |
|
||||
| 1,000 | 499,500 | 1,000 | 499.5× |
|
||||
|
||||
Template dependency chains of depth 50–100 are realistic in large Rails applications with
|
||||
nested layouts, shared partials, and component hierarchies. At depth 500 (possible in
|
||||
generated or framework-heavy view hierarchies) this is a 249× regression.
|
||||
24
defects/rustc/patch/CLEAN.md
Normal file
24
defects/rustc/patch/CLEAN.md
Normal file
|
|
@ -0,0 +1,24 @@
|
|||
# rustc — CWE-407 Diamond Recursion Scan — CLEAN
|
||||
|
||||
Scanned: `compiler/rustc_trait_selection/src/`, `compiler/rustc_infer/src/traits/`, `compiler/rustc_middle/src/ty/`
|
||||
|
||||
(Note: sparse clone — only `rustc_infer`, `rustc_middle`, `rustc_trait_selection` crates present.
|
||||
`rustc_type_ir` contains the `Elaborator` struct; it re-exports into scope via `pub use rustc_middle::ty::elaborate::*`.)
|
||||
|
||||
## Functions Examined
|
||||
|
||||
| Function | File | Guard | Verdict |
|
||||
|----------|------|-------|---------|
|
||||
| `transitive_bounds_that_define_assoc_item` | rustc_infer/traits/util.rs | `if !seen.insert(...) { continue; }` | CLEAN |
|
||||
| vtable DFS loop | rustc_trait_selection/traits/vtable.rs | `visited.insert(super_trait)` as guard in `.find()` | CLEAN |
|
||||
| `auto_trait` predicate loop | rustc_trait_selection/traits/auto_trait.rs | `if !already_visited.insert(pred)` | CLEAN |
|
||||
| `seen_projection_preds` | rustc_trait_selection/traits/util.rs | `if !seen_projection_preds.insert(...)` | CLEAN |
|
||||
| `checked_wf_args` | rustc_trait_selection/src/traits/query/... | `if !checked_wf_args.insert(arg)` | CLEAN |
|
||||
|
||||
## Note
|
||||
|
||||
The `elaborate` iterator in `rustc_type_ir::elaborate` (not cloned) is called as a BFS/worklist
|
||||
iterator, not as a recursive function. The sparse-clone boundary stops here; the pattern as used
|
||||
through all call sites in the 3 available crates is iterator-based with deduplication guards.
|
||||
|
||||
**Scan verdict: CLEAN — no CWE-407 diamond recursion defects found in cloned crates.**
|
||||
|
|
@ -1,11 +1,11 @@
|
|||
# UNDF: (pending)
|
||||
# spring-0001: AnnotationsScanner.processClassHierarchy — O(2^D) diamond annotation re-traversal
|
||||
# spring-0007: AnnotationsScanner.processClassHierarchy — O(2^D) diamond annotation re-traversal
|
||||
|
||||
## CWE-407 — Algorithmic Complexity: O(2^D) recursive diamond re-traversal
|
||||
|
||||
| Field | Value |
|
||||
|-------|-------|
|
||||
| ID | spring-0001 |
|
||||
| ID | spring-0007 |
|
||||
| Severity | HIGH |
|
||||
| Ecosystem | spring-framework |
|
||||
| Package | org.springframework.core.annotation |
|
||||
|
|
@ -0,0 +1,117 @@
|
|||
# UNDF: (pending)
|
||||
# typescript-0005: hasBaseType — O(2^D) diamond interface/class hierarchy re-traversal
|
||||
|
||||
## CWE-407 — Algorithmic Complexity: O(2^D) recursive diamond re-traversal
|
||||
|
||||
| Field | Value |
|
||||
|-------|-------|
|
||||
| ID | typescript-0005 |
|
||||
| Severity | HIGH |
|
||||
| Ecosystem | typescript |
|
||||
| Package | `typescript` (compiler internals) |
|
||||
| File | `src/compiler/checker.ts` |
|
||||
| Lines | 13029–13041 |
|
||||
| Complexity | O(2^D) on diamond interface/class hierarchies |
|
||||
| Hot path | Type assignability checking (`isTypeDerivedFrom` → `hasBaseType`) |
|
||||
|
||||
## Background
|
||||
|
||||
`hasBaseType(type, checkBase)` tests whether `checkBase` appears anywhere in the
|
||||
transitive base-type hierarchy of `type`. It recurses through `getBaseTypes(target)`
|
||||
with an inner `check` closure that carries **no visited set**.
|
||||
|
||||
TypeScript interfaces support multiple inheritance:
|
||||
|
||||
```typescript
|
||||
interface A {}
|
||||
interface B extends A {}
|
||||
interface C extends A {}
|
||||
interface D extends B, C {} // diamond: A reachable via B and via C
|
||||
```
|
||||
|
||||
For such a diamond at depth D, `check` re-visits every shared ancestor exponentially.
|
||||
|
||||
## Defect
|
||||
|
||||
```typescript
|
||||
// src/compiler/checker.ts lines 13029-13041
|
||||
function hasBaseType(type: Type, checkBase: Type | undefined) {
|
||||
return check(type);
|
||||
function check(type: Type): boolean {
|
||||
if (getObjectFlags(type) & (ObjectFlags.ClassOrInterface | ObjectFlags.Reference)) {
|
||||
const target = getTargetType(type) as InterfaceType;
|
||||
return target === checkBase || some(getBaseTypes(target), check);
|
||||
// ^^^ DEFECT: no visited guard — re-enters
|
||||
// shared ancestors exponentially on diamond hierarchies
|
||||
}
|
||||
else if (type.flags & TypeFlags.Intersection) {
|
||||
return some((type as IntersectionType).types, check);
|
||||
// ^^^ same issue for intersection types
|
||||
}
|
||||
return false;
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
`some(getBaseTypes(target), check)` calls `check` on every direct base type.
|
||||
If two branches both reach a shared ancestor, that ancestor's entire subtree is
|
||||
traversed again. No memoisation prevents this.
|
||||
|
||||
### Call sites (hot paths)
|
||||
|
||||
```
|
||||
isTypeDerivedFrom() → hasBaseType() (type assignability; called in isRelatedTo)
|
||||
resolveBaseTypesOfInterface() → hasBaseType()
|
||||
getBaseTypeNodeOfClass() → hasBaseType()
|
||||
checkAccessOfProtectedMember → hasBaseType()
|
||||
```
|
||||
|
||||
`isTypeDerivedFrom` is called from the core type-relation machinery for every
|
||||
assignability check involving class/interface types.
|
||||
|
||||
## Complexity table
|
||||
|
||||
| Diamond depth D | `check` calls (before fix) | `check` calls (after fix) | Speedup |
|
||||
|-----------------|---------------------------|--------------------------|---------|
|
||||
| 1 | 2 | 2 | 1× |
|
||||
| 5 | 31 | 5 | 6× |
|
||||
| 10 | 1,023 | 10 | 102× |
|
||||
| 15 | 32,767 | 15 | 2,185× |
|
||||
| 20 | 1,048,575 | 20 | 52,429× |
|
||||
|
||||
## Fix
|
||||
|
||||
Add a `visited` Set to the outer function and guard with `has()` before recursing:
|
||||
|
||||
```typescript
|
||||
function hasBaseType(type: Type, checkBase: Type | undefined) {
|
||||
const visited = new Set<Type>(); // FIX: per-call visited set
|
||||
return check(type);
|
||||
function check(type: Type): boolean {
|
||||
if (getObjectFlags(type) & (ObjectFlags.ClassOrInterface | ObjectFlags.Reference)) {
|
||||
const target = getTargetType(type) as InterfaceType;
|
||||
if (target === checkBase) return true;
|
||||
if (visited.has(target)) return false; // FIX: guard
|
||||
visited.add(target);
|
||||
return some(getBaseTypes(target), check);
|
||||
}
|
||||
else if (type.flags & TypeFlags.Intersection) {
|
||||
return some((type as IntersectionType).types, check);
|
||||
}
|
||||
return false;
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
The `visited` set is allocated once per `hasBaseType` call and shared across the
|
||||
entire recursive descent. `Set.has()` / `Set.add()` are O(1) amortised.
|
||||
|
||||
## Notes
|
||||
|
||||
- `getBaseTypes(target)` is already memoised via `target.resolvedBaseTypes`; the
|
||||
defect is purely in the recursive descent of `check`, not in type-set resolution.
|
||||
- Intersection branches do not themselves recurse into `getBaseTypes` and so are
|
||||
bounded by the width of the intersection, not exponential; however they benefit
|
||||
from the same visited guard when the intersection members are shared.
|
||||
- TypeScript's structural type system does permit arbitrarily deep diamond interface
|
||||
hierarchies in user code; library authors composing mixins can easily reach D≥10.
|
||||
|
|
@ -0,0 +1,94 @@
|
|||
# UNDF: (pending)
|
||||
# weld-0005: Services.identifyServiceInterfaces — O(2^D) diamond interface re-traversal
|
||||
|
||||
## CWE-407 — Algorithmic Complexity: O(2^D) recursive diamond re-traversal in CDI service registry discovery
|
||||
|
||||
| Field | Value |
|
||||
|-------|-------|
|
||||
| ID | weld-0005 |
|
||||
| Severity | HIGH |
|
||||
| Ecosystem | weld |
|
||||
| Package | org.jboss.weld.util |
|
||||
| File | `impl/src/main/java/org/jboss/weld/util/Services.java` |
|
||||
| Lines | 50–65 |
|
||||
| Complexity | O(2^D) on diamond interface hierarchies |
|
||||
| Hot path | Weld CDI bootstrap — service registry population (startup) |
|
||||
|
||||
## Defect
|
||||
|
||||
```java
|
||||
public static Set<Class<? extends Service>> identifyServiceInterfaces(Class<?> clazz,
|
||||
Set<Class<? extends Service>> serviceInterfaces) {
|
||||
if (clazz == null || Object.class.equals(clazz) || BootstrapService.class.equals(clazz)) {
|
||||
return serviceInterfaces;
|
||||
}
|
||||
for (Class<?> interfac3 : clazz.getInterfaces()) {
|
||||
if (Service.class.equals(interfac3) || BootstrapService.class.equals(interfac3)) {
|
||||
serviceInterfaces.add(Reflections.cast(clazz));
|
||||
}
|
||||
}
|
||||
for (Class<?> interfac3 : clazz.getInterfaces()) {
|
||||
identifyServiceInterfaces(interfac3, serviceInterfaces); // unconditional recursion
|
||||
}
|
||||
identifyServiceInterfaces(clazz.getSuperclass(), serviceInterfaces); // unconditional recursion
|
||||
return serviceInterfaces;
|
||||
}
|
||||
```
|
||||
|
||||
The method has no visited guard. `serviceInterfaces` accumulates *results* (classes that directly
|
||||
implement `Service`), but it is not used to guard traversal — an interface class `I3` shared by
|
||||
two branches of a diamond hierarchy is passed to `identifyServiceInterfaces` twice per diamond
|
||||
level, giving O(2^D) recursive calls. The method iterates `clazz.getInterfaces()` twice (once for
|
||||
result collection, once for recursion), doubling the constant factor.
|
||||
|
||||
A custom `BootstrapService` implementation with a layered interface hierarchy (e.g., a monitoring
|
||||
service that extends multiple diagnostic interfaces that share a common `Metrics` parent) reaches
|
||||
the diamond case. At D=10 the traversal calls the method 1,024 times vs 10 with a visited guard.
|
||||
|
||||
## Fix
|
||||
|
||||
Add a visited guard using the `clazz` itself as the key:
|
||||
|
||||
```java
|
||||
public static Set<Class<? extends Service>> identifyServiceInterfaces(Class<?> clazz,
|
||||
Set<Class<? extends Service>> serviceInterfaces) {
|
||||
return identifyServiceInterfaces(clazz, serviceInterfaces, new HashSet<>());
|
||||
}
|
||||
|
||||
private static Set<Class<? extends Service>> identifyServiceInterfaces(Class<?> clazz,
|
||||
Set<Class<? extends Service>> serviceInterfaces, Set<Class<?>> visited) {
|
||||
if (clazz == null || Object.class.equals(clazz) || BootstrapService.class.equals(clazz)) {
|
||||
return serviceInterfaces;
|
||||
}
|
||||
if (!visited.add(clazz)) { // guard: skip if already visited
|
||||
return serviceInterfaces;
|
||||
}
|
||||
for (Class<?> interfac3 : clazz.getInterfaces()) {
|
||||
if (Service.class.equals(interfac3) || BootstrapService.class.equals(interfac3)) {
|
||||
serviceInterfaces.add(Reflections.cast(clazz));
|
||||
}
|
||||
}
|
||||
for (Class<?> interfac3 : clazz.getInterfaces()) {
|
||||
identifyServiceInterfaces(interfac3, serviceInterfaces, visited);
|
||||
}
|
||||
identifyServiceInterfaces(clazz.getSuperclass(), serviceInterfaces, visited);
|
||||
return serviceInterfaces;
|
||||
}
|
||||
```
|
||||
|
||||
Alternatively, since the public API accepts a `Set`, the visited guard can reuse a local `HashSet`
|
||||
passed through a private overload as shown above.
|
||||
|
||||
## Speedup
|
||||
|
||||
| D | Recursive calls (before) | Recursive calls (after) | Speedup |
|
||||
|---|--------------------------|------------------------|---------|
|
||||
| 5 | 31 | 5 | 6× |
|
||||
| 10 | 1,023 | 10 | 102× |
|
||||
| 15 | 32,767 | 15 | 2,184× |
|
||||
| 20 | 1,048,575 | 20 | 52,428× |
|
||||
|
||||
This method is called during Weld CDI container initialization to build the service registry.
|
||||
An application with custom CDI extensions implementing a layered `Service` hierarchy directly
|
||||
triggers the diamond case. At D=10, startup time for service discovery is 1000× worse than
|
||||
necessary.
|
||||
Loading…
Add table
Add a link
Reference in a new issue