From be661774027f94d95166bed52c6c7577969a2dea Mon Sep 17 00:00:00 2001 From: "russell@unturf.com" Date: Mon, 30 Mar 2026 08:26:27 -0400 Subject: [PATCH] =?UTF-8?q?netty-0001:=20DnsResolveContext=20finalResult?= =?UTF-8?q?=20ArrayList.contains=20O(A=C2=B2)=20=E2=86=92=20LinkedHashSet?= =?UTF-8?q?=20O(A)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- defects/netty/patch/netty-0001.patch | 41 ++++ .../netty/unit/NettyDnsDeduplicateTest.class | Bin 0 -> 5288 bytes .../netty/unit/NettyDnsDeduplicateTest.java | 190 ++++++++++++++++++ 3 files changed, 231 insertions(+) create mode 100644 defects/netty/patch/netty-0001.patch create mode 100644 defects/netty/unit/NettyDnsDeduplicateTest.class create mode 100644 defects/netty/unit/NettyDnsDeduplicateTest.java diff --git a/defects/netty/patch/netty-0001.patch b/defects/netty/patch/netty-0001.patch new file mode 100644 index 000000000..d7c066e05 --- /dev/null +++ b/defects/netty/patch/netty-0001.patch @@ -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 { + + private List finalResult; ++ // Tracks items already in finalResult for O(1) duplicate detection. ++ // Only allocated when isDuplicateAllowed() returns false (address resolution). ++ private Set finalResultSet; + private int allowedQueries; + private boolean triedCNAME; + +@@ -903,17 +906,15 @@ abstract class DnsResolveContext { + 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(8); ++ if (!isDuplicateAllowed()) { ++ finalResultSet = new LinkedHashSet(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; diff --git a/defects/netty/unit/NettyDnsDeduplicateTest.class b/defects/netty/unit/NettyDnsDeduplicateTest.class new file mode 100644 index 0000000000000000000000000000000000000000..e1def01e41aec7f82d095d070636023e421df05a GIT binary patch literal 5288 zcmaJ^3wRXO75*oi-PvqLNR|LW0xSqhUYiF9N&pE7NFWKwA~9OjAv;M{c4y+w1QXx3 z*ece(eNm}OTcTB4#THg5YHh2nZSBK8Z0*xNv=7_b&+ltLK7s!C&OVdq=F6RX@0@$i z{m=Q&xp!VVb@Ukkt#~_tB6w63Yw*ISpzJDrx84xf&4Gr^JFhbO90k6WF*D|@R^VA& zy}cBEl&A=3D21kAiWDDqV)2H~m`!1S%rTO>VLSX|wid)<+qZz;tTOjS{)p&UU4 z!HFUY)Ir_uHb$tAHZK(`Dl|;T3i zip>Tv3snO49I9R1;bNPoA%yv~t4E^>rZ098{!f)XP!`o&k~dA0gB!n3-GK88deo(G9vi7%{lvT2z&y z0rONeDmWu2CSNvViHfBf&J}U28fMHIx(&w}Z8z<9BRV`3kM-$}vCR-+n(gq>W+ju{ zm}%H{x7D5zAd+S**KnRtGiRJ!Ps|xClw6^qMH(C@*&2@*3a-+ydNP$;l7?+0cN_NR zWYkCs<7+e$lg6hCm0OHtzm-hrW}i`lHngi)r(r!VER6K1?HCD0%^D`S>2AKw& zz#$C`J<;Mvhk|NuD`X0<)HgOZHig=H*3~aWu!MT21oEx6V3_%o z(03VamPz^x{<3E~y6Mo91+IIasZ zHBVHW_Ob+@-MB|+D#mp|8f4>x?Z*dMX_^|t;;%_mnmrnB#fMn~Y*x6q;aH~2h=uuP zGrJp4h#*v<^L7PV)A`Nere)`ao5D?ve%w(o)6AJW3ucyZ=Hmr3OB;hMhTLU|*mMA& z7VtkKYJW$^B;ZusUBKwAx*4?+UdBrNY67?m`-GoAD=PBF>g@vRehr_)1EhdGINaYK zCtDXyU~3*iS0zIdV6}Ki!vTDrzB6^x+7?T2F%OiD8G}(j#uN;MLYHk>w{`uFP?YHs zGy9xXi=v@y!L--ueS>1gu^c_V)#$U5QCo~d(M$rZS`=Q=pRhxrevu5>kv(LHY~#48 ze`Jvvz$5sQibn;dA5&1C<9&ze7z2hpG7G$xo^SEBhWFeoJQTnacv8h7q2j5GCnbB@ ztzjmbpl5r&-`#1JfKB+ChQm0*+> z#?v|jcm~g^cuvDN#DPCGhr2D#QbDpr4Lg=JqS^G1IJz^;7sYK>l$fUS6g0Yr6FZIM zHc?JUdrI`}>e7eOE$v$I!Ol+{~Jx<=R0MEd!wJgVWlKN1W;S5?)X5-7ut87JH5mNm45!K7; zDn~!zVcZq76S~tkxFU~wN}6s6_F)TM3o05^nP2PpskVGC)>0kA%`W7t)#8Q6@pUe`k*PlXC4e{Zri#A` zy8cbU!WSuyK7%W%{ZyLXfDet0I#mo~ZKY=23E$8rUsq+Q)c=bu6c$Fx`8Dc4&N|%eHn_H7OwzA{m zSW!$MbvYo%P%9zTch$}+b}`k7PFRAQnd#pLE&G1#^rMr)YlcSitGJ!jbmtm6xfN$& zEdjP+Iodh14xPLu_Ymln1lq^?FQCqY?-WWU)CP%mjYKZHJEp1^`y(oAX~!(0un(6t$%weI)^@U)cBTtY42=7CE| zmL1H8%Xu%kLi%=)q_`71aX)=5JX{9PDO@N$Og{NT3%QP8QfbitC1^qau`=Iy)Wq)- z&{g3Ce;WV$_(r(p@-E?H)*atGDg1t%gBU3m zCnpl|py#@;~LX4xV7S|HrbwqeQZs4B? zt;E(f9$Sdm_+6XEc0A4M8!`dDO@P}gz_I5GepCdQLx1pN5@y;3IVlSG+;@4rf6v%{ zA{#c>7oQvl?M!YAxP_2zW!P?G*luUw_7c<`64DeXr;x}7i%?oKP?T4hzAU23Oi{tg zJ#VrWZ}P01U9P-y0Ck>KRo(-bSHmIVq8U1hPXTUK+f$^Boqm0f!uFM#W4Nog0{0xj zeJR|}^<@*DW)4drH zuJlsm?X1Xh@BF8+40TaTY<-|6u!cMa9MWX zuT;h4VSKemrq%39wF0So_9H0jMWuQaM_r$bs^^x5YcZGoyLb=28$^oPxED=)F6Vd^ z3&SQnz*_hqF2O@A9uKfke38fKL1xYv?;!`tv`4T9k1;%t(}!Q?%Ez&vC*{L9#P%sX z%S!YbYuuYi;oo%eJDmF;jwv&6Tn4cLr6+L)gQ&dA{}TwEfcITYWpDaPc#HWgJBc#! zEI)~nVh^MI-At6Rl*ARk@?i5}9N!1Ecnrm!r(opp32uv{L*#%LMcNvEMByb4its&r zpU;gf@_(k#4`lm8+5T9zugLaivi*f@e<|D7WcwT0{!X@kknJDwcj^ 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 (finalResultSet) alongside the + * ArrayList (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 deduplicateDefective(java.util.List incoming) { + java.util.List 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 deduplicateFixed(java.util.List incoming) { + java.util.List finalResult = null; + java.util.Set 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 input = java.util.Arrays.asList( + "1.2.3.4", "1.2.3.5", "1.2.3.6", "1.2.3.7" + ); + java.util.List d = deduplicateDefective(input); + java.util.List 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 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 expected = java.util.Arrays.asList( + "1.2.3.4", "1.2.3.5", "1.2.3.6", "1.2.3.7" + ); + java.util.List d = deduplicateDefective(input); + java.util.List 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 input = java.util.Arrays.asList( + "10.0.0.1", "10.0.0.1", "10.0.0.1", "10.0.0.1" + ); + java.util.List expected = java.util.Collections.singletonList("10.0.0.1"); + java.util.List d = deduplicateDefective(input); + java.util.List 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 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 expected = java.util.Arrays.asList( + "192.168.1.10", "192.168.1.20", "192.168.1.30", "192.168.1.40" + ); + java.util.List 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 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"; + } +}