netty-0001: DnsResolveContext finalResult ArrayList.contains O(A²) → LinkedHashSet O(A)
This commit is contained in:
parent
192219a62f
commit
be66177402
3 changed files with 231 additions and 0 deletions
41
defects/netty/patch/netty-0001.patch
Normal file
41
defects/netty/patch/netty-0001.patch
Normal file
|
|
@ -0,0 +1,41 @@
|
|||
# UNDF: UNDF-2026-000000706
|
||||
--- a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsResolveContext.java
|
||||
+++ b/resolver-dns/src/main/java/io/netty/resolver/dns/DnsResolveContext.java
|
||||
@@ -55,6 +55,7 @@ import java.util.List;
|
||||
import java.util.Locale;
|
||||
import java.util.Map;
|
||||
import java.util.NoSuchElementException;
|
||||
+import java.util.LinkedHashSet;
|
||||
import java.util.Set;
|
||||
|
||||
@@ -117,6 +118,8 @@ abstract class DnsResolveContext<T> {
|
||||
|
||||
private List<T> finalResult;
|
||||
+ // Tracks items already in finalResult for O(1) duplicate detection.
|
||||
+ // Only allocated when isDuplicateAllowed() returns false (address resolution).
|
||||
+ private Set<T> finalResultSet;
|
||||
private int allowedQueries;
|
||||
private boolean triedCNAME;
|
||||
|
||||
@@ -903,17 +906,15 @@ abstract class DnsResolveContext<T> {
|
||||
if (!promise.isDone()) {
|
||||
- // We want to ensure we do not have duplicates in finalResult as this may be unexpected.
|
||||
- //
|
||||
- // While using a LinkedHashSet or HashSet may sound like the perfect fit for this we will use an
|
||||
- // ArrayList here as duplicates should be found quite unfrequently in the wild and we dont want to pay
|
||||
- // for the extra memory copy and allocations in this cases later on.
|
||||
if (finalResult == null) {
|
||||
finalResult = new ArrayList<T>(8);
|
||||
+ if (!isDuplicateAllowed()) {
|
||||
+ finalResultSet = new LinkedHashSet<T>(8);
|
||||
+ }
|
||||
finalResult.add(converted);
|
||||
- } else if (isDuplicateAllowed() || !finalResult.contains(converted)) {
|
||||
+ if (finalResultSet != null) {
|
||||
+ finalResultSet.add(converted);
|
||||
+ }
|
||||
+ } else if (isDuplicateAllowed() || finalResultSet.add(converted)) {
|
||||
+ // finalResultSet.add() returns false if already present: O(1) duplicate check
|
||||
finalResult.add(converted);
|
||||
} else {
|
||||
shouldRelease = true;
|
||||
BIN
defects/netty/unit/NettyDnsDeduplicateTest.class
Normal file
BIN
defects/netty/unit/NettyDnsDeduplicateTest.class
Normal file
Binary file not shown.
190
defects/netty/unit/NettyDnsDeduplicateTest.java
Normal file
190
defects/netty/unit/NettyDnsDeduplicateTest.java
Normal file
|
|
@ -0,0 +1,190 @@
|
|||
package unit;
|
||||
|
||||
/**
|
||||
* Regression test for netty-0001: CWE-407 O(A²) duplicate-address check in
|
||||
* DnsResolveContext.finishResolve().
|
||||
*
|
||||
* File: resolver-dns/src/main/java/io/netty/resolver/dns/DnsResolveContext.java
|
||||
* Lines 906–918 (approx, varies by Netty version)
|
||||
*
|
||||
* When processing DNS answer records, the code accumulates resolved addresses in
|
||||
* an ArrayList<T> and, for each new record, calls finalResult.contains(converted)
|
||||
* to detect duplicates. ArrayList.contains() is O(A) where A = number of already
|
||||
* accumulated addresses. With A answer records the total cost is O(A²).
|
||||
*
|
||||
* The author explicitly noted in a comment that LinkedHashSet "may sound like the
|
||||
* perfect fit" but chose ArrayList anyway, reasoning that duplicates are rare.
|
||||
* The comment misjudges the hot-path exposure: every DNS response for a CDN or
|
||||
* round-robin hostname (Google, Cloudflare, Akamai) may carry 8–64 A records,
|
||||
* and the contains() scan fires for every record beyond the first.
|
||||
*
|
||||
* Fix: maintain a parallel LinkedHashSet<T> (finalResultSet) alongside the
|
||||
* ArrayList<T> (finalResult). Use finalResultSet.add() — which returns false if
|
||||
* the element is already present — as the O(1) duplicate gate. The ArrayList is
|
||||
* kept so that filterResults() (which sorts by preferred address type) receives a
|
||||
* mutable List. The Set is only allocated when isDuplicateAllowed() returns false
|
||||
* (i.e., DnsAddressResolveContext, the common address-resolution path).
|
||||
*
|
||||
* Complexity: O(A) → O(A) overall (constant factor reduction, no quadratic term).
|
||||
* At A=64 addresses: ~2000 comparisons → ~64.
|
||||
*/
|
||||
public class NettyDnsDeduplicateTest {
|
||||
|
||||
// ---- Defective algorithm (mirrors DnsResolveContext before fix) ----
|
||||
|
||||
/**
|
||||
* Simulates the defective finalResult accumulation loop.
|
||||
* Each new address is checked with ArrayList.contains() — O(A) per check.
|
||||
*/
|
||||
static java.util.List<String> deduplicateDefective(java.util.List<String> incoming) {
|
||||
java.util.List<String> finalResult = null;
|
||||
|
||||
for (String addr : incoming) {
|
||||
if (finalResult == null) {
|
||||
finalResult = new java.util.ArrayList<>(8);
|
||||
finalResult.add(addr);
|
||||
} else if (!finalResult.contains(addr)) { // O(A) — CWE-407
|
||||
finalResult.add(addr);
|
||||
}
|
||||
// duplicate: discard
|
||||
}
|
||||
return finalResult != null ? finalResult : java.util.Collections.emptyList();
|
||||
}
|
||||
|
||||
// ---- Fixed algorithm (mirrors DnsResolveContext after fix) ----
|
||||
|
||||
/**
|
||||
* Uses a parallel LinkedHashSet for O(1) duplicate detection.
|
||||
* Insertion order is preserved; the ArrayList is kept for post-processing.
|
||||
*/
|
||||
static java.util.List<String> deduplicateFixed(java.util.List<String> incoming) {
|
||||
java.util.List<String> finalResult = null;
|
||||
java.util.Set<String> finalResultSet = null;
|
||||
|
||||
for (String addr : incoming) {
|
||||
if (finalResult == null) {
|
||||
finalResult = new java.util.ArrayList<>(8);
|
||||
finalResultSet = new java.util.LinkedHashSet<>(8);
|
||||
finalResult.add(addr);
|
||||
finalResultSet.add(addr);
|
||||
} else if (finalResultSet.add(addr)) { // O(1) — returns false if duplicate
|
||||
finalResult.add(addr);
|
||||
}
|
||||
// duplicate: discard
|
||||
}
|
||||
return finalResult != null ? finalResult : java.util.Collections.emptyList();
|
||||
}
|
||||
|
||||
// ---- Tests ----
|
||||
|
||||
public static void main(String[] args) {
|
||||
testCorrectnessNoDuplicates();
|
||||
testCorrectnessWithDuplicates();
|
||||
testCorrectnessAllDuplicates();
|
||||
testCorrectnessPreservesOrder();
|
||||
testPerformance();
|
||||
System.out.println("All netty-0001 CWE-407 unit tests passed.");
|
||||
}
|
||||
|
||||
static void testCorrectnessNoDuplicates() {
|
||||
java.util.List<String> input = java.util.Arrays.asList(
|
||||
"1.2.3.4", "1.2.3.5", "1.2.3.6", "1.2.3.7"
|
||||
);
|
||||
java.util.List<String> d = deduplicateDefective(input);
|
||||
java.util.List<String> f = deduplicateFixed(input);
|
||||
assert d.equals(input) : "Defective: " + d;
|
||||
assert f.equals(input) : "Fixed: " + f;
|
||||
assert d.equals(f) : "Mismatch: defective=" + d + " fixed=" + f;
|
||||
System.out.println(" [PASS] no-duplicates: " + f);
|
||||
}
|
||||
|
||||
static void testCorrectnessWithDuplicates() {
|
||||
java.util.List<String> input = java.util.Arrays.asList(
|
||||
"1.2.3.4", "1.2.3.5", "1.2.3.4", "1.2.3.6", "1.2.3.5", "1.2.3.7"
|
||||
);
|
||||
java.util.List<String> expected = java.util.Arrays.asList(
|
||||
"1.2.3.4", "1.2.3.5", "1.2.3.6", "1.2.3.7"
|
||||
);
|
||||
java.util.List<String> d = deduplicateDefective(input);
|
||||
java.util.List<String> f = deduplicateFixed(input);
|
||||
assert d.equals(expected) : "Defective: expected " + expected + " got " + d;
|
||||
assert f.equals(expected) : "Fixed: expected " + expected + " got " + f;
|
||||
System.out.println(" [PASS] with-duplicates: " + f);
|
||||
}
|
||||
|
||||
static void testCorrectnessAllDuplicates() {
|
||||
java.util.List<String> input = java.util.Arrays.asList(
|
||||
"10.0.0.1", "10.0.0.1", "10.0.0.1", "10.0.0.1"
|
||||
);
|
||||
java.util.List<String> expected = java.util.Collections.singletonList("10.0.0.1");
|
||||
java.util.List<String> d = deduplicateDefective(input);
|
||||
java.util.List<String> f = deduplicateFixed(input);
|
||||
assert d.equals(expected) : "Defective: " + d;
|
||||
assert f.equals(expected) : "Fixed: " + f;
|
||||
System.out.println(" [PASS] all-duplicates: " + f);
|
||||
}
|
||||
|
||||
static void testCorrectnessPreservesOrder() {
|
||||
// Insertion order must be preserved — LinkedHashSet guarantees this.
|
||||
java.util.List<String> input = java.util.Arrays.asList(
|
||||
"192.168.1.10", "192.168.1.20", "192.168.1.10", "192.168.1.30",
|
||||
"192.168.1.20", "192.168.1.40"
|
||||
);
|
||||
java.util.List<String> expected = java.util.Arrays.asList(
|
||||
"192.168.1.10", "192.168.1.20", "192.168.1.30", "192.168.1.40"
|
||||
);
|
||||
java.util.List<String> f = deduplicateFixed(input);
|
||||
assert f.equals(expected) : "Order mismatch: expected " + expected + " got " + f;
|
||||
System.out.println(" [PASS] order-preserved: " + f);
|
||||
}
|
||||
|
||||
static void testPerformance() {
|
||||
// Simulate a large DNS response with many duplicate addresses.
|
||||
// CDN hostnames (Akamai, Cloudflare) can return 16–64 A records;
|
||||
// a malformed or adversarial response could include many duplicates.
|
||||
final int DISTINCT = 500; // distinct addresses (worst-case large response)
|
||||
final int DUPLICATES = 2; // each repeated → A = 1000, quadratic dominates
|
||||
final int REPS = 100;
|
||||
|
||||
java.util.List<String> input = new java.util.ArrayList<>(DISTINCT * DUPLICATES);
|
||||
for (int i = 0; i < DISTINCT; i++) {
|
||||
for (int j = 0; j < DUPLICATES; j++) {
|
||||
input.add("10." + (i / 256) + "." + (i % 256) + ".1");
|
||||
}
|
||||
}
|
||||
// Shuffle to simulate non-contiguous duplicate distribution.
|
||||
java.util.Collections.shuffle(input, new java.util.Random(42));
|
||||
|
||||
// Warm up
|
||||
for (int r = 0; r < 20; r++) {
|
||||
deduplicateDefective(input);
|
||||
deduplicateFixed(input);
|
||||
}
|
||||
|
||||
// Defective: O(A²) — ArrayList.contains() per record
|
||||
long t0 = System.nanoTime();
|
||||
for (int r = 0; r < REPS; r++) {
|
||||
deduplicateDefective(input);
|
||||
}
|
||||
long defectiveNs = (System.nanoTime() - t0) / REPS;
|
||||
|
||||
// Fixed: O(A) — LinkedHashSet.add() per record
|
||||
t0 = System.nanoTime();
|
||||
for (int r = 0; r < REPS; r++) {
|
||||
deduplicateFixed(input);
|
||||
}
|
||||
long fixedNs = (System.nanoTime() - t0) / REPS;
|
||||
|
||||
double speedup = (double) defectiveNs / fixedNs;
|
||||
System.out.printf(
|
||||
" [PERF] distinct=%d duplicatesEach=%d totalRecords=%d " +
|
||||
"defective=%.3fms fixed=%.3fms speedup=%.1fx%n",
|
||||
DISTINCT, DUPLICATES, DISTINCT * DUPLICATES,
|
||||
defectiveNs / 1_000_000.0,
|
||||
fixedNs / 1_000_000.0,
|
||||
speedup);
|
||||
|
||||
assert speedup > 2.0
|
||||
: "Expected >2x speedup at A=" + (DISTINCT * DUPLICATES) + ", got " + speedup + "x";
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue