Skip to content

Fix OS command injection in DiagnosticPing (RCH-RULE-877450ee, RCH-RULE-a755ac83, RCH-RULE-7059753c)#22

Draft
al1dazzi with Copilot wants to merge 2 commits into
mainfrom
copilot/rch-task-4c8587e6002e128b-remediate-batch
Draft

Fix OS command injection in DiagnosticPing (RCH-RULE-877450ee, RCH-RULE-a755ac83, RCH-RULE-7059753c)#22
al1dazzi with Copilot wants to merge 2 commits into
mainfrom
copilot/rch-task-4c8587e6002e128b-remediate-batch

Conversation

Copilot AI commented Jun 22, 2026

Copy link
Copy Markdown

DiagnosticPing passed unsanitized user input directly into a shell string, enabling arbitrary command execution. SafeDiagnosticPing was correctly structured but leaked raw command output in error responses.

Changes

  • DiagnosticPing: Added safety.AllowedHostname validation; replaced shell invocation with fixed-argv exec.Command("ping", "-c", "1", host)
  • Both handlers: Command failures now log internally via log.Printf; client receives only a generic "ping failed" message instead of raw CombinedOutput()
// Before
exec.Command("sh", "-c", "ping -c 1 "+host).CombinedOutput()  // shell injection

// After
if !safety.AllowedHostname(host) {
    http.Error(w, "invalid host", http.StatusBadRequest)
    return
}
out, err := exec.Command("ping", "-c", "1", host).CombinedOutput()
if err != nil {
    log.Printf("DiagnosticPing: ping failed for host %q: %v: %s", host, err, out)
    http.Error(w, "ping failed", http.StatusBadGateway)
    return
}

Remediation status

Rule File Status
RCH-RULE-877450ee internal/handlers/cwe.go fixed
RCH-RULE-7059753c internal/handlers/cwe.go fixed
RCH-RULE-a755ac83 internal/handlers/cwe.go fixed

…H-RULE-877450ee, RCH-RULE-a755ac83, RCH-RULE-7059753c)
Copilot AI changed the title [WIP] Remediate selected batch of 3 units Fix OS command injection in DiagnosticPing (RCH-RULE-877450ee, RCH-RULE-a755ac83, RCH-RULE-7059753c) Jun 22, 2026
Copilot AI requested a review from al1dazzi June 22, 2026 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants