fix multiple panics & missing matcher-status in flow templates (#4978)

* validate and fix empty internal-event

* fix on error with interactsh req

* disable clustering in flow & multiproto

* fix empty/missing matcher-status result

* fix cluster unit test

* fix no results found unit test

pkg/protocols/http/request.go

@@ -472,18 +472,16 @@ func (request *Request) ExecuteWithResults(input *contextargs.Context, dynamicVa
 const drainReqSize = int64(8 * 1024)
 
 // executeRequest executes the actual generated request and returns error if occurred
-func (request *Request) executeRequest(input *contextargs.Context, generatedRequest *generatedRequest, previousEvent output.InternalEvent, hasInteractMatchers bool, callback protocols.OutputEventCallback, requestCount int) (err error) {
+func (request *Request) executeRequest(input *contextargs.Context, generatedRequest *generatedRequest, previousEvent output.InternalEvent, hasInteractMatchers bool, processEvent protocols.OutputEventCallback, requestCount int) (err error) {
-	var event *output.InternalWrappedEvent
+
-	defer func() {
+	// wrap one more callback for validation and fixing event
-		if event != nil {
+	callback := func(event *output.InternalWrappedEvent) {
-			if event.InternalEvent == nil {
+		// validateNFixEvent performs necessary validation on generated event
-				event.InternalEvent = make(map[string]interface{})
+		// and attempts to fix it , this includes things like making sure
-				event.InternalEvent["template-id"] = request.options.TemplateID
+		// `template-id` is set , `request-url-pattern` is set etc
+		request.validateNFixEvent(input, generatedRequest, err, event)
+		processEvent(event)
 	}
-			// add the request URL pattern to the event
-			event.InternalEvent[ReqURLPatternKey] = generatedRequest.requestURLPattern
-		}
-	}()
 
 	request.setCustomHeaders(generatedRequest)
 
@@ -692,7 +690,7 @@ func (request *Request) executeRequest(input *contextargs.Context, generatedRequ
 		if len(generatedRequest.interactshURLs) > 0 {
 			// according to logic we only need to trigger a callback if interactsh was used
 			// and request failed in hope that later on oast interaction will be received
-			event = &output.InternalWrappedEvent{}
+			event := &output.InternalWrappedEvent{}
 			if request.CompiledOperators != nil && request.CompiledOperators.HasDSL() {
 				event.InternalEvent = outputEvent
 			}
@@ -807,7 +805,7 @@ func (request *Request) executeRequest(input *contextargs.Context, generatedRequ
 		// prune signature internal values if any
 		request.pruneSignatureInternalValues(generatedRequest.meta)
 
-		event = eventcreator.CreateEventWithAdditionalOptions(request, generators.MergeMaps(generatedRequest.dynamicValues, finalEvent), request.options.Options.Debug || request.options.Options.DebugResponse, func(internalWrappedEvent *output.InternalWrappedEvent) {
+		event := eventcreator.CreateEventWithAdditionalOptions(request, generators.MergeMaps(generatedRequest.dynamicValues, finalEvent), request.options.Options.Debug || request.options.Options.DebugResponse, func(internalWrappedEvent *output.InternalWrappedEvent) {
 			internalWrappedEvent.OperatorsResult.PayloadValues = generatedRequest.meta
 		})
 		if hasInteractMatchers {
@@ -843,6 +841,37 @@ func (request *Request) executeRequest(input *contextargs.Context, generatedRequ
 	return errx
 }
 
+// validateNFixEvent validates and fixes the event
+// it adds any missing template-id and request-url-pattern
+func (request *Request) validateNFixEvent(input *contextargs.Context, gr *generatedRequest, err error, event *output.InternalWrappedEvent) {
+	if event != nil {
+		if event.InternalEvent == nil {
+			event.InternalEvent = make(map[string]interface{})
+			event.InternalEvent["template-id"] = request.options.TemplateID
+		}
+		// add the request URL pattern to the event
+		event.InternalEvent[ReqURLPatternKey] = gr.requestURLPattern
+		if event.InternalEvent["host"] == nil {
+			event.InternalEvent["host"] = input.MetaInput.Input
+		}
+		if event.InternalEvent["template-id"] == nil {
+			event.InternalEvent["template-id"] = request.options.TemplateID
+		}
+		if event.InternalEvent["type"] == nil {
+			event.InternalEvent["type"] = request.Type().String()
+		}
+		if event.InternalEvent["template-path"] == nil {
+			event.InternalEvent["template-path"] = request.options.TemplatePath
+		}
+		if event.InternalEvent["template-info"] == nil {
+			event.InternalEvent["template-info"] = request.options.TemplateInfo
+		}
+		if err != nil {
+			event.InternalEvent["error"] = err.Error()
+		}
+	}
+}
+
 // handleSignature of the http request
 func (request *Request) handleSignature(generatedRequest *generatedRequest) error {
 	switch request.Signature.Value {

pkg/templates/cluster.go

@@ -59,6 +59,14 @@ func Cluster(list []*Template) [][]*Template {
 			final = append(final, []*Template{template})
 			continue
 		}
+
+		// it is not possible to cluster flow and multiprotocol due to dependent execution
+		if template.Flow != "" || template.Options.IsMultiProtocol {
+			_ = skip.Set(key, struct{}{})
+			final = append(final, []*Template{template})
+			continue
+		}
+
 		_ = skip.Set(key, struct{}{})
 
 		var templateType types.ProtocolType
@@ -81,6 +89,13 @@ func Cluster(list []*Template) [][]*Template {
 				continue
 			}
 
+			// it is not possible to cluster flow and multiprotocol due to dependent execution
+			if other.Flow != "" || other.Options.IsMultiProtocol {
+				_ = skip.Set(otherKey, struct{}{})
+				final = append(final, []*Template{other})
+				continue
+			}
+
 			switch templateType {
 			case types.DNSProtocol:
 				if len(other.RequestsDNS) != 1 {

pkg/templates/cluster_test.go

@@ -3,15 +3,25 @@ package templates
 import (
 	"testing"
 
+	"github.com/projectdiscovery/nuclei/v3/pkg/model"
+	"github.com/projectdiscovery/nuclei/v3/pkg/model/types/severity"
 	"github.com/projectdiscovery/nuclei/v3/pkg/protocols/dns"
 	"github.com/projectdiscovery/nuclei/v3/pkg/protocols/http"
+	"github.com/projectdiscovery/nuclei/v3/pkg/testutils"
 	"github.com/stretchr/testify/require"
 )
 
 func TestClusterTemplates(t *testing.T) {
+	// state of whether template is flow or multiprotocol is stored in executerOptions i.e why we need to pass it
+	execOptions := testutils.NewMockExecuterOptions(testutils.DefaultOptions, &testutils.TemplateInfo{
+		ID:   "templateID",
+		Info: model.Info{SeverityHolder: severity.Holder{Severity: severity.Low}, Name: "test"},
+	})
 	t.Run("http-cluster-get", func(t *testing.T) {
 		tp1 := &Template{Path: "first.yaml", RequestsHTTP: []*http.Request{{Path: []string{"{{BaseURL}}"}}}}
 		tp2 := &Template{Path: "second.yaml", RequestsHTTP: []*http.Request{{Path: []string{"{{BaseURL}}"}}}}
+		tp1.Options = execOptions
+		tp2.Options = execOptions
 		tpls := []*Template{tp1, tp2}
 		// cluster 0
 		expected := []*Template{tp1, tp2}
@@ -21,6 +31,8 @@ func TestClusterTemplates(t *testing.T) {
 	t.Run("no-http-cluster", func(t *testing.T) {
 		tp1 := &Template{Path: "first.yaml", RequestsHTTP: []*http.Request{{Path: []string{"{{BaseURL}}/random"}}}}
 		tp2 := &Template{Path: "second.yaml", RequestsHTTP: []*http.Request{{Path: []string{"{{BaseURL}}/another"}}}}
+		tp1.Options = execOptions
+		tp2.Options = execOptions
 		tpls := []*Template{tp1, tp2}
 		expected := [][]*Template{{tp1}, {tp2}}
 		got := Cluster(tpls)
@@ -29,6 +41,8 @@ func TestClusterTemplates(t *testing.T) {
 	t.Run("dns-cluster", func(t *testing.T) {
 		tp1 := &Template{Path: "first.yaml", RequestsDNS: []*dns.Request{{Name: "{{Hostname}}"}}}
 		tp2 := &Template{Path: "second.yaml", RequestsDNS: []*dns.Request{{Name: "{{Hostname}}"}}}
+		tp1.Options = execOptions
+		tp2.Options = execOptions
 		tpls := []*Template{tp1, tp2}
 		// cluster 0
 		expected := []*Template{tp1, tp2}

pkg/tmplexec/exec.go

@@ -92,7 +92,11 @@ func (e *TemplateExecuter) Requests() int {
 
 // Execute executes the protocol group and returns true or false if results were found.
 func (e *TemplateExecuter) Execute(ctx *scan.ScanContext) (bool, error) {
-	results := &atomic.Bool{}
+	// executed contains status of execution if it was successfully executed or not
+	// doesn't matter if it was matched or not
+	executed := &atomic.Bool{}
+	// matched in this case means something was exported / written to output
+	matched := &atomic.Bool{}
 	defer func() {
 		// it is essential to remove template context of `Scan i.e template x input pair`
 		// since it is of no use after scan is completed (regardless of success or failure)
@@ -101,11 +105,11 @@ func (e *TemplateExecuter) Execute(ctx *scan.ScanContext) (bool, error) {
 
 	var lastMatcherEvent *output.InternalWrappedEvent
 	writeFailureCallback := func(event *output.InternalWrappedEvent, matcherStatus bool) {
-		if !results.Load() && matcherStatus {
+		if !matched.Load() && matcherStatus {
 			if err := e.options.Output.WriteFailure(event); err != nil {
 				gologger.Warning().Msgf("Could not write failure event to output: %s\n", err)
 			}
-			results.CompareAndSwap(false, true)
+			executed.CompareAndSwap(false, true)
 		}
 	}
 
@@ -133,11 +137,11 @@ func (e *TemplateExecuter) Execute(ctx *scan.ScanContext) (bool, error) {
 		// If no results were found, and also interactsh is not being used
 		// in that case we can skip it, otherwise we've to show failure in
 		// case of matcher-status flag.
-		if !event.HasOperatorResult() && !event.UsesInteractsh {
+		if !event.HasOperatorResult() && event.InternalEvent != nil {
 			lastMatcherEvent = event
 		} else {
 			if writer.WriteResult(event, e.options.Output, e.options.Progress, e.options.IssuesClient) {
-				results.CompareAndSwap(false, true)
+				matched.Store(true)
 			} else {
 				lastMatcherEvent = event
 			}
@@ -152,7 +156,7 @@ func (e *TemplateExecuter) Execute(ctx *scan.ScanContext) (bool, error) {
 	// so in compile step earlier we compile it to validate javascript syntax and other things
 	// and while executing we create new instance of flow executor everytime
 	if e.options.Flow != "" {
-		flowexec, err := flow.NewFlowExecutor(e.requests, ctx, e.options, results, e.program)
+		flowexec, err := flow.NewFlowExecutor(e.requests, ctx, e.options, executed, e.program)
 		if err != nil {
 			ctx.LogError(err)
 			return false, fmt.Errorf("could not create flow executor: %s", err)
@@ -169,7 +173,7 @@ func (e *TemplateExecuter) Execute(ctx *scan.ScanContext) (bool, error) {
 	if lastMatcherEvent != nil {
 		writeFailureCallback(lastMatcherEvent, e.options.Options.MatcherStatus)
 	}
-	return results.Load(), errx
+	return executed.Load() || matched.Load(), errx
 }
 
 // ExecuteWithResults executes the protocol requests and returns results instead of writing them.