diff --git a/internal/api/alerts.go b/internal/api/alerts.go index 00608f6..ea1704c 100644 --- a/internal/api/alerts.go +++ b/internal/api/alerts.go @@ -36,18 +36,24 @@ func (h *AlertHandlers) SetMonitor(m *monitoring.Monitor) { // validateAlertID validates an alert ID for security func validateAlertID(alertID string) bool { - // Check length to prevent DOS attacks with huge IDs + // Guard against empty strings or extremely large payloads that could impact memory usage. if len(alertID) == 0 || len(alertID) > 500 { return false } - // Alert IDs should only contain alphanumeric, hyphens, underscores, colons, and slashes - // (e.g., "pve1:qemu/101-cpu", "node-offline-pve1") - for _, c := range alertID { - if !((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || - (c >= '0' && c <= '9') || c == '-' || c == '_' || c == ':' || c == '/' || c == '.') { + + // Reject attempts to traverse directories via crafted path segments. + if strings.Contains(alertID, "../") || strings.Contains(alertID, "/..") { + return false + } + + // Permit any printable ASCII character (including spaces) so user supplied identifiers + // like instance names remain valid, while excluding control characters and DEL. + for _, r := range alertID { + if r < 32 || r == 127 { return false } } + return true } diff --git a/internal/api/alerts_test.go b/internal/api/alerts_test.go new file mode 100644 index 0000000..2fe09a3 --- /dev/null +++ b/internal/api/alerts_test.go @@ -0,0 +1,37 @@ +package api + +import "testing" + +func TestValidateAlertID(t *testing.T) { + testCases := []struct { + name string + id string + valid bool + }{ + {name: "basic", id: "guest-powered-off-pve-101", valid: true}, + {name: "with spaces", id: "cluster one-node-101-cpu", valid: true}, + {name: "with slash and colon", id: "pve1:qemu/101-cpu", valid: true}, + {name: "empty", id: "", valid: false}, + {name: "too long", id: string(make([]byte, 501)), valid: false}, + {name: "control char", id: "bad\nvalue", valid: false}, + {name: "path traversal", id: "../etc/passwd", valid: false}, + {name: "path traversal middle", id: "pve/../secret", valid: false}, + } + + // Populate the oversized id string once to avoid zero bytes being mistaken for a valid character set. + for i := range testCases { + if testCases[i].name == "too long" { + value := make([]byte, 501) + for j := range value { + value[j] = 'a' + } + testCases[i].id = string(value) + } + } + + for _, tc := range testCases { + if got := validateAlertID(tc.id); got != tc.valid { + t.Errorf("validateAlertID(%s) = %v, want %v", tc.name, got, tc.valid) + } + } +}