carverauto/serviceradar

Fork 0

Large CIDR expansion returns only one IP due to non-incremented loop variable #684

New issue

Closed

opened 2026-03-28 04:27:24 +00:00 by mfreeman451 · 0 comments

mfreeman451 commented

2026-03-28 04:27:24 +00:00

Owner

Imported from GitHub.

Original GitHub issue: #2146
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/issues/2146
Original created: 2025-12-16T05:16:44Z

Summary

Context: The collectIPsFromRange function in pkg/mapper/utils.go is responsible for expanding CIDR ranges into individual IP addresses for network discovery, with a limit of 256 IPs for large ranges (> /24).
Bug: In the large range code path (lines 227-236), the loop variable i is initialized but never incremented, while the function increments ip but uses i.String() to get the IP address.
Actual vs. expected: Instead of generating 256 unique, incrementing IP addresses from a large CIDR range, the function only returns 1 IP (the network address), because i remains constant throughout the loop.
Impact: Network discovery on large CIDR ranges (> /24, such as /16 or /8) will only scan a single IP address instead of the intended 256 addresses, causing nearly complete failure of network device discovery for large networks.

Code with bug

if hostBits > defaultHostBitsCheckMin { // More than 256 hosts
    e.logger.Warn().Str("cidr", ipNet.String()).Int("host_bits", hostBits).
        Msg("CIDR range too large, limiting scan")

    // Only scan first 256 IPs
    count := 0
    ipCopy := make(net.IP, len(ip))

    copy(ipCopy, ip)

    for i := ipCopy.Mask(ipNet.Mask); ipNet.Contains(ip) && count < defaultMaxIPRange; incrementIP(ip) {
        // changed from ip to i, to avoid modifying the original IP
        ipStr := i.String()  // <-- BUG 🔴 i is never incremented, always returns the same IP

        if !seen[ipStr] {
            targets = append(targets, ipStr)
            seen[ipStr] = true
            count++
        }
    }
}

The bug is on lines 227-229. The loop declares i := ipCopy.Mask(ipNet.Mask) in the initialization statement but increments ip in the post-statement (incrementIP(ip)). The loop body then uses i.String() to get the IP address. Since i is never modified, it always returns the same IP address (the network address).

Evidence

Example

Consider a user initiating discovery on CIDR range 192.168.0.0/16:

Initial state: ip = 192.168.0.0, ipNet.Mask = /16, hostBits = 16 (> 8, so enters large range path)
Loop initialization: i = ipCopy.Mask(ipNet.Mask) = 192.168.0.0
First iteration:
- i.String() returns "192.168.0.0"
- Added to targets and seen map
- count becomes 1
- incrementIP(ip) changes ip to 192.168.0.1 (but i stays 192.168.0.0)
Second iteration:
- i.String() still returns "192.168.0.0" (because i was never incremented)
- Already in seen map, so not added to targets
- count stays 1
- incrementIP(ip) changes ip to 192.168.0.2
Iterations 3-256: Same as iteration 2 - i never changes, so same IP is checked repeatedly
Loop terminates: When ipNet.Contains(ip) becomes false or when the loop has run 256 times
Result: targets contains only ["192.168.0.0"] instead of 256 unique IPs

The expected behavior would be to return 256 unique, sequential IP addresses: 192.168.0.0, 192.168.0.1, 192.168.0.2, ..., 192.168.0.255.

Failing test

Test script

/*
 * Copyright 2025 Carver Automation Corporation.
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *     http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

package mapper

import (
	"net"
	"testing"
	"time"

	"github.com/carverauto/serviceradar/pkg/logger"
)

// TestCollectIPsFromRange_LargeRange tests that the collectIPsFromRange function
// correctly limits large CIDR ranges to 256 IPs and returns unique incrementing IPs.
func TestCollectIPsFromRange_LargeRange(t *testing.T) {
	// Create a minimal DiscoveryEngine for testing
	engine := &DiscoveryEngine{
		config: &Config{
			Timeout: 5 * time.Second,
			Retries: 3,
		},
		logger: logger.NewTestLogger(),
	}

	// Test with a /16 network (65536 hosts) - should be limited to 256 IPs
	cidr := "192.168.0.0/16"
	ip, ipNet, err := net.ParseCIDR(cidr)
	if err != nil {
		t.Fatalf("Failed to parse CIDR: %v", err)
	}

	ones, bits := ipNet.Mask.Size()
	hostBits := bits - ones // This will be 16 for a /16 network

	seen := make(map[string]bool)
	targets := engine.collectIPsFromRange(ip, ipNet, hostBits, seen)

	// Check that we got 256 unique IPs
	if len(targets) != 256 {
		t.Errorf("Expected 256 IPs for large range, got %d", len(targets))
	}

	// Check that all IPs are unique
	uniqueCheck := make(map[string]bool)
	for _, ip := range targets {
		if uniqueCheck[ip] {
			t.Errorf("Found duplicate IP: %s", ip)
		}
		uniqueCheck[ip] = true
	}

	// Check that IPs are incrementing (not all the same)
	if len(targets) >= 2 {
		first := targets[0]
		allSame := true
		for _, ip := range targets[1:] {
			if ip != first {
				allSame = false
				break
			}
		}
		if allSame {
			t.Errorf("All IPs are the same (%s), expected incrementing IPs", first)
		}
	}

	// Check that the first IP is the network address masked
	expectedFirst := ip.Mask(ipNet.Mask).String()
	if len(targets) > 0 && targets[0] != expectedFirst {
		t.Errorf("Expected first IP to be %s, got %s", expectedFirst, targets[0])
	}

	// Check that IPs are sequential
	if len(targets) >= 10 {
		// Parse first IP and check that the 10th IP is first + 9
		firstIP := net.ParseIP(targets[0])
		tenthIP := net.ParseIP(targets[9])

		if firstIP == nil || tenthIP == nil {
			t.Fatal("Failed to parse IPs")
		}

		// Create expected 10th IP by incrementing first IP 9 times
		expectedTenth := make(net.IP, len(firstIP))
		copy(expectedTenth, firstIP)
		for i := 0; i < 9; i++ {
			incrementIP(expectedTenth)
		}

		if tenthIP.String() != expectedTenth.String() {
			t.Errorf("Expected 10th IP to be %s (first + 9), got %s", expectedTenth.String(), tenthIP.String())
		}
	}
}

Test output

=== RUN   TestCollectIPsFromRange_LargeRange
    utils_bug_test.go:54: Expected 256 IPs for large range, got 1
    utils_bug_test.go:84: Expected first IP to be 192.169.0.0, got 192.168.0.0
--- FAIL: TestCollectIPsFromRange_LargeRange (0.01s)
FAIL
FAIL	github.com/carverauto/serviceradar/pkg/mapper	0.012s
FAIL

The test clearly shows that instead of returning 256 IPs, the function only returns 1 IP.

Inconsistency within the codebase

Reference code

pkg/mapper/utils.go lines 238-250 (small range path in same function):

} else {
    // Process all IPs in the range
    ipCopy := make(net.IP, len(ip))
    copy(ipCopy, ip)

    for ip := ipCopy.Mask(ipNet.Mask); ipNet.Contains(ip); incrementIP(ip) {
        ipStr := ip.String()

        if !seen[ipStr] {
            targets = append(targets, ipStr)
            seen[ipStr] = true
        }
    }
}

Current code

pkg/mapper/utils.go lines 214-236 (large range path):

if hostBits > defaultHostBitsCheckMin { // More than 256 hosts
    e.logger.Warn().Str("cidr", ipNet.String()).Int("host_bits", hostBits).
        Msg("CIDR range too large, limiting scan")

    // Only scan first 256 IPs
    count := 0
    ipCopy := make(net.IP, len(ip))

    copy(ipCopy, ip)

    for i := ipCopy.Mask(ipNet.Mask); ipNet.Contains(ip) && count < defaultMaxIPRange; incrementIP(ip) {
        // changed from ip to i, to avoid modifying the original IP
        ipStr := i.String()

        if !seen[ipStr] {
            targets = append(targets, ipStr)
            seen[ipStr] = true
            count++
        }
    }
}

Contradiction

The small range path (lines 238-250) correctly shadows the loop variable ip in the for loop declaration and both increments and uses the same variable. The large range path (lines 214-236) attempts to do something similar but makes a critical error: it declares loop variable i but increments ip instead, then uses i in the loop body. This means i never changes, defeating the purpose of the loop. The comment "changed from ip to i, to avoid modifying the original IP" suggests the developer's intent, but the implementation is incorrect because i is never actually incremented.

Full context

The collectIPsFromRange function is called by expandCIDR (pkg/mapper/utils.go:326), which is in turn called by expandSeeds (pkg/mapper/utils.go:291). The expandSeeds function is called at the beginning of runDiscoveryJob (pkg/mapper/discovery.go:1215) to convert user-provided CIDR ranges and individual IPs into a list of target IP addresses for network discovery.

When a network administrator configures ServiceRadar to discover devices on a network, they typically provide CIDR ranges like 10.0.0.0/8, 172.16.0.0/12, or 192.168.0.0/16. For large networks (CIDR masks with > 8 host bits, i.e., more than 256 hosts), the code intentionally limits scanning to the first 256 IPs to avoid overwhelming the system.

However, due to this bug, when a large CIDR range is provided:

The function only returns 1 IP address (the network address) instead of 256 IPs
The discovery job runs but only scans that single IP
The system reports completion but has actually only scanned 0.4% (1/256) of the intended targets for a /24 equivalent range
For a /16 network with 65,536 hosts, this means scanning 0.0015% (1/65,536) of the network

This severely impacts the core functionality of the discovery engine, as network administrators expecting to discover up to 256 devices per large CIDR range will instead only scan a single IP (which is typically the network address itself and unlikely to be a valid host).

The bug affects any discovery job where users provide CIDR ranges larger than /24. Common use cases that would trigger this bug include:

Enterprise networks using private IP ranges: 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16
Data center networks with large subnets
Cloud environments with VPC CIDR blocks
Any network discovery job targeting more than 256 hosts

The small range path (≤ 256 hosts) works correctly, so this bug only manifests for larger networks, which explains why it may have gone undetected if testing focused on smaller subnets.

Why has this bug gone undetected?

Several factors explain why this bug has remained undetected in production:

Git history reveals the bug was introduced during refactoring: The commit 137dc9cb (WIP refactoring to fix agentID and pollerID propagation) introduced this code with the comment "changed from ip to i, to avoid modifying the original IP". This was part of a work-in-progress refactoring, suggesting the code may not have been thoroughly tested before being merged.
No unit tests for CIDR expansion: There are no existing unit tests for collectIPsFromRange, expandCIDR, or expandSeeds functions. The test suite does not validate that large CIDR ranges are properly expanded into multiple IP addresses.
The seen map masks the problem: Because the same IP is attempted to be added 256 times but the seen map prevents duplicates, the function returns a single-element slice rather than crashing or producing obviously broken output. This makes the bug subtle and harder to notice in logs.
Discovery may appear to complete successfully: The discovery job runs to completion and reports success, but with only 1 device scanned instead of up to 256. Without comparing the number of IPs generated against expectations, this looks like a normal completion.
Small networks work correctly: The bug only affects CIDR ranges with > 8 host bits (more than 256 hosts). Smaller ranges like /24, /25, /26, etc. use the correct code path (lines 238-250) and work properly. If testing primarily used smaller subnets, the bug would not manifest.
Real-world deployments may use smaller ranges: Organizations might partition their networks into smaller /24 subnets for discovery, inadvertently avoiding the bug. A network admin might configure multiple /24 ranges rather than a single /16, which would work correctly.
Limited large-scale testing: Testing network discovery on truly large CIDR ranges (like 10.0.0.0/8 or 192.168.0.0/16) requires either a large test network or careful validation of the IP list size. If such testing wasn't performed, the bug would remain hidden.

Recommended fix

Change line 227 from:

for i := ipCopy.Mask(ipNet.Mask); ipNet.Contains(ip) && count < defaultMaxIPRange; incrementIP(ip) {

To:

for ip := ipCopy.Mask(ipNet.Mask); ipNet.Contains(ip) && count < defaultMaxIPRange; incrementIP(ip) {

And change line 229 from:

ipStr := i.String()

To:

ipStr := ip.String()

This makes the large range path consistent with the small range path (lines 242-249), where the loop variable is properly shadowed and both incremented and used within the loop body. The loop variable ip shadows the function parameter ip, which is acceptable in Go and prevents modification of the original parameter, achieving the intent expressed in the comment.

Imported from GitHub. Original GitHub issue: #2146 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/issues/2146 Original created: 2025-12-16T05:16:44Z --- # Summary - **Context**: The `collectIPsFromRange` function in `pkg/mapper/utils.go` is responsible for expanding CIDR ranges into individual IP addresses for network discovery, with a limit of 256 IPs for large ranges (> /24). - **Bug**: In the large range code path (lines 227-236), the loop variable `i` is initialized but never incremented, while the function increments `ip` but uses `i.String()` to get the IP address. - **Actual vs. expected**: Instead of generating 256 unique, incrementing IP addresses from a large CIDR range, the function only returns 1 IP (the network address), because `i` remains constant throughout the loop. - **Impact**: Network discovery on large CIDR ranges (> /24, such as /16 or /8) will only scan a single IP address instead of the intended 256 addresses, causing nearly complete failure of network device discovery for large networks. # Code with bug ```go if hostBits > defaultHostBitsCheckMin { // More than 256 hosts e.logger.Warn().Str("cidr", ipNet.String()).Int("host_bits", hostBits). Msg("CIDR range too large, limiting scan") // Only scan first 256 IPs count := 0 ipCopy := make(net.IP, len(ip)) copy(ipCopy, ip) for i := ipCopy.Mask(ipNet.Mask); ipNet.Contains(ip) && count < defaultMaxIPRange; incrementIP(ip) { // changed from ip to i, to avoid modifying the original IP ipStr := i.String() // <-- BUG 🔴 i is never incremented, always returns the same IP if !seen[ipStr] { targets = append(targets, ipStr) seen[ipStr] = true count++ } } } ``` The bug is on lines 227-229. The loop declares `i := ipCopy.Mask(ipNet.Mask)` in the initialization statement but increments `ip` in the post-statement (`incrementIP(ip)`). The loop body then uses `i.String()` to get the IP address. Since `i` is never modified, it always returns the same IP address (the network address). # Evidence ## Example Consider a user initiating discovery on CIDR range `192.168.0.0/16`: 1. **Initial state**: `ip = 192.168.0.0`, `ipNet.Mask = /16`, `hostBits = 16` (> 8, so enters large range path) 2. **Loop initialization**: `i = ipCopy.Mask(ipNet.Mask) = 192.168.0.0` 3. **First iteration**: - `i.String()` returns `"192.168.0.0"` - Added to `targets` and `seen` map - `count` becomes 1 - `incrementIP(ip)` changes `ip` to `192.168.0.1` (but `i` stays `192.168.0.0`) 4. **Second iteration**: - `i.String()` still returns `"192.168.0.0"` (because `i` was never incremented) - Already in `seen` map, so not added to `targets` - `count` stays 1 - `incrementIP(ip)` changes `ip` to `192.168.0.2` 5. **Iterations 3-256**: Same as iteration 2 - `i` never changes, so same IP is checked repeatedly 6. **Loop terminates**: When `ipNet.Contains(ip)` becomes false or when the loop has run 256 times 7. **Result**: `targets` contains only `["192.168.0.0"]` instead of 256 unique IPs The expected behavior would be to return 256 unique, sequential IP addresses: `192.168.0.0`, `192.168.0.1`, `192.168.0.2`, ..., `192.168.0.255`. ## Failing test ### Test script ```go /* * Copyright 2025 Carver Automation Corporation. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at * * http://www.apache.org/licenses/LICENSE-2.0 * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * See the License for the specific language governing permissions and * limitations under the License. */ package mapper import ( "net" "testing" "time" "github.com/carverauto/serviceradar/pkg/logger" ) // TestCollectIPsFromRange_LargeRange tests that the collectIPsFromRange function // correctly limits large CIDR ranges to 256 IPs and returns unique incrementing IPs. func TestCollectIPsFromRange_LargeRange(t *testing.T) { // Create a minimal DiscoveryEngine for testing engine := &DiscoveryEngine{ config: &Config{ Timeout: 5 * time.Second, Retries: 3, }, logger: logger.NewTestLogger(), } // Test with a /16 network (65536 hosts) - should be limited to 256 IPs cidr := "192.168.0.0/16" ip, ipNet, err := net.ParseCIDR(cidr) if err != nil { t.Fatalf("Failed to parse CIDR: %v", err) } ones, bits := ipNet.Mask.Size() hostBits := bits - ones // This will be 16 for a /16 network seen := make(map[string]bool) targets := engine.collectIPsFromRange(ip, ipNet, hostBits, seen) // Check that we got 256 unique IPs if len(targets) != 256 { t.Errorf("Expected 256 IPs for large range, got %d", len(targets)) } // Check that all IPs are unique uniqueCheck := make(map[string]bool) for _, ip := range targets { if uniqueCheck[ip] { t.Errorf("Found duplicate IP: %s", ip) } uniqueCheck[ip] = true } // Check that IPs are incrementing (not all the same) if len(targets) >= 2 { first := targets[0] allSame := true for _, ip := range targets[1:] { if ip != first { allSame = false break } } if allSame { t.Errorf("All IPs are the same (%s), expected incrementing IPs", first) } } // Check that the first IP is the network address masked expectedFirst := ip.Mask(ipNet.Mask).String() if len(targets) > 0 && targets[0] != expectedFirst { t.Errorf("Expected first IP to be %s, got %s", expectedFirst, targets[0]) } // Check that IPs are sequential if len(targets) >= 10 { // Parse first IP and check that the 10th IP is first + 9 firstIP := net.ParseIP(targets[0]) tenthIP := net.ParseIP(targets[9]) if firstIP == nil || tenthIP == nil { t.Fatal("Failed to parse IPs") } // Create expected 10th IP by incrementing first IP 9 times expectedTenth := make(net.IP, len(firstIP)) copy(expectedTenth, firstIP) for i := 0; i < 9; i++ { incrementIP(expectedTenth) } if tenthIP.String() != expectedTenth.String() { t.Errorf("Expected 10th IP to be %s (first + 9), got %s", expectedTenth.String(), tenthIP.String()) } } } ``` ### Test output ``` === RUN TestCollectIPsFromRange_LargeRange utils_bug_test.go:54: Expected 256 IPs for large range, got 1 utils_bug_test.go:84: Expected first IP to be 192.169.0.0, got 192.168.0.0 --- FAIL: TestCollectIPsFromRange_LargeRange (0.01s) FAIL FAIL github.com/carverauto/serviceradar/pkg/mapper 0.012s FAIL ``` The test clearly shows that instead of returning 256 IPs, the function only returns 1 IP. ## Inconsistency within the codebase ### Reference code `pkg/mapper/utils.go` lines 238-250 (small range path in same function): ```go } else { // Process all IPs in the range ipCopy := make(net.IP, len(ip)) copy(ipCopy, ip) for ip := ipCopy.Mask(ipNet.Mask); ipNet.Contains(ip); incrementIP(ip) { ipStr := ip.String() if !seen[ipStr] { targets = append(targets, ipStr) seen[ipStr] = true } } } ``` ### Current code `pkg/mapper/utils.go` lines 214-236 (large range path): ```go if hostBits > defaultHostBitsCheckMin { // More than 256 hosts e.logger.Warn().Str("cidr", ipNet.String()).Int("host_bits", hostBits). Msg("CIDR range too large, limiting scan") // Only scan first 256 IPs count := 0 ipCopy := make(net.IP, len(ip)) copy(ipCopy, ip) for i := ipCopy.Mask(ipNet.Mask); ipNet.Contains(ip) && count < defaultMaxIPRange; incrementIP(ip) { // changed from ip to i, to avoid modifying the original IP ipStr := i.String() if !seen[ipStr] { targets = append(targets, ipStr) seen[ipStr] = true count++ } } } ``` ### Contradiction The small range path (lines 238-250) correctly shadows the loop variable `ip` in the for loop declaration and both increments and uses the same variable. The large range path (lines 214-236) attempts to do something similar but makes a critical error: it declares loop variable `i` but increments `ip` instead, then uses `i` in the loop body. This means `i` never changes, defeating the purpose of the loop. The comment "changed from ip to i, to avoid modifying the original IP" suggests the developer's intent, but the implementation is incorrect because `i` is never actually incremented. # Full context The `collectIPsFromRange` function is called by `expandCIDR` (`pkg/mapper/utils.go:326`), which is in turn called by `expandSeeds` (`pkg/mapper/utils.go:291`). The `expandSeeds` function is called at the beginning of `runDiscoveryJob` (`pkg/mapper/discovery.go:1215`) to convert user-provided CIDR ranges and individual IPs into a list of target IP addresses for network discovery. When a network administrator configures ServiceRadar to discover devices on a network, they typically provide CIDR ranges like `10.0.0.0/8`, `172.16.0.0/12`, or `192.168.0.0/16`. For large networks (CIDR masks with > 8 host bits, i.e., more than 256 hosts), the code intentionally limits scanning to the first 256 IPs to avoid overwhelming the system. However, due to this bug, when a large CIDR range is provided: 1. The function only returns 1 IP address (the network address) instead of 256 IPs 2. The discovery job runs but only scans that single IP 3. The system reports completion but has actually only scanned 0.4% (1/256) of the intended targets for a /24 equivalent range 4. For a /16 network with 65,536 hosts, this means scanning 0.0015% (1/65,536) of the network This severely impacts the core functionality of the discovery engine, as network administrators expecting to discover up to 256 devices per large CIDR range will instead only scan a single IP (which is typically the network address itself and unlikely to be a valid host). The bug affects any discovery job where users provide CIDR ranges larger than /24. Common use cases that would trigger this bug include: - Enterprise networks using private IP ranges: `10.0.0.0/8`, `172.16.0.0/12`, `192.168.0.0/16` - Data center networks with large subnets - Cloud environments with VPC CIDR blocks - Any network discovery job targeting more than 256 hosts The small range path (≤ 256 hosts) works correctly, so this bug only manifests for larger networks, which explains why it may have gone undetected if testing focused on smaller subnets. # Why has this bug gone undetected? Several factors explain why this bug has remained undetected in production: 1. **Git history reveals the bug was introduced during refactoring**: The commit `137dc9cb` (WIP refactoring to fix agentID and pollerID propagation) introduced this code with the comment "changed from ip to i, to avoid modifying the original IP". This was part of a work-in-progress refactoring, suggesting the code may not have been thoroughly tested before being merged. 2. **No unit tests for CIDR expansion**: There are no existing unit tests for `collectIPsFromRange`, `expandCIDR`, or `expandSeeds` functions. The test suite does not validate that large CIDR ranges are properly expanded into multiple IP addresses. 3. **The `seen` map masks the problem**: Because the same IP is attempted to be added 256 times but the `seen` map prevents duplicates, the function returns a single-element slice rather than crashing or producing obviously broken output. This makes the bug subtle and harder to notice in logs. 4. **Discovery may appear to complete successfully**: The discovery job runs to completion and reports success, but with only 1 device scanned instead of up to 256. Without comparing the number of IPs generated against expectations, this looks like a normal completion. 5. **Small networks work correctly**: The bug only affects CIDR ranges with > 8 host bits (more than 256 hosts). Smaller ranges like `/24`, `/25`, `/26`, etc. use the correct code path (lines 238-250) and work properly. If testing primarily used smaller subnets, the bug would not manifest. 6. **Real-world deployments may use smaller ranges**: Organizations might partition their networks into smaller /24 subnets for discovery, inadvertently avoiding the bug. A network admin might configure multiple /24 ranges rather than a single /16, which would work correctly. 7. **Limited large-scale testing**: Testing network discovery on truly large CIDR ranges (like `10.0.0.0/8` or `192.168.0.0/16`) requires either a large test network or careful validation of the IP list size. If such testing wasn't performed, the bug would remain hidden. # Recommended fix Change line 227 from: ```go for i := ipCopy.Mask(ipNet.Mask); ipNet.Contains(ip) && count < defaultMaxIPRange; incrementIP(ip) { ``` To: ```go for ip := ipCopy.Mask(ipNet.Mask); ipNet.Contains(ip) && count < defaultMaxIPRange; incrementIP(ip) { ``` And change line 229 from: ```go ipStr := i.String() ``` To: ```go ipStr := ip.String() ``` This makes the large range path consistent with the small range path (lines 242-249), where the loop variable is properly shadowed and both incremented and used within the loop body. The loop variable `ip` shadows the function parameter `ip`, which is acceptable in Go and prevents modification of the original parameter, achieving the intent expressed in the comment.