address review

internal/seclang/directives.go

@@ -354,6 +354,9 @@ func directiveSecRuleRemoveByID(options *DirectiveOptions) error {
 
 			options.WAF.Rules.DeleteByID(id)
 		} else {
+			if idx == 0 {
+				return fmt.Errorf("SecRuleUpdateTargetById: invalid negative id: %s", idOrRange)
+			}
 			start, err := strconv.Atoi(idOrRange[:idx])
 			if err != nil {
 				return err
@@ -975,19 +978,11 @@ func directiveSecRuleUpdateTargetByID(options *DirectiveOptions) error {
 			if err != nil {
 				return err
 			}
-
-			rule := options.WAF.Rules.FindByID(id)
-			if rule == nil {
-				return fmt.Errorf("SecRuleUpdateTargetById: rule \"%d\" not found", id)
-			}
-			rp := RuleParser{
-				rule:           rule,
-				options:        RuleOptions{},
-				defaultActions: map[types.RulePhase][]ruleAction{},
-			}
-			return rp.ParseVariables(strings.Trim(variables, "\""))
-
+			return updateTargetBySingleID(id, variables, options)
 		} else {
+			if idx == 0 {
+				return fmt.Errorf("SecRuleUpdateTargetById: invalid negative id: %s", idOrRange)
+			}
 			start, err := strconv.Atoi(idOrRange[:idx])
 			if err != nil {
 				return err
@@ -997,7 +992,9 @@ func directiveSecRuleUpdateTargetByID(options *DirectiveOptions) error {
 			if err != nil {
 				return err
 			}
-
+			if start == end {
+				return updateTargetBySingleID(start, variables, options)
+			}
 			if start > end {
 				return fmt.Errorf("invalid range: %s", idOrRange)
 			}
@@ -1019,6 +1016,20 @@ func directiveSecRuleUpdateTargetByID(options *DirectiveOptions) error {
 	return nil
 }
 
+func updateTargetBySingleID(id int, variables string, options *DirectiveOptions) error {
+
+	rule := options.WAF.Rules.FindByID(id)
+	if rule == nil {
+		return fmt.Errorf("SecRuleUpdateTargetById: rule \"%d\" not found", id)
+	}
+	rp := RuleParser{
+		rule:           rule,
+		options:        RuleOptions{},
+		defaultActions: map[types.RulePhase][]ruleAction{},
+	}
+	return rp.ParseVariables(strings.Trim(variables, "\""))
+}
+
 // Description: Updates the target (variable) list of the specified rule(s) by tag.
 // Syntax: SecRuleUpdateTargetByTag TAG TARGET1[|TARGET2|TARGET3]
 // ---
@@ -1035,13 +1046,15 @@ func directiveSecRuleUpdateTargetByTag(options *DirectiveOptions) error {
 	}
 
 	for _, rule := range options.WAF.Rules.GetRules() {
-		if utils.InSlice(strings.Trim(tagAndvars[0], "\""), rule.Tags_) {
+		inputTag := strings.Trim(tagAndvars[0], "\"")
+		if utils.InSlice(inputTag, rule.Tags_) {
 			rp := RuleParser{
 				rule:           &rule,
 				options:        RuleOptions{},
 				defaultActions: map[types.RulePhase][]ruleAction{},
 			}
-			if err := rp.ParseVariables(strings.Trim(tagAndvars[1], "\"")); err != nil {
+			inputVars := strings.Trim(tagAndvars[1], "\"")
+			if err := rp.ParseVariables(inputVars); err != nil {
 				return err
 			}
 		}

internal/seclang/directives_test.go

@@ -170,6 +170,9 @@ func TestDirectives(t *testing.T) {
 			{"1-a", expectErrorOnDirective},
 			{"a-2", expectErrorOnDirective},
 			{"2-1", expectErrorOnDirective},
+			{"-1", expectErrorOnDirective},
+			{"-5--1", expectErrorOnDirective},
+			{"5--1", expectErrorOnDirective},
 			{"1", expectNoErrorOnDirective},
 			{"1 2", expectNoErrorOnDirective},
 			{"1 2 3-4", expectNoErrorOnDirective},
@@ -183,9 +186,13 @@ func TestDirectives(t *testing.T) {
 			{"1-a \"ARGS:wp_post\"", expectErrorOnDirective},
 			{"a-2 \"ARGS:wp_post\"", expectErrorOnDirective},
 			{"2-1 \"ARGS:wp_post\"", expectErrorOnDirective},
+			{"-1 \"ARGS:wp_post\"", expectErrorOnDirective},
+			{"-5--1 \"ARGS:wp_post\"", expectErrorOnDirective},
+			{"5--1 \"ARGS:wp_post\"", expectErrorOnDirective},
 			// Variables has also to be provided to the directive
 			{"1", expectErrorOnDirective},
 			{"1 \"ARGS:wp_post\"", expectNoErrorOnDirective},
+			{"7-7 \"ARGS:wp_post\"", expectNoErrorOnDirective},
 			{"1 2 \"ARGS:wp_post\"", expectNoErrorOnDirective},
 			{"1 2 3-4 \"ARGS:wp_post\"", expectNoErrorOnDirective},
 			{"1 \"REQUEST_BODY|ARGS:wp_post\"", expectNoErrorOnDirective},