Skip to content

Commit a917d02

Browse files
kochj23claude
andcommitted
security: Fix command injection, TLS trust, CORS, force unwraps, add API auth
- CRITICAL: IP validation on /api/scan/start (regex + octet range check) - CRITICAL: Removed wildcard CORS from NovaAPIServer - HIGH: TLS auto-trust replaced with TOFU certificate pinning - Added bearer token auth on all POST endpoints - Fixed force unwraps in HistoricalTracker and NetworkTrafficAnalyzer Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent f6e7c10 commit a917d02

4 files changed

Lines changed: 91 additions & 27 deletions

File tree

NMAPScanner/HistoricalTracker.swift

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -394,10 +394,25 @@ class HistoricalTracker: ObservableObject {
394394
let allPorts = Set(deviceSnapshots.flatMap { $0.openPorts })
395395
let deviceChanges = changes.filter { $0.ipAddress == ipAddress }
396396

397+
guard let firstSnapshot = deviceSnapshots.first,
398+
let lastSnapshot = deviceSnapshots.last else {
399+
return DeviceStatistics(
400+
ipAddress: ipAddress,
401+
firstSeen: Date(),
402+
lastSeen: Date(),
403+
totalScans: 0,
404+
onlineScans: 0,
405+
offlineScans: 0,
406+
uptimePercentage: 0,
407+
uniquePortsSeen: 0,
408+
totalChanges: 0
409+
)
410+
}
411+
397412
return DeviceStatistics(
398413
ipAddress: ipAddress,
399-
firstSeen: deviceSnapshots.first!.timestamp,
400-
lastSeen: deviceSnapshots.last!.timestamp,
414+
firstSeen: firstSnapshot.timestamp,
415+
lastSeen: lastSnapshot.timestamp,
401416
totalScans: deviceSnapshots.count,
402417
onlineScans: onlineCount,
403418
offlineScans: offlineCount,

NMAPScanner/NetworkTrafficAnalyzer.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -144,8 +144,8 @@ class NetworkTrafficAnalyzer: ObservableObject {
144144
trafficHistory[ip, default: []].append(dataPoint)
145145

146146
// Limit history size
147-
if trafficHistory[ip]!.count > maxHistoryPoints {
148-
trafficHistory[ip]!.removeFirst()
147+
if let count = trafficHistory[ip]?.count, count > maxHistoryPoints {
148+
trafficHistory[ip]?.removeFirst()
149149
}
150150

151151
stats.history = trafficHistory[ip] ?? []

NMAPScanner/NovaAPIServer.swift

Lines changed: 43 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,18 @@ class NovaAPIServer {
2525
let port: UInt16 = 37423
2626
private var listener: NWListener?
2727
private let startTime = Date()
28+
29+
/// Local-only anti-CSRF bearer token (not a secret — just prevents drive-by POST from browser JS)
30+
private let apiToken: String = {
31+
let key = "NovaAPIToken"
32+
if let existing = UserDefaults.standard.string(forKey: key), !existing.isEmpty {
33+
return existing
34+
}
35+
let token = UUID().uuidString
36+
UserDefaults.standard.set(token, forKey: key)
37+
return token
38+
}()
39+
2840
private init() {}
2941

3042
func start() {
@@ -56,6 +68,13 @@ class NovaAPIServer {
5668
private func route(_ req: NovaRequest) async -> String {
5769
if req.method == "OPTIONS" { return http(200, "") }
5870

71+
// Require bearer token for all POST requests (anti-CSRF)
72+
if req.method == "POST" {
73+
guard let auth = req.headers["authorization"], auth == "Bearer \(apiToken)" else {
74+
return json(401, ["error": "Unauthorized — missing or invalid Bearer token"] as [String: Any])
75+
}
76+
}
77+
5978
switch (req.method, req.path) {
6079

6180
case ("GET", "/api/status"):
@@ -90,12 +109,28 @@ class NovaAPIServer {
90109
guard let body = req.bodyJSON(), let ip = body["ip"] as? String else {
91110
return json(400, ["error": "'ip' required"] as [String: Any])
92111
}
93-
// Scan via nmap subprocess
112+
// SECURITY: Validate IP/CIDR format to prevent command injection.
113+
// Only allow IPv4 addresses with optional CIDR notation (e.g. 192.168.1.0/24).
114+
let ipRegex = /^\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}(\/\d{1,2})?$/
115+
guard ip.wholeMatch(of: ipRegex) != nil else {
116+
return json(400, ["error": "Invalid IP address format. Expected: x.x.x.x or x.x.x.x/n"] as [String: Any])
117+
}
118+
// Additionally validate each octet is 0-255
119+
let octets = ip.components(separatedBy: "/").first!.components(separatedBy: ".")
120+
let validOctets = octets.allSatisfy { if let n = Int($0) { return n >= 0 && n <= 255 } else { return false } }
121+
guard validOctets else {
122+
return json(400, ["error": "Invalid IP address: octets must be 0-255"] as [String: Any])
123+
}
124+
if let cidrPart = ip.components(separatedBy: "/").last, ip.contains("/"),
125+
let cidr = Int(cidrPart), (cidr < 0 || cidr > 32) {
126+
return json(400, ["error": "Invalid CIDR prefix: must be 0-32"] as [String: Any])
127+
}
128+
// Scan via nmap subprocess — arguments passed as array, never through shell
94129
Task {
95-
let result = Process()
96-
result.executableURL = URL(fileURLWithPath: "/usr/bin/env")
97-
result.arguments = ["nmap", "-sV", "--open", ip]
98-
try? result.run()
130+
let nmapProcess = Process()
131+
nmapProcess.executableURL = URL(fileURLWithPath: "/usr/local/bin/nmap")
132+
nmapProcess.arguments = ["-sV", "--open", ip]
133+
try? nmapProcess.run()
99134
}
100135
return json(200, ["status": "scan_started", "ip": ip] as [String: Any])
101136

@@ -208,7 +243,7 @@ class NovaAPIServer {
208243
}
209244

210245
private struct NovaRequest {
211-
let method: String; let path: String; let body: String
246+
let method: String; let path: String; let body: String; let headers: [String: String]
212247
func bodyJSON() -> [String: Any]? { guard let d = body.data(using: .utf8) else { return nil }; return try? JSONSerialization.jsonObject(with: d) as? [String: Any] }
213248
init?(_ data: Data) {
214249
guard let raw = String(data: data, encoding: .utf8), raw.contains("\r\n\r\n") else { return nil }
@@ -217,7 +252,7 @@ class NovaAPIServer {
217252
var hdrs: [String: String] = [:]; for l in lines.dropFirst() { let kv = l.components(separatedBy: ": "); if kv.count >= 2 { hdrs[kv[0].lowercased()] = kv.dropFirst().joined(separator: ": ") } }
218253
let rawBody = parts.dropFirst().joined(separator: "\r\n\r\n")
219254
if let cl = hdrs["content-length"], let n = Int(cl), rawBody.utf8.count < n { return nil }
220-
method = tokens[0]; path = tokens[1].components(separatedBy: "?").first ?? tokens[1]; body = rawBody
255+
method = tokens[0]; path = tokens[1].components(separatedBy: "?").first ?? tokens[1]; body = rawBody; headers = hdrs
221256
}
222257
}
223258
// Map severity to STIX 2.1 indicator type
@@ -231,5 +266,5 @@ class NovaAPIServer {
231266

232267
private func json(_ s: Int, _ d: [String: Any]) -> String { guard let data = try? JSONSerialization.data(withJSONObject: d, options: .prettyPrinted), let body = String(data: data, encoding: .utf8) else { return http(500, "") }; return http(s, body, "application/json") }
233268
private func jsonArray(_ s: Int, _ a: [[String: Any]]) -> String { guard let data = try? JSONSerialization.data(withJSONObject: a, options: .prettyPrinted), let body = String(data: data, encoding: .utf8) else { return http(500, "") }; return http(s, body, "application/json") }
234-
private func http(_ s: Int, _ body: String, _ ct: String = "text/plain") -> String { let st = [200:"OK",201:"Created",400:"Bad Request",404:"Not Found",500:"Internal Server Error"][s] ?? "Unknown"; return "HTTP/1.1 \(s) \(st)\r\nContent-Type: \(ct); charset=utf-8\r\nContent-Length: \(body.utf8.count)\r\nAccess-Control-Allow-Origin: *\r\nConnection: close\r\n\r\n\(body)" }
269+
private func http(_ s: Int, _ body: String, _ ct: String = "text/plain") -> String { let st = [200:"OK",201:"Created",400:"Bad Request",401:"Unauthorized",404:"Not Found",500:"Internal Server Error"][s] ?? "Unknown"; return "HTTP/1.1 \(s) \(st)\r\nContent-Type: \(ct); charset=utf-8\r\nContent-Length: \(body.utf8.count)\r\nConnection: close\r\n\r\n\(body)" }
235270
}

NMAPScanner/UniFiController.swift

Lines changed: 29 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -568,23 +568,37 @@ class UniFiController: ObservableObject {
568568
}
569569
}
570570

571-
// MARK: - Certificate Trust Management
571+
// MARK: - Certificate Trust Management (TOFU — Trust On First Use)
572572

573-
/// Prompt user to trust certificate
574-
private func promptUserToTrustCertificate(host: String, commonName: String, fingerprint: String) async -> Bool {
575-
// This will be called from the secure delegate
576-
// In a real implementation, you'd show a SwiftUI alert
577-
// For now, return true to maintain existing behavior but with tracking
578-
579-
SecureLogger.log("Certificate trust prompt for \(host): CN=\(commonName), FP=\(fingerprint)", level: .warning)
573+
/// UserDefaults key prefix for stored certificate fingerprints
574+
private static let certFingerprintKeyPrefix = "NMAPScanner_TrustedCertFingerprint_"
580575

581-
// SECURITY: Auto-trusting certificates bypasses TLS verification.
582-
// A proper implementation should present a SwiftUI confirmation dialog showing
583-
// the certificate common name and fingerprint, allowing the user to accept or reject.
584-
// Until that UI is built, all UniFi controller certificates are auto-trusted.
585-
SecurityAuditLog.log(event: .certificateTrusted, details: "Auto-trusted certificate for \(host) (pending UI implementation)", level: .security)
586-
587-
return true // Auto-trust for now (better than blind trust)
576+
/// Prompt user to trust certificate using Trust-On-First-Use (TOFU).
577+
/// On first connection to a host, the certificate fingerprint is saved.
578+
/// On subsequent connections, the fingerprint is compared — if it changes, trust is DENIED
579+
/// and a warning is logged (possible MITM attack).
580+
private func promptUserToTrustCertificate(host: String, commonName: String, fingerprint: String) async -> Bool {
581+
let key = Self.certFingerprintKeyPrefix + host
582+
583+
if let savedFingerprint = UserDefaults.standard.string(forKey: key) {
584+
// We have a previously trusted fingerprint for this host
585+
if savedFingerprint == fingerprint {
586+
SecureLogger.log("Certificate fingerprint matches stored value for \(host)", level: .info)
587+
SecurityAuditLog.log(event: .certificateTrusted, details: "TOFU: Certificate fingerprint verified for \(host)", level: .info)
588+
return true
589+
} else {
590+
// SECURITY: Fingerprint changed — possible MITM attack. Reject and warn.
591+
SecureLogger.log("CERTIFICATE FINGERPRINT CHANGED for \(host)! Expected: \(savedFingerprint), Got: \(fingerprint). Possible MITM attack. Connection REJECTED.", level: .error)
592+
SecurityAuditLog.log(event: .certificateTrusted, details: "TOFU VIOLATION: Certificate fingerprint changed for \(host). Old=\(savedFingerprint) New=\(fingerprint). Connection rejected.", level: .critical)
593+
return false
594+
}
595+
} else {
596+
// First connection to this host — trust and save fingerprint (TOFU)
597+
UserDefaults.standard.set(fingerprint, forKey: key)
598+
SecureLogger.log("TOFU: First connection to \(host). Saving certificate fingerprint: \(fingerprint), CN=\(commonName)", level: .warning)
599+
SecurityAuditLog.log(event: .certificateTrusted, details: "TOFU: First-use trust for \(host), CN=\(commonName), FP=\(fingerprint)", level: .security)
600+
return true
601+
}
588602
}
589603
}
590604

0 commit comments

Comments
 (0)