Incorrect result output from conditional workflows

[tovask]

I have some complex scenarios with workflows:

• a child template is executed even if its parent template's condition does not match, if another non-direct parent matches before its own one.
• in other times, a match of a top level template (that does not have a child template) does not show up in the output if another top level template's child does not match.

(This may sounds complicated, so better to check out the example below 😉⬇️)

Nuclei version:

3.1.0 - 3.2.2

Steps To Reproduce:

I created this simplified workflow to reproduce the issue:

id: debug_workflow
info:
  name: Debug workflow
  author: Levente Kovats

workflows:
      - template: always_match1.yaml
        subtemplates:
          - template: never_match.yaml
            subtemplates:
              - template: always_match3.yaml
      - template: always_match2.yaml

always_match1.yaml:

id: always_match1
info:
  name: Test
  author: Levente Kovats
  severity: info
  description: Test template for investigations
  reference:
   - https://docs.projectdiscovery.io/templates/workflows/overview

http:
  - method: GET
    path:
      - "{{BaseURL}}"
    matchers:
      - type: dsl
        name: always_true
        dsl:
          - true

Similarly, there're always_match2.yaml, always_match3.yaml and a never_match.yaml template (this last one has an always_false matcher of course).

Current Behavior:

If always_match2 finishes before never_match, then always_match3 is executed, despite never_match did not match:

./nuclei -disable-update-check -target https://httpbin.org/user-agent -workflows debug_workflow.yaml -v

                     __     _
   ____  __  _______/ /__  (_)
  / __ \/ / / / ___/ / _ \/ /
 / / / / /_/ / /__/ /  __/ /
/_/ /_/\__,_/\___/_/\___/_/   v3.2.2

                projectdiscovery.io

[VER] Started metrics server at localhost:9092
[INF] Current nuclei version: v3.2.2 (outdated)
[INF] Current nuclei-templates version:  (latest)
[WRN] Scan results upload to cloud is disabled.
[INF] Workflows loaded for current scan: 1
[INF] Targets loaded for current scan: 1
[VER] [always_match1] Sent HTTP request to https://httpbin.org/user-agent
[VER] [always_match2] Sent HTTP request to https://httpbin.org/user-agent
[VER] [never_match] Sent HTTP request to https://httpbin.org/user-agent
[VER] [always_match3] Sent HTTP request to https://httpbin.org/user-agent
[always_match3:always_true] [http] [info] https://httpbin.org/user-agent

Or when the always_match2 is slowler than never_match, then even the always_match2 does not show up:

 ./nuclei -disable-update-check -target https://httpbin.org/user-agent -workflows debug_workflow.yaml -v

                     __     _
   ____  __  _______/ /__  (_)
  / __ \/ / / / ___/ / _ \/ /
 / / / / /_/ / /__/ /  __/ /
/_/ /_/\__,_/\___/_/\___/_/   v3.2.2

                projectdiscovery.io

[VER] Started metrics server at localhost:9092
[INF] Current nuclei version: v3.2.2 (outdated)
[INF] Current nuclei-templates version:  (latest)
[WRN] Scan results upload to cloud is disabled.
[INF] Workflows loaded for current scan: 1
[INF] Targets loaded for current scan: 1
[VER] [always_match1] Sent HTTP request to https://httpbin.org/user-agent
[VER] [never_match] Sent HTTP request to https://httpbin.org/user-agent
[VER] [always_match2] Sent HTTP request to https://httpbin.org/user-agent
[INF] No results found. Better luck next time!

Expected Behavior:

Until version 3.0.4 it works as expected:

./nuclei -disable-update-check -target https://httpbin.org/user-agent -workflows debug_workflow.yaml -v

                     __     _
   ____  __  _______/ /__  (_)
  / __ \/ / / / ___/ / _ \/ /
 / / / / /_/ / /__/ /  __/ /
/_/ /_/\__,_/\___/_/\___/_/   v3.0.4

                projectdiscovery.io

[VER] Started metrics server at localhost:9092
[INF] Current nuclei version: v3.0.4 (outdated)
[INF] Current nuclei-templates version:  (latest)
[INF] Workflows loaded for current scan: 1
[INF] Targets loaded for current scan: 1
[VER] [always_match1] Sent HTTP request to https://httpbin.org/user-agent
[VER] [always_match2] Sent HTTP request to https://httpbin.org/user-agent
[always_match2:always_true] [http] [info] https://httpbin.org/user-agent
[VER] [never_match] Sent HTTP request to https://httpbin.org/user-agent

So it seems like the results of the templates got mixed inside the workflow. Any guess or idea?
Using Matcher Name based conditions might be a workaround, but in my case it's not an option, as I have templates with matchers-condition: and, and couldn't get it work by referring a named matcher.

[tovask]

The issue seems to be that all templates in a workflow use the same ScanContext, and they override each other's OnResult function.
The ScanContext is introduced by #4373

[Mzack9999]

I think workflows should have never been implemented in the actual way and should be deprecated as a whole. I think that the implementation should have been from the very beginning as proposed in #641 (comment) in a scripting like fashion.

As now finally a scripting engine has been introduced, it would be just more functional to extend the js protocol to accept a new native helper like run(...) or runWithOptions(...) as done in the linked PR over 3y ago and get rid of a good amount of hard to maintain recursive code. What do you think @ehsandeep ?

The reported example would become more concise and readable, something like:

if run(always_match1.yaml) {
    if run(never_match.yaml) {
        run(always_match3.yaml)
    }
    run(always_match2.yaml)
}

[tovask]

The big advantage of the workflow feature is that it allows templates to be executed from separate files, which is really useful when we have a lot of (conditionally concatenated) tests to run or when we want to reuse some templates in multiple scans.

[Mzack9999]

@tovask This would be already covered by the previous syntax with the additional benefit of more complex control flow. The template loading would not be limited to one file name but to patterns (ex. run("*_wordpress.yaml")) or (runWithOptions(tags=xxx,yyy))

[tovask]

@Mzack9999 you're right!
However this functionality in the js protocol is not implemented yet, so until that I would like this bug fixed, in order to be able to use the latest version.

Btw, another advantage of the workflows is that the templates (in the same level) can run in parallel. Not sure how hard to achieve that from js.