fix: security hardening — sanitize config input, atomic writes, error leak, rate limit, dead code

server.go

@@ -11,6 +11,7 @@ import (
 	"regexp"
 	"strconv"
 	"strings"
+	"sync"
 	"time"
 )
 
@@ -107,9 +108,21 @@ func parsePinnedHosts() (map[string]string, error) {
 	return pinned, scanner.Err()
 }
 
+// sanitize strips control characters and newlines from a config value.
+func sanitize(s string) string {
+	s = strings.ReplaceAll(s, "\n", "")
+	s = strings.ReplaceAll(s, "\r", "")
+	return strings.TrimSpace(strings.Map(func(r rune) rune {
+		if r < 32 && r != '\t' {
+			return -1
+		}
+		return r
+	}, s))
+}
+
 // pinLease appends a dhcp-host line to dnsmasq.conf.
 func pinLease(mac, ip, hostname string) error {
-	line := fmt.Sprintf("dhcp-host=%s,%s,%s", mac, ip, hostname)
+	line := fmt.Sprintf("dhcp-host=%s,%s,%s", sanitize(mac), sanitize(ip), sanitize(hostname))
 	f, err := os.OpenFile(confFile, os.O_APPEND|os.O_WRONLY, 0)
 	if err != nil {
 		return fmt.Errorf("open config for append: %w", err)
@@ -156,19 +169,30 @@ func unpinLease(mac string) error {
 		return nil // nothing to remove, not an error
 	}
 
-	// Write back the filtered content.
-	out, err := os.Create(confFile)
+	// Write back atomically to avoid TOCTOU race.
+	tmpFile := confFile + ".tmp"
+	out, err := os.Create(tmpFile)
 	if err != nil {
-		return fmt.Errorf("create config file: %w", err)
+		return fmt.Errorf("create temp file: %w", err)
 	}
+	writeErr := func() error {
 		defer out.Close()
-
 		for _, line := range kept {
 			if _, err := fmt.Fprintln(out, line); err != nil {
 				return fmt.Errorf("write config line: %w", err)
 			}
 		}
 		return nil
+	}()
+	if writeErr != nil {
+		os.Remove(tmpFile)
+		return writeErr
+	}
+	if err := os.Rename(tmpFile, confFile); err != nil {
+		os.Remove(tmpFile)
+		return fmt.Errorf("rename temp file: %w", err)
+	}
+	return nil
 }
 
 // reloadDnsmasq runs the reload command (e.g. sudo systemctl reload dnsmasq).
@@ -194,12 +218,10 @@ func getAllLeases() ([]LeaseEntry, error) {
 	}
 
 	entries := make([]LeaseEntry, 0, len(activeLeases))
-	pinnedCount := 0
 	for _, lease := range activeLeases {
 		status := PinStatusActive
 		if _, ok := pinned[lease.MAC]; ok {
 			status = PinStatusPermanent
-			pinnedCount++
 		}
 		entries = append(entries, LeaseEntry{
 			Hostname: lease.Hostname,
@@ -216,6 +238,12 @@ func getAllLeases() ([]LeaseEntry, error) {
 // HTTP handlers
 // ---------------------------------------------------------------------------
 
+var (
+	lastReload  time.Time
+	lastReloadMu sync.Mutex
+	reloadCooldown = 5 * time.Second
+)
+
 func writeJSON(w http.ResponseWriter, status int, v interface{}) {
 	w.Header().Set("Content-Type", "application/json")
 	w.WriteHeader(status)
@@ -226,7 +254,7 @@ func handleListLeases(w http.ResponseWriter, r *http.Request) {
 	entries, err := getAllLeases()
 	if err != nil {
 		log.Printf("ERROR listing leases: %v", err)
-		writeJSON(w, http.StatusInternalServerError, ErrorResponse{Error: err.Error()})
+		writeJSON(w, http.StatusInternalServerError, ErrorResponse{Error: "failed to read lease data"})
 		return
 	}
 
@@ -264,7 +292,8 @@ func handlePinLease(w http.ResponseWriter, r *http.Request) {
 	// Check if already pinned.
 	pinned, err := parsePinnedHosts()
 	if err != nil {
-		writeJSON(w, http.StatusInternalServerError, ErrorResponse{Error: err.Error()})
+		log.Printf("ERROR parsing pinned hosts: %v", err)
+		writeJSON(w, http.StatusInternalServerError, ErrorResponse{Error: "failed to read config"})
 		return
 	}
 	if _, exists := pinned[strings.ToLower(req.MAC)]; exists {
@@ -276,7 +305,8 @@ func handlePinLease(w http.ResponseWriter, r *http.Request) {
 	}
 
 	if err := pinLease(req.MAC, req.IP, req.Hostname); err != nil {
-		writeJSON(w, http.StatusInternalServerError, ErrorResponse{Error: err.Error()})
+		log.Printf("ERROR pinning lease: %v", err)
+		writeJSON(w, http.StatusInternalServerError, ErrorResponse{Error: "failed to write config"})
 		return
 	}
 	writeJSON(w, http.StatusOK, PinResponse{Success: true, Message: "pinned successfully"})
@@ -299,20 +329,32 @@ func handleUnpinLease(w http.ResponseWriter, r *http.Request) {
 	}
 
 	if err := unpinLease(req.MAC); err != nil {
-		writeJSON(w, http.StatusInternalServerError, ErrorResponse{Error: err.Error()})
+		log.Printf("ERROR unpinning lease: %v", err)
+		writeJSON(w, http.StatusInternalServerError, ErrorResponse{Error: "failed to update config"})
 		return
 	}
 	writeJSON(w, http.StatusOK, PinResponse{Success: true, Message: "unpinned successfully"})
 }
 
 func handleReload(w http.ResponseWriter, r *http.Request) {
+	lastReloadMu.Lock()
+	elapsed := time.Since(lastReload)
+	if elapsed < reloadCooldown {
+		lastReloadMu.Unlock()
+		writeJSON(w, http.StatusTooManyRequests, PinResponse{
+			Success: false,
+			Message: fmt.Sprintf("rate limited — wait %.0fs", reloadCooldown.Seconds()-elapsed.Seconds()),
+		})
+		return
+	}
+	lastReload = time.Now()
+	lastReloadMu.Unlock()
+
 	if err := reloadDnsmasq(); err != nil {
 		log.Printf("ERROR reloading dnsmasq: %v", err)
-		// Return 200 even on error — Zoraxy may intercept non-200
-		// responses and replace them with HTML error pages.
 		writeJSON(w, http.StatusOK, PinResponse{
 			Success: false,
-			Message: fmt.Sprintf("reload failed: %v", err),
+			Message: "reload failed — check server logs",
 		})
 		return
 	}