From 347cb57f0970567363db4a41215cc0679399c48f Mon Sep 17 00:00:00 2001 From: "russell@unturf.com" Date: Sun, 29 Mar 2026 18:50:56 -0400 Subject: [PATCH] =?UTF-8?q?hv-0001/0002:=20DefaultValidationOrder=20groupL?= =?UTF-8?q?ist=20O(N=C2=B2)=2050x=20+=20addInheritedGroups=20diamond=20O(2?= =?UTF-8?q?^D)=2052428x;=20count=20654=E2=86=92656?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ...faultvalidationorder-grouplist-contains.md | 91 +++++++++++++++++++ ...002-sequence-addinheritedgroups-diamond.md | 69 ++++++++++++++ 2 files changed, 160 insertions(+) create mode 100644 defects/hibernate-validator/patch/hv-0001-defaultvalidationorder-grouplist-contains.md create mode 100644 defects/hibernate-validator/patch/hv-0002-sequence-addinheritedgroups-diamond.md diff --git a/defects/hibernate-validator/patch/hv-0001-defaultvalidationorder-grouplist-contains.md b/defects/hibernate-validator/patch/hv-0001-defaultvalidationorder-grouplist-contains.md new file mode 100644 index 000000000..c9011fd4f --- /dev/null +++ b/defects/hibernate-validator/patch/hv-0001-defaultvalidationorder-grouplist-contains.md @@ -0,0 +1,91 @@ +# UNDF: (pending) +# hv-0001: DefaultValidationOrder.insertGroup — groupList.contains() O(N) list scan + +## CWE-407 — Algorithmic Complexity: O(N) List.contains() dedup in validation group ordering + +| Field | Value | +|--------------|-------| +| ID | hv-0001 | +| Severity | MEDIUM | +| Ecosystem | hibernate-validator | +| Package | hibernate-validator | +| File | `engine/src/main/java/org/hibernate/validator/internal/engine/groups/DefaultValidationOrder.java` | +| Lines | 57–65 | +| Complexity | O(N) per call; O(N²) for N groups | +| Hot path | Called per validation group during `ValidatorImpl.validate()` | + +## Defect + +`DefaultValidationOrder.insertGroup` uses `List.contains()` to deduplicate groups: + +```java +// DefaultValidationOrder.java:57-65 (DEFECT) +private List groupList; // initialized with new ArrayList<>(5) + +public void insertGroup(Group group) { + if ( groupList == null ) { + groupList = new ArrayList<>( 5 ); + } + if ( !groupList.contains( group ) ) { // O(N) linear scan + groupList.add( group ); + } +} +``` + +Also in `ensureDefaultGroupSequenceIsExpandable` (line 119): +```java +int index = groupList.indexOf( group ); // O(N) linear scan +``` + +And `ValidationOrderGenerator.addGroups` (lines 166-167): +```java +if ( resolvedGroupSequence.contains( tmpGroup ) // O(N) scan #1 + && resolvedGroupSequence.indexOf( tmpGroup ) < resolvedGroupSequence.size() - 1 ) { // O(N) scan #2 +``` + +Two O(N) scans per group in `addGroups` — O(N²) total. + +## Fix + +Replace `List` with a `LinkedHashSet` for O(1) membership and preserved insertion order: + +```java +// AFTER — O(1) per operation +private Set groupList; // LinkedHashSet preserves insertion order + +public void insertGroup(Group group) { + if ( groupList == null ) { + groupList = new LinkedHashSet<>( 5 ); + } + groupList.add( group ); // O(1) — Set.add() deduplicates automatically +} +``` + +For `ensureDefaultGroupSequenceIsExpandable`, build an index map: +```java +Map groupIndex = new LinkedHashMap<>(); +for (int i = 0; i < sequenceGroups.size(); i++) groupIndex.put(sequenceGroups.get(i), i); +int index = groupIndex.getOrDefault(group, -1); // O(1) +``` + +For `addGroups`, build a position index before the loop: +```java +Map positionMap = new LinkedHashMap<>(); +for (int i = 0; i < resolvedGroupSequence.size(); i++) positionMap.put(resolvedGroupSequence.get(i), i); +for (Group tmpGroup : groups) { + Integer pos = positionMap.get(tmpGroup); + if (pos != null && pos < resolvedGroupSequence.size() - 1) throw LOG.getUnableToExpandGroupSequenceException(); + resolvedGroupSequence.add(tmpGroup); + positionMap.put(tmpGroup, resolvedGroupSequence.size() - 1); +} +``` + +## Speedup + +| Groups (N) | Before (scans) | After (scans) | Speedup | +|------------|----------------|---------------|---------| +| 10 | 55 | 10 | 5.5× | +| 50 | 1,275 | 50 | 25.5× | +| 100 | 5,050 | 100 | 50.5× | + +Growth before: O(N²). Growth after: O(N). diff --git a/defects/hibernate-validator/patch/hv-0002-sequence-addinheritedgroups-diamond.md b/defects/hibernate-validator/patch/hv-0002-sequence-addinheritedgroups-diamond.md new file mode 100644 index 000000000..4a77aae02 --- /dev/null +++ b/defects/hibernate-validator/patch/hv-0002-sequence-addinheritedgroups-diamond.md @@ -0,0 +1,69 @@ +# UNDF: (pending) +# hv-0002: Sequence.addInheritedGroups — O(2^D) diamond re-traversal in group inheritance expansion + +## CWE-407 — Algorithmic Complexity: O(2^D) recursive diamond re-traversal in validation group hierarchy + +| Field | Value | +|--------------|-------| +| ID | hv-0002 | +| Severity | HIGH | +| Ecosystem | hibernate-validator | +| Package | hibernate-validator | +| File | `engine/src/main/java/org/hibernate/validator/internal/engine/groups/Sequence.java` | +| Lines | 130–139 | +| Complexity | O(2^D) on diamond group inheritance hierarchies | +| Hot path | Called during validation order initialization for @GroupSequence | + +## Defect + +`Sequence.addInheritedGroups` recursively expands group interface hierarchies. Although it uses +`Set expandedGroups` to track visited groups, the recursion guard is MISSING — the +method recurses unconditionally even after `expandedGroups.add(g)` returns `false` (indicating +the group was already visited): + +```java +// Sequence.java:130-139 (DEFECT) +private void addInheritedGroups(Group group, Set expandedGroups) { + for ( Class inheritedGroup : group.getDefiningClass().getInterfaces() ) { + if ( isGroupSequence( inheritedGroup ) ) { throw LOG.getSequenceDefinitionsNotAllowedException(); } + Group g = new Group( inheritedGroup ); + expandedGroups.add( g ); // adds to set — returns false if already present + addInheritedGroups( g, expandedGroups ); // DEFECT: recurses unconditionally, ignores add() return value + } +} +``` + +On a diamond group hierarchy (interface D extends A and B; interface A extends BaseGroup; interface +B extends BaseGroup; class C implements D), BaseGroup is traversed twice: +- Via D→A→BaseGroup: `expandedGroups.add(BaseGroup)` succeeds → recurse into BaseGroup +- Via D→B→BaseGroup: `expandedGroups.add(BaseGroup)` fails (already present) → BUT STILL RECURSES into BaseGroup! + +At diamond depth D, BaseGroup is visited 2^D times. + +## Fix + +Guard the recursion with the return value of `expandedGroups.add(g)`: + +```java +// AFTER — O(N+E) where N=groups, E=inheritance edges +private void addInheritedGroups(Group group, Set expandedGroups) { + for ( Class inheritedGroup : group.getDefiningClass().getInterfaces() ) { + if ( isGroupSequence( inheritedGroup ) ) { throw LOG.getSequenceDefinitionsNotAllowedException(); } + Group g = new Group( inheritedGroup ); + if ( expandedGroups.add( g ) ) { // only recurse if newly added + addInheritedGroups( g, expandedGroups ); + } + } +} +``` + +## Speedup + +| Diamond depth (D) | Before (visits) | After (visits) | Speedup | +|------------------|----------------|----------------|---------| +| 5 | 31 | 5 | 6× | +| 10 | 1,023 | 10 | 102× | +| 15 | 32,767 | 15 | 2,184× | +| 20 | 1,048,575 | 20 | 52,428× | + +Growth before: O(2^D). Growth after: O(D).