C#: address review feedback — full RFC1918 172.16/12 coverage, accurate guard-literal comment, tighter NAT64 WKP example

Author: tonghuaroot

csharp/src/security/CWE-918/SsrfIpv6TransitionIncompleteGuard.ql

@@ -32,13 +32,15 @@ predicate hasIsPrivateCall(Callable c) {
 }
 
 /**
- * Holds if `c` contains a hand-written RFC 1918, loopback or cloud-metadata IPv4 literal
- * used as a denylist entry.
+ * Holds if `c` contains a hand-written denylist literal: an RFC 1918 / loopback /
+ * link-local-metadata IPv4 form (`127.0.0.1`, `169.254.169.254`, `10.`, `192.168`, the full
+ * `172.16.0.0/12` range `172.16`-`172.31`), an IPv6 loopback or ULA fragment (`::1`, `fc00`,
+ * `fd00`), or the cloud-metadata hostname fragment `metadata.google`.
  */
 predicate hasRfc1918Literal(Callable c) {
   exists(StringLiteral s | s.getEnclosingCallable() = c |
     s.getValue()
-        .regexpMatch("(?i).*(127\\.0\\.0\\.1|169\\.254\\.169\\.254|10\\.|192\\.168|172\\.1[6-9]|::1|fc00|fd00|metadata\\.google).*")
+        .regexpMatch("(?i).*(127\\.0\\.0\\.1|169\\.254\\.169\\.254|10\\.|192\\.168|172\\.(1[6-9]|2[0-9]|3[01])|::1|fc00|fd00|metadata\\.google).*")
   )
 }
 

csharp/src/security/CWE-918/examples/SsrfIpv6TransitionIncompleteGuardGood.cs

@@ -12,8 +12,13 @@ public class GoodFetcher
     private static IPAddress UnwrapTransition(IPAddress addr)
     {
         byte[] b = addr.GetAddressBytes();
-        // NAT64 well-known prefix 64:ff9b::/96 -> last 4 bytes are the embedded IPv4.
-        if (b.Length == 16 && b[0] == 0x00 && b[1] == 0x64 && b[2] == 0xff && b[3] == 0x9b)
+        // NAT64 well-known prefix 64:ff9b::/96 -> last 4 bytes are the embedded IPv4. The full
+        // /96 must be matched: bytes 0..3 are 00 64 ff 9b and bytes 4..11 are all zero, so an
+        // address that merely starts with 64:ff9b but carries non-zero middle bytes is not
+        // mistaken for a NAT64 address and is left unwrapped.
+        if (b.Length == 16 && b[0] == 0x00 && b[1] == 0x64 && b[2] == 0xff && b[3] == 0x9b
+            && b[4] == 0 && b[5] == 0 && b[6] == 0 && b[7] == 0
+            && b[8] == 0 && b[9] == 0 && b[10] == 0 && b[11] == 0)
         {
             return new IPAddress(new[] { b[12], b[13], b[14], b[15] });
         }
@@ -36,7 +41,9 @@ private static bool IsPrivateHost(string host)
         byte[] b = addr.GetAddressBytes();
         return b.Length == 4
             && (b[0] == 127 || b[0] == 10 || (b[0] == 169 && b[1] == 254)
-                || (b[0] == 192 && b[1] == 168) || (b[0] == 172 && b[1] == 16));
+                || (b[0] == 192 && b[1] == 168)
+                // Full RFC 1918 172.16.0.0/12 range: second octet 16..31.
+                || (b[0] == 172 && b[1] >= 16 && b[1] <= 31));
     }
 
     public static async Task<string> FetchAsync(string host)

csharp/test/security/CWE-918/SsrfIpv6TransitionIncompleteGuard/SsrfIpv6TransitionIncompleteGuard.cs

@@ -19,6 +19,17 @@ public static bool ValidateTargetHost(string host) // NOT OK
             return true;
         }
 
+        // BAD: a denylist covering the upper half of the RFC 1918 172.16.0.0/12 block
+        // (172.20-172.31). The query must recognize the full /12 range, not just 172.16-172.19.
+        public static bool CheckTargetIp(string host) // NOT OK
+        {
+            if (host.StartsWith("172.20") || host.StartsWith("172.31"))
+            {
+                throw new Exception("blocked internal host");
+            }
+            return true;
+        }
+
         // BAD: an `IsPrivateHost`-named guard that only does the partial `::ffff:` unwrap via
         // `IsIPv4MappedToIPv6` / `MapToIPv4`, leaving NAT64 and 6to4 forms live.
         public static bool IsPrivateHostAddress(FakeIPAddress addr) // NOT OK

csharp/test/security/CWE-918/SsrfIpv6TransitionIncompleteGuard/SsrfIpv6TransitionIncompleteGuard.expected

@@ -1,2 +1,3 @@
 | SsrfIpv6TransitionIncompleteGuard.cs:9:28:9:45 | ValidateTargetHost | This SSRF host guard rejects private IPv4 ranges but never unwraps IPv6-transition forms (IPv4-mapped '::ffff:', NAT64 '64:ff9b::', 6to4 '2002::'); an attacker can wrap an internal IPv4 address in a transition literal to bypass it and reach internal endpoints. |
-| SsrfIpv6TransitionIncompleteGuard.cs:24:28:24:47 | IsPrivateHostAddress | This SSRF host guard rejects private IPv4 ranges but never unwraps IPv6-transition forms (IPv4-mapped '::ffff:', NAT64 '64:ff9b::', 6to4 '2002::'); an attacker can wrap an internal IPv4 address in a transition literal to bypass it and reach internal endpoints. |
+| SsrfIpv6TransitionIncompleteGuard.cs:24:28:24:40 | CheckTargetIp | This SSRF host guard rejects private IPv4 ranges but never unwraps IPv6-transition forms (IPv4-mapped '::ffff:', NAT64 '64:ff9b::', 6to4 '2002::'); an attacker can wrap an internal IPv4 address in a transition literal to bypass it and reach internal endpoints. |
+| SsrfIpv6TransitionIncompleteGuard.cs:35:28:35:47 | IsPrivateHostAddress | This SSRF host guard rejects private IPv4 ranges but never unwraps IPv6-transition forms (IPv4-mapped '::ffff:', NAT64 '64:ff9b::', 6to4 '2002::'); an attacker can wrap an internal IPv4 address in a transition literal to bypass it and reach internal endpoints. |