Merge pull request #1232 from projectdiscovery/fix-redirect-response-bug

fix #1173: perform matching on all redirect responses instead of final

integration_tests/http/get-redirects-chain-headers.yaml

@@ -0,0 +1,23 @@
+id: basic-get-redirects-chain-headers
+
+info:
+  name: Basic GET Redirects Request With Chain header
+  author: pdteam
+  severity: info
+
+requests:
+  - method: GET
+    path:
+      - "{{BaseURL}}"
+    redirects: true
+    max-redirects: 3
+    matchers-condition: and
+    matchers:
+      - type: word
+        part: header
+        words:
+          - "TestRedirectHeaderMatch"
+
+      - type: status
+        status:
+          - 302

v2/cmd/integration-test/http.go

@@ -35,6 +35,7 @@ var httpTestcases = map[string]testutils.TestCase{
 	"http/self-contained.yaml":                     &httpRequestSelContained{},
 	"http/get-case-insensitive.yaml":               &httpGetCaseInsensitive{},
 	"http/get.yaml,http/get-case-insensitive.yaml": &httpGetCaseInsensitiveCluster{},
+	"http/get-redirects-chain-headers.yaml":        &httpGetRedirectsChainHeaders{},
 }
 
 type httpInteractshRequest struct{}
@@ -599,3 +600,31 @@ func (h *httpGetCaseInsensitiveCluster) Execute(filesPath string) error {
 	}
 	return nil
 }
+
+type httpGetRedirectsChainHeaders struct{}
+
+// Execute executes a test case and returns an error if occurred
+func (h *httpGetRedirectsChainHeaders) Execute(filePath string) error {
+	router := httprouter.New()
+	router.GET("/", func(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
+		http.Redirect(w, r, "/redirected", http.StatusFound)
+	})
+	router.GET("/redirected", func(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
+		w.Header().Set("Secret", "TestRedirectHeaderMatch")
+		http.Redirect(w, r, "/final", http.StatusFound)
+	})
+	router.GET("/final", func(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
+		_, _ = w.Write([]byte("ok"))
+	})
+	ts := httptest.NewServer(router)
+	defer ts.Close()
+
+	results, err := testutils.RunNucleiTemplateAndGetResults(filePath, ts.URL, debug)
+	if err != nil {
+		return err
+	}
+	if len(results) != 1 {
+		return errIncorrectResultsCount(results)
+	}
+	return nil
+}

v2/cmd/integration-test/integration-test.go

@@ -51,5 +51,5 @@ func main() {
 }
 
 func errIncorrectResultsCount(results []string) error {
-	return fmt.Errorf("incorrect number of results %s", strings.Join(results, "\n\t"))
+	return fmt.Errorf("incorrect number of results \n\t%s", strings.Join(results, "\n\t"))
 }

v2/pkg/protocols/http/request.go

@@ -400,7 +400,7 @@ func (request *Request) executeRequest(reqURL string, generatedRequest *generate
 		return errors.Wrap(err, "could not dump http response")
 	}
 
-	var data, redirectedResponse []byte
+	var dumpedResponse []redirectedResponse
 	// If the status code is HTTP 101, we should not proceed with reading body.
 	if resp.StatusCode != http.StatusSwitchingProtocols {
 		var bodyReader io.Reader
@@ -409,7 +409,7 @@ func (request *Request) executeRequest(reqURL string, generatedRequest *generate
 		} else {
 			bodyReader = resp.Body
 		}
-		data, err = ioutil.ReadAll(bodyReader)
+		data, err := ioutil.ReadAll(bodyReader)
 		if err != nil {
 			// Ignore body read due to server misconfiguration errors
 			if stringsutil.ContainsAny(err.Error(), "gzip: invalid header") {
@@ -420,96 +420,61 @@ func (request *Request) executeRequest(reqURL string, generatedRequest *generate
 		}
 		resp.Body.Close()
 
-		redirectedResponse, err = dumpResponseWithRedirectChain(resp, data)
+		dumpedResponse, err = dumpResponseWithRedirectChain(resp, data)
 		if err != nil {
 			return errors.Wrap(err, "could not read http response with redirect chain")
 		}
 	} else {
-		redirectedResponse = dumpedResponseHeaders
+		dumpedResponse = []redirectedResponse{{fullResponse: dumpedResponseHeaders, headers: dumpedResponseHeaders}}
 	}
 
-	// net/http doesn't automatically decompress the response body if an
-	// encoding has been specified by the user in the request so in case we have to
-	// manually do it.
-	dataOrig := data
-	data, err = handleDecompression(resp, data)
-	// in case of error use original data
-	if err != nil {
-		data = dataOrig
-	}
-
-	// Dump response - step 2 - replace gzip body with deflated one or with itself (NOP operation)
-	dumpedResponseBuilder := &bytes.Buffer{}
-	dumpedResponseBuilder.Write(dumpedResponseHeaders)
-	dumpedResponseBuilder.Write(data)
-	dumpedResponse := dumpedResponseBuilder.Bytes()
-	redirectedResponse = bytes.ReplaceAll(redirectedResponse, dataOrig, data)
-
-	// Decode gbk response content-types
-	// gb18030 supersedes gb2312
-	responseContentType := resp.Header.Get("Content-Type")
-	if isContentTypeGbk(responseContentType) {
-		dumpedResponse, err = decodegbk(dumpedResponse)
-		if err != nil {
-			return errors.Wrap(err, "could not gbk decode")
-		}
-		redirectedResponse, err = decodegbk(redirectedResponse)
-		if err != nil {
-			return errors.Wrap(err, "could not gbk decode")
+	for _, response := range dumpedResponse {
+		// if nuclei-project is enabled store the response if not previously done
+		if request.options.ProjectFile != nil && !fromCache {
+			if err := request.options.ProjectFile.Set(dumpedRequest, resp, response.body); err != nil {
+				return errors.Wrap(err, "could not store in project file")
+			}
 		}
 
-		// the uncompressed body needs to be decoded to standard utf8
-		data, err = decodegbk(data)
-		if err != nil {
-			return errors.Wrap(err, "could not gbk decode")
+		matchedURL := reqURL
+		if generatedRequest.rawRequest != nil && generatedRequest.rawRequest.FullURL != "" {
+			matchedURL = generatedRequest.rawRequest.FullURL
 		}
-	}
-
-	// if nuclei-project is enabled store the response if not previously done
-	if request.options.ProjectFile != nil && !fromCache {
-		if err := request.options.ProjectFile.Set(dumpedRequest, resp, data); err != nil {
-			return errors.Wrap(err, "could not store in project file")
+		if generatedRequest.request != nil {
+			matchedURL = generatedRequest.request.URL.String()
 		}
-	}
+		finalEvent := make(output.InternalEvent)
 
-	matchedURL := reqURL
-	if generatedRequest.rawRequest != nil && generatedRequest.rawRequest.FullURL != "" {
-		matchedURL = generatedRequest.rawRequest.FullURL
-	}
-	if generatedRequest.request != nil {
-		matchedURL = generatedRequest.request.URL.String()
-	}
-	finalEvent := make(output.InternalEvent)
-
-	outputEvent := request.responseToDSLMap(resp, reqURL, matchedURL, tostring.UnsafeToString(dumpedRequest), tostring.UnsafeToString(dumpedResponse), tostring.UnsafeToString(data), headersToString(resp.Header), duration, generatedRequest.meta)
-	if i := strings.LastIndex(hostname, ":"); i != -1 {
-		hostname = hostname[:i]
-	}
-	outputEvent["curl-command"] = curlCommand
-	outputEvent["ip"] = httpclientpool.Dialer.GetDialedIP(hostname)
-	outputEvent["redirect-chain"] = tostring.UnsafeToString(redirectedResponse)
-	for k, v := range previousEvent {
-		finalEvent[k] = v
-	}
-	for k, v := range outputEvent {
-		finalEvent[k] = v
-	}
-	// Add to history the current request number metadata if asked by the user.
-	if request.ReqCondition {
+		outputEvent := request.responseToDSLMap(response.resp, reqURL, matchedURL, tostring.UnsafeToString(dumpedRequest), tostring.UnsafeToString(response.fullResponse), tostring.UnsafeToString(response.body), tostring.UnsafeToString(response.headers), duration, generatedRequest.meta)
+		if i := strings.LastIndex(hostname, ":"); i != -1 {
+			hostname = hostname[:i]
+		}
+		outputEvent["curl-command"] = curlCommand
+		outputEvent["ip"] = httpclientpool.Dialer.GetDialedIP(hostname)
+		for k, v := range previousEvent {
+			finalEvent[k] = v
+		}
 		for k, v := range outputEvent {
-			key := fmt.Sprintf("%s_%d", k, requestCount)
-			previousEvent[key] = v
-			finalEvent[key] = v
+			finalEvent[k] = v
 		}
+		// Add to history the current request number metadata if asked by the user.
+		if request.ReqCondition {
+			for k, v := range outputEvent {
+				key := fmt.Sprintf("%s_%d", k, requestCount)
+				previousEvent[key] = v
+				finalEvent[key] = v
+			}
+		}
+
+		event := eventcreator.CreateEventWithAdditionalOptions(request, finalEvent, request.options.Options.Debug || request.options.Options.DebugResponse, func(internalWrappedEvent *output.InternalWrappedEvent) {
+			internalWrappedEvent.OperatorsResult.PayloadValues = generatedRequest.meta
+		})
+
+		responseContentType := resp.Header.Get("Content-Type")
+		dumpResponse(event, request.options, response.fullResponse, formedURL, responseContentType)
+
+		callback(event)
 	}
-
-	event := eventcreator.CreateEventWithAdditionalOptions(request, finalEvent, request.options.Options.Debug || request.options.Options.DebugResponse, func(internalWrappedEvent *output.InternalWrappedEvent) {
-		internalWrappedEvent.OperatorsResult.PayloadValues = generatedRequest.meta
-	})
-
-	dumpResponse(event, request.options, redirectedResponse, formedURL, responseContentType)
-
-	callback(event)
 	return nil
 }
 

v2/pkg/protocols/http/utils.go

@@ -10,14 +10,21 @@ import (
 	"net/http/httputil"
 	"strings"
 
+	"github.com/pkg/errors"
 	"github.com/projectdiscovery/nuclei/v2/pkg/protocols/common/generators"
-	"github.com/projectdiscovery/nuclei/v2/pkg/protocols/common/tostring"
 	"github.com/projectdiscovery/rawhttp"
 	"github.com/projectdiscovery/stringsutil"
 	"golang.org/x/text/encoding/simplifiedchinese"
 	"golang.org/x/text/transform"
 )
 
+type redirectedResponse struct {
+	headers      []byte
+	body         []byte
+	fullResponse []byte
+	resp         *http.Response
+}
+
 // dumpResponseWithRedirectChain dumps a http response with the
 // complete http redirect chain.
 //
@@ -25,18 +32,23 @@ import (
 // and returns the data to the user for matching and viewing in that order.
 //
 // Inspired from - https://github.com/ffuf/ffuf/issues/324#issuecomment-719858923
-func dumpResponseWithRedirectChain(resp *http.Response, body []byte) ([]byte, error) {
-	redirects := []string{}
+func dumpResponseWithRedirectChain(resp *http.Response, body []byte) ([]redirectedResponse, error) {
+	var response []redirectedResponse
+
 	respData, err := httputil.DumpResponse(resp, false)
 	if err != nil {
 		return nil, err
 	}
-	redirectChain := &bytes.Buffer{}
-
-	redirectChain.WriteString(tostring.UnsafeToString(respData))
-	redirectChain.Write(body)
-	redirects = append(redirects, redirectChain.String())
-	redirectChain.Reset()
+	respObj := redirectedResponse{
+		headers:      respData,
+		body:         body,
+		resp:         resp,
+		fullResponse: bytes.Join([][]byte{respData, body}, []byte{}),
+	}
+	if err := normalizeResponseBody(resp, &respObj); err != nil {
+		return nil, err
+	}
+	response = append(response, respObj)
 
 	var redirectResp *http.Response
 	if resp != nil && resp.Request != nil {
@@ -52,40 +64,51 @@ func dumpResponseWithRedirectChain(resp *http.Response, body []byte) ([]byte, er
 		if redirectResp.Body != nil {
 			body, _ = ioutil.ReadAll(redirectResp.Body)
 		}
-		redirectChain.WriteString(tostring.UnsafeToString(respData))
-		if len(body) > 0 {
-			redirectChain.WriteString(tostring.UnsafeToString(body))
+		respObj := redirectedResponse{
+			headers:      respData,
+			body:         body,
+			resp:         redirectResp,
+			fullResponse: bytes.Join([][]byte{respData, body}, []byte{}),
 		}
-		redirects = append(redirects, redirectChain.String())
+		if err := normalizeResponseBody(redirectResp, &respObj); err != nil {
+			return nil, err
+		}
+		response = append(response, respObj)
 		redirectResp = redirectResp.Request.Response
-		redirectChain.Reset()
 	}
-	for i := len(redirects) - 1; i >= 0; i-- {
-		redirectChain.WriteString(redirects[i])
-	}
-	return redirectChain.Bytes(), nil
+	return response, nil
 }
 
-// headersToString converts http headers to string
-func headersToString(headers http.Header) string {
-	builder := &strings.Builder{}
-
-	for header, values := range headers {
-		builder.WriteString(header)
-		builder.WriteString(": ")
-
-		for i, value := range values {
-			builder.WriteString(value)
-
-			if i != len(values)-1 {
-				builder.WriteRune('\n')
-				builder.WriteString(header)
-				builder.WriteString(": ")
-			}
-		}
-		builder.WriteRune('\n')
+// normalizeResponseBody performs normalization on the http response object.
+func normalizeResponseBody(resp *http.Response, response *redirectedResponse) error {
+	var err error
+	// net/http doesn't automatically decompress the response body if an
+	// encoding has been specified by the user in the request so in case we have to
+	// manually do it.
+	dataOrig := response.body
+	response.body, err = handleDecompression(resp, response.body)
+	// in case of error use original data
+	if err != nil {
+		response.body = dataOrig
 	}
-	return builder.String()
+	response.fullResponse = bytes.ReplaceAll(response.fullResponse, dataOrig, response.body)
+
+	// Decode gbk response content-types
+	// gb18030 supersedes gb2312
+	responseContentType := resp.Header.Get("Content-Type")
+	if isContentTypeGbk(responseContentType) {
+		response.fullResponse, err = decodegbk(response.fullResponse)
+		if err != nil {
+			return errors.Wrap(err, "could not gbk decode")
+		}
+
+		// the uncompressed body needs to be decoded to standard utf8
+		response.body, err = decodegbk(response.body)
+		if err != nil {
+			return errors.Wrap(err, "could not gbk decode")
+		}
+	}
+	return nil
 }
 
 // dump creates a dump of the http request in form of a byte slice