enrich the details of dsl evaluate errors

[akkuman]

Proposed changes

enrich the details of dsl evaluate errors

see #580

I think it is necessary to expose these errors

the error message can be see in https://github.com/Knetic/govaluate/blob/master/parameters.go#L27

Checklist

• Pull request is created against the dev branch
• All checks passed (lint, unit/integration/regression tests etc.) with my changes
• I have added tests that prove my fix is effective or that my feature works
• I have added necessary documentation (if appropriate)

[ehsandeep]

@akkuman few fixes and improvements about errors are added in #1372 in the dev branch also please resolve conflict errors and some examples run or example templates?

[akkuman]

@ehsandeep
testcase can be found at https://github.com/projectdiscovery/nuclei/pull/580/files#diff-98dc01124352997c04f517cf67c1705d4944514b930489433bf0125329a852e9R19
the relevant test cases can pass

[forgedhallpass]

If a target is given, the code will already show the correct DSL method signature (see #1295):

This being said, we should probably handle the case when no input is given with -u

[Mzack9999]

I think we should eventually expose the error with gologger and keep a tolerant logic with continue as follows:

compiled, err := govaluate.NewEvaluableExpressionWithFunctions(expr, dsl.HelperFunctions())
if err != nil {
	gologger.Warning().Msgf("%s\n", err)
	continue
}
result, err := compiled.Evaluate(base)
if err != nil {
	gologger.Warning().Msgf("%s\n", err)
	continue
}

The risk with an early return is being not able to use a valid template where the body contains strings matching any helper function and shouldn't be evaluated but sent literally, which is happening now via continue-on-error, for instance with the following template:

id: dsl-test

info:
  name: dsl literal in body
  author: test
  severity: info

requests:
  - raw:
      - |
        GET / HTTP/1.1
        Host: {{Hostname}}
        Content-Type: application/x-www-form-urlencoded

        {{test}} # literal 1
        {{to_lower("a", "b", "c", "d")}} # literal 2

    skip-variables-check: true

[akkuman]

@forgedhallpass there isn't error when the dsl function is in raw request

[akkuman]

@Mzack9999 I think it's good idea. But user cannot catch the error if they want to use nuclei from go code, like https://github.com/projectdiscovery/nuclei/blob/master/DESIGN.md#using-nuclei-from-go-code

hopefully we can find a better way

[forgedhallpass]

@akkuman good point. Will have to extend the tests to cover this scenario as well.

A possible solution could also be to collect and return a slice of errors, and let the client code decide what to do with it.

The logging part will be covered under #1166.

Since we'll soon introduce the REST API, using nuclei as a library will probably not be a priority.

[akkuman]

I think we should eventually expose the error with gologger and keep a tolerant logic with continue as follows:

compiled, err := govaluate.NewEvaluableExpressionWithFunctions(expr, dsl.HelperFunctions())
if err != nil {
	gologger.Warning().Msgf("%s\n", err)
	continue
}
result, err := compiled.Evaluate(base)
if err != nil {
	gologger.Warning().Msgf("%s\n", err)
	continue
}

The risk with an early return is being not able to use a valid template where the body contains strings matching any helper function and shouldn't be evaluated but sent literally, which is happening now via continue-on-error, for instance with the following template:

id: dsl-test

info:
  name: dsl literal in body
  author: test
  severity: info

requests:
  - raw:
      - |
        GET / HTTP/1.1
        Host: {{Hostname}}
        Content-Type: application/x-www-form-urlencoded

        {{test}} # literal 1
        {{to_lower("a", "b", "c", "d")}} # literal 2

    skip-variables-check: true

The situation you mentioned can be avoided. I added a test case

@Mzack9999 @forgedhallpass

[akkuman]

@forgedhallpass My solution does look ugly

A possible solution could also be to collect and return a slice of errors, and let the client code decide what to do with it.

if implement it, there will be a lot of code changes involved, I'm not sure it's worth it.

[akkuman]

But my solution doesn't cover every wrong situation

[Mzack9999]

On hold - Expression lexer is being reworked in #1516

[Mzack9999]

Closing - The PR seems not actual with actual core changes (lexer + dsl)