From 3719dceb712164276e3164ddc544d6f9d0d7b14e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Felipe=20Zipitr=C3=ADa?=
 <3012076+fzipi@users.noreply.github.com>
Date: Wed, 30 Oct 2024 21:39:38 -0300
Subject: [PATCH] fix: redirect action status codes (#1183)

* fix: redirect action status codes

Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>

* docs: reword redirect description

Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>

---------

Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>
---
 internal/actions/redirect.go         | 13 ++++--
 testing/engine/disruptive_actions.go | 70 ++++++++++++++++++++++++++++
 2 files changed, 79 insertions(+), 4 deletions(-)

diff --git a/internal/actions/redirect.go b/internal/actions/redirect.go
index 5ba2740b8..12cd11dd5 100644
--- a/internal/actions/redirect.go
+++ b/internal/actions/redirect.go
@@ -12,9 +12,9 @@ import (
 //
 // Description:
 // Intercepts transaction by issuing an external (client-visible) redirection to the given location.
-// If the status action is presented on the same rule,
-// and its value can be used for a redirection (i.e., one of the following: 301, 302, 303, or 307),
-// the value will be used for the redirection status code. Otherwise, status code 302 will be used.
+// If the status action is presented on the same rule,  and its value can be used for a redirection
+// (supported redirection codes: 301, 302, 303, 307) the value will be used for the redirection status code.
+// Otherwise, status code 302 will be used.
 //
 // Example:
 // ```
@@ -34,12 +34,17 @@ func (a *redirectFn) Init(_ plugintypes.RuleMetadata, data string) error {
 }
 
 func (a *redirectFn) Evaluate(r plugintypes.RuleMetadata, tx plugintypes.TransactionState) {
+	status := 302 // default status code for redirection
 	rid := r.ID()
 	if rid == noID {
 		rid = r.ParentID()
 	}
+	rstatus := r.Status()
+	if rstatus == 301 || rstatus == 302 || rstatus == 303 || rstatus == 307 {
+		status = rstatus
+	}
 	tx.Interrupt(&types.Interruption{
-		Status: r.Status(),
+		Status: status,
 		RuleID: rid,
 		Action: "redirect",
 		Data:   a.target,
diff --git a/testing/engine/disruptive_actions.go b/testing/engine/disruptive_actions.go
index 1efb066d2..7173aee89 100644
--- a/testing/engine/disruptive_actions.go
+++ b/testing/engine/disruptive_actions.go
@@ -84,6 +84,71 @@ var _ = profile.RegisterProfile(profile.Profile{
 						},
 					},
 				},
+				// Phase 2
+				{
+					Stage: profile.SubStage{
+						Input: profile.StageInput{
+							URI: "/redirect2",
+						},
+						Output: profile.ExpectedOutput{
+							TriggeredRules: []int{21},
+							Interruption: &profile.ExpectedInterruption{
+								Status: 302,
+								Data:   "https://www.example.com",
+								RuleID: 21,
+								Action: "redirect",
+							},
+						},
+					},
+				},
+				{
+					Stage: profile.SubStage{
+						Input: profile.StageInput{
+							URI: "/redirect6",
+						},
+						Output: profile.ExpectedOutput{
+							TriggeredRules: []int{61},
+							Interruption: &profile.ExpectedInterruption{
+								Status: 302,
+								Data:   "https://www.example.com",
+								RuleID: 61,
+								Action: "redirect",
+							},
+						},
+					},
+				},
+				{
+					Stage: profile.SubStage{
+						Input: profile.StageInput{
+							URI: "/redirect7",
+						},
+						Output: profile.ExpectedOutput{
+							TriggeredRules: []int{62},
+							Interruption: &profile.ExpectedInterruption{
+								Status: 307,
+								Data:   "https://www.example.com",
+								RuleID: 62,
+								Action: "redirect",
+							},
+						},
+					},
+				},
+				{
+					Stage: profile.SubStage{
+						Input: profile.StageInput{
+							URI: "/redirect8",
+						},
+						Output: profile.ExpectedOutput{
+							TriggeredRules: []int{63},
+							Interruption: &profile.ExpectedInterruption{
+								Status: 302,
+								Data:   "https://www.example.com",
+								RuleID: 63,
+								Action: "redirect",
+							},
+						},
+					},
+				},
 				{
 					Stage: profile.SubStage{
 						Input: profile.StageInput{
@@ -305,6 +370,11 @@ SecRule REQUEST_URI "/redirect5$" "phase:5,id:51,log,status:302,redirect:https:/
 SecRule REQUEST_URI "/deny5$" "phase:5,id:52,log,status:500,deny"
 SecRule REQUEST_URI "/drop5$" "phase:5,id:53,log,drop"
 
+SecRule REQUEST_URI "/redirect6$" "phase:2,id:61,log,redirect:https://www.example.com"
+SecRule REQUEST_URI "/redirect7$" "phase:2,id:62,log,status:307,redirect:https://www.example.com"
+SecRule REQUEST_URI "/redirect8$" "phase:2,id:63,log,status:401,redirect:https://www.example.com"
+
+
 # Rule 103 is missing the phase, therefore phase:2 is implicitly applied with its related default actions
 # So we will expect a deny with 501 response for the blocking action.
 SecDefaultAction "phase:2,deny,status:501,log"