Repository navigation
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces several improvements, including context propagation to blocking I/O calls for immediate cancellation, macOS-specific timeout handling in POSIX ping, enhanced CSV injection sanitization, and new fuzz and POSIX-specific tests. The review feedback highlights three key areas for improvement: refining the CSV sanitization logic to replace newlines with spaces to fully prevent CSV injection, adding IP validation to the Windows osPing implementation for defense-in-depth, and checking for context cancellation in StartScan before executing subsequent blocking network calls.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| func sanitizeCSVField(field string) string { | ||
| if len(field) > 0 { | ||
| fc := field[0] | ||
| if fc == '=' || fc == '+' || fc == '-' || fc == '@' || fc == '\t' || fc == '\r' { | ||
| return "'" + field | ||
| } | ||
| if len(field) == 0 { | ||
| return field | ||
| } | ||
|
|
||
| fc := field[0] | ||
| if fc == '=' || fc == '+' || fc == '-' || fc == '@' || fc == '\t' || fc == '\r' || fc == '\n' { | ||
| return "'" + field | ||
| } | ||
|
|
||
| // Previne injeções que tentam escapar da célula atual usando quebras de linha | ||
| if strings.ContainsAny(field, "\n\r") { | ||
| return "'" + field | ||
| } | ||
|
|
||
| return field | ||
| } |
There was a problem hiding this comment.
Prepending a single quote (') to a field containing newlines (\n or \r) does not fully prevent CSV injection. If a naive CSV parser or spreadsheet importer splits rows on newlines before parsing cells, the second line of the field will be treated as a new row. If that second line starts with a formula trigger (like =), it will still be executed as a formula because the prepended single quote only protects the first line.
Since IP addresses, hostnames, and MAC addresses should never contain newlines, the safest and most robust approach is to replace all carriage returns and newlines with spaces (or strip them entirely) before performing the leading character check.
func sanitizeCSVField(field string) string {
field = strings.ReplaceAll(field, "\n", " ")
field = strings.ReplaceAll(field, "\r", " ")
if len(field) == 0 {
return field
}
fc := field[0]
if fc == '=' || fc == '+' || fc == '-' || fc == '@' || fc == '\t' {
return "'" + field
}
return field
}| func osPing(ctx context.Context, ip string, timeoutMs int) bool { | ||
| if timeoutMs <= 0 { | ||
| timeoutMs = 1000 // safe default | ||
| } |
There was a problem hiding this comment.
For consistency and defense-in-depth, osPing on Windows should validate that the input ip is a valid IP address before executing the external ping command, similar to the validation performed in the POSIX implementation of osPing. This prevents potential command injection vulnerabilities if osPing is ever called with untrusted input in the future.
| func osPing(ctx context.Context, ip string, timeoutMs int) bool { | |
| if timeoutMs <= 0 { | |
| timeoutMs = 1000 // safe default | |
| } | |
| func osPing(ctx context.Context, ip string, timeoutMs int) bool { | |
| if net.ParseIP(ip) == nil { | |
| return false | |
| } | |
| if timeoutMs <= 0 { | |
| timeoutMs = 1000 // safe default | |
| } |
| di.IsAlive = discovery.Ping(ctx, ip, cfg.PingTimeoutMs) | ||
| if di.IsAlive { | ||
| di.Hostname = discovery.ReverseDNS(ip) | ||
| di.MAC = discovery.GetMAC(ip) | ||
| di.OpenPorts = ports.ScanPorts(ip, cfg.DefaultPorts, cfg.PortTimeoutMs) | ||
| di.OpenPorts = ports.ScanPorts(ctx, ip, cfg.DefaultPorts, cfg.PortTimeoutMs) | ||
| } |
There was a problem hiding this comment.
If the context is cancelled during or immediately after the Ping call, we should avoid making subsequent blocking network calls like ReverseDNS (which performs a DNS lookup) and ScanPorts. Checking ctx.Err() == nil before proceeding to these steps ensures that the scan respects cancellation immediately and avoids wasting resources.
| di.IsAlive = discovery.Ping(ctx, ip, cfg.PingTimeoutMs) | |
| if di.IsAlive { | |
| di.Hostname = discovery.ReverseDNS(ip) | |
| di.MAC = discovery.GetMAC(ip) | |
| di.OpenPorts = ports.ScanPorts(ip, cfg.DefaultPorts, cfg.PortTimeoutMs) | |
| di.OpenPorts = ports.ScanPorts(ctx, ip, cfg.DefaultPorts, cfg.PortTimeoutMs) | |
| } | |
| di.IsAlive = discovery.Ping(ctx, ip, cfg.PingTimeoutMs) | |
| if di.IsAlive && ctx.Err() == nil { | |
| di.Hostname = discovery.ReverseDNS(ip) | |
| di.MAC = discovery.GetMAC(ip) | |
| if ctx.Err() == nil { | |
| di.OpenPorts = ports.ScanPorts(ctx, ip, cfg.DefaultPorts, cfg.PortTimeoutMs) | |
| } | |
| } |
Summary
This PR finalizes the 0.1.1 release by improving cross‑platform ping behavior, tightening CSV export sanitization, and making network I/O fully context‑aware to support immediate cancellation of scans. It also adds fuzz testing for target parsing and extends CI to macOS to validate POSIX builds.
Type of change
Security hardening
Bug fix
Refactor
Documentation
Details
Propagate context.Context down to discovery.Ping and ports.ScanPorts, switching to exec.CommandContext and Dialer.DialContext so blocking ping and TCP dials respect cancellation.
Fix POSIX ping timeout semantics by using milliseconds for macOS (runtime.GOOS == "darwin") and seconds for Linux, with a dedicated os_posix_test.go to guard macOS behavior.
Strengthen sanitizeCSVField to block newline‑based CSV injection by guarding against leading control chars and any \n/\r within the field body.
Introduce FuzzParseRange to stress target parsing against malformed IPs, CIDRs, and ranges, ensuring it never panics on adversarial input.
Expand GitHub Actions test matrix to include macos-latest, keeping Go and CI versions aligned with the documented minimums.
Add docs/remediation_202606.md and update CHANGELOG.md with the 0.1.1 remediation summary and release links.
Validation
Built locally
Tests pass locally (including fuzz and new OS‑specific tests)
CI green on Linux and macOS
Docs and changelog updated for 0.1.1