diff --git a/sddl/sddl.go b/sddl/sddl.go index be638e5..723a8a7 100644 --- a/sddl/sddl.go +++ b/sddl/sddl.go @@ -1,24 +1,33 @@ package sddl import ( + "fmt" "strings" ) // CutSDDL parses an SDDL string into its component parts. // +// The scan is parenthesis-aware: the O:, G:, D:, and S: component markers are +// only recognised at the top level (parenthesis depth 0), so a ':' or a marker +// letter appearing inside an ACE body (for example in a conditional or +// resource-attribute ACE) does not split the string. Malformed input — leading +// characters before the first marker, or unbalanced parentheses — is reported +// as an error instead of being silently discarded. +// // Parameters: // - sddlString (string): The SDDL string to parse. // // Returns: -// - (string, string, []string, []string): The owner SID, group SID, DACL ACEs, and SACL ACEs. -func CutSDDL(sddlString string) (string, string, []string, []string) { +// - (string, string, []string, []string, error): The owner SID, group SID, +// DACL ACEs, SACL ACEs, and an error if the string is malformed. +func CutSDDL(sddlString string) (string, string, []string, []string, error) { sddlString = strings.TrimSpace(sddlString) if len(sddlString) == 0 { - return "", "", nil, nil + return "", "", nil, nil, nil } - // Match components starting with O:, G:, D:, or S: using regex + // Match components starting with O:, G:, D:, or S:. components := map[string]string{ "O:": "", "G:": "", @@ -27,41 +36,79 @@ func CutSDDL(sddlString string) (string, string, []string, []string) { } currentComponent := "" + depth := 0 k := 0 for k < len(sddlString) { - upperChar := strings.ToUpper(string(sddlString[k])) - if k+1 < len(sddlString) && (upperChar == "O" || upperChar == "G" || upperChar == "D" || upperChar == "S") && sddlString[k+1] == ':' { - // Normalise the key to uppercase so lowercase markers (o:, g:, d:, s:) - // land in the same bucket as their uppercase counterparts; SDDL is - // case-insensitive at the component-marker level. - currentComponent = upperChar + ":" - k += 2 - continue + c := sddlString[k] + + // A component marker can only start at the top level. Recognising markers + // at depth > 0 would let an ACE body containing "D:" (etc.) be mistaken + // for a new component. + if depth == 0 && k+1 < len(sddlString) && sddlString[k+1] == ':' { + upperChar := strings.ToUpper(string(c)) + if upperChar == "O" || upperChar == "G" || upperChar == "D" || upperChar == "S" { + // Normalise the key to uppercase so lowercase markers (o:, g:, d:, + // s:) land in the same bucket as their uppercase counterparts; SDDL + // is case-insensitive at the component-marker level. + currentComponent = upperChar + ":" + k += 2 + continue + } + } + + switch c { + case '(': + depth++ + case ')': + depth-- + if depth < 0 { + return "", "", nil, nil, fmt.Errorf("malformed SDDL: unbalanced ')' at position %d", k) + } } - if currentComponent != "" { - components[currentComponent] += string(sddlString[k]) + + if currentComponent == "" { + // Any character before the first component marker is invalid; the + // string has already been trimmed, so this is genuine garbage. + return "", "", nil, nil, fmt.Errorf("malformed SDDL: unexpected character %q at position %d before any component marker", c, k) } + components[currentComponent] += string(c) k++ } - daclAces := CutAces(components["D:"]) - saclAces := CutAces(components["S:"]) + if depth != 0 { + return "", "", nil, nil, fmt.Errorf("malformed SDDL: unbalanced '(' (missing %d closing parenthesis)", depth) + } + + daclAces, err := CutAces(components["D:"]) + if err != nil { + return "", "", nil, nil, fmt.Errorf("invalid DACL: %w", err) + } + saclAces, err := CutAces(components["S:"]) + if err != nil { + return "", "", nil, nil, fmt.Errorf("invalid SACL: %w", err) + } - return components["O:"], components["G:"], daclAces, saclAces + return components["O:"], components["G:"], daclAces, saclAces, nil } // CutAces extracts individual ACE strings from a DACL/SACL component. // Handles the format: flags(ace1)(ace2)...(aceN) -func CutAces(aclStr string) []string { +// +// Nested parentheses inside an ACE (for example in a conditional expression) +// are preserved: only top-level parentheses delimit ACEs. Unbalanced +// parentheses and stray characters between or after ACEs are reported as an +// error rather than silently dropping or truncating ACEs. +func CutAces(aclStr string) ([]string, error) { var aces []string - // Find first ( to separate flags + // Find first ( to separate flags. No '(' means the component has no ACEs + // (e.g. an empty DACL with only flags), which is valid. start := strings.Index(aclStr, "(") if start == -1 { - return aces + return aces, nil } - // Extract ACEs between parentheses + // Extract ACEs between top-level parentheses. depth := 0 aceStart := start for i := start; i < len(aclStr); i++ { @@ -73,11 +120,22 @@ func CutAces(aclStr string) []string { depth++ case ')': depth-- + if depth < 0 { + return nil, fmt.Errorf("unbalanced ')' at position %d", i) + } if depth == 0 { aces = append(aces, aclStr[aceStart:i]) } + default: + if depth == 0 { + return nil, fmt.Errorf("unexpected character %q at position %d (outside any ACE)", aclStr[i], i) + } } } - return aces + if depth != 0 { + return nil, fmt.Errorf("unbalanced '(' (missing %d closing parenthesis)", depth) + } + + return aces, nil } diff --git a/sddl/sddl_test.go b/sddl/sddl_test.go index e55946d..8101f91 100644 --- a/sddl/sddl_test.go +++ b/sddl/sddl_test.go @@ -62,7 +62,10 @@ func TestSddlCut(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - gotOwner, gotGroup, gotDaclAces, gotSaclAces := sddl.CutSDDL(tt.input) + gotOwner, gotGroup, gotDaclAces, gotSaclAces, err := sddl.CutSDDL(tt.input) + if err != nil { + t.Fatalf("CutSDDL() unexpected error = %v", err) + } if gotOwner != tt.wantOwner { t.Errorf("CutSDDL() owner = %v, want %v", gotOwner, tt.wantOwner) @@ -135,7 +138,10 @@ func TestSddlCut_LowercaseMarkers(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - gotOwner, gotGroup, gotDaclAces, gotSaclAces := sddl.CutSDDL(tt.input) + gotOwner, gotGroup, gotDaclAces, gotSaclAces, err := sddl.CutSDDL(tt.input) + if err != nil { + t.Fatalf("CutSDDL() unexpected error = %v", err) + } if gotOwner != tt.wantOwner { t.Errorf("CutSDDL() owner = %q, want %q", gotOwner, tt.wantOwner) @@ -153,6 +159,52 @@ func TestSddlCut_LowercaseMarkers(t *testing.T) { } } +// TestSddlCut_ConditionalAceSingleToken verifies that an ACE containing nested +// parentheses and a ':' (as in a conditional expression) is tokenized as a +// single ACE, and that a marker-like substring inside the ACE body does not +// start a new component. +func TestSddlCut_ConditionalAceSingleToken(t *testing.T) { + // The inner "(@xD:1)" carries both nested parens and a "D:" substring. + input := "D:(XA;;FA;;;WD;(@xD:1))" + + gotOwner, gotGroup, gotDaclAces, gotSaclAces, err := sddl.CutSDDL(input) + if err != nil { + t.Fatalf("CutSDDL() unexpected error = %v", err) + } + if gotOwner != "" || gotGroup != "" { + t.Errorf("CutSDDL() owner/group = %q/%q, want empty", gotOwner, gotGroup) + } + if len(gotSaclAces) != 0 { + t.Errorf("CutSDDL() saclAces = %v, want none (inner \"D:\" must not start a component)", gotSaclAces) + } + want := []string{"XA;;FA;;;WD;(@xD:1)"} + if !slices.Equal(gotDaclAces, want) { + t.Errorf("CutSDDL() daclAces = %v, want %v", gotDaclAces, want) + } +} + +// TestSddlCut_MalformedReturnsError verifies that malformed SDDL is rejected +// with an error instead of being silently truncated. +func TestSddlCut_MalformedReturnsError(t *testing.T) { + tests := []struct { + name string + input string + }{ + {name: "unbalanced open paren in DACL", input: "D:(A;;GA;;;WD"}, + {name: "extra close paren", input: "D:(A;;GA;;;WD))"}, + {name: "leading garbage before marker", input: "garbageO:BA"}, + {name: "trailing garbage after last ACE", input: "D:(A;;GA;;;WD)junk"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if _, _, _, _, err := sddl.CutSDDL(tt.input); err == nil { + t.Errorf("CutSDDL(%q) = nil error, want error", tt.input) + } + }) + } +} + // func TestSDDLToBinary(t *testing.T) { // tests := []struct { // name string