Skip to content

Commit a3f3c99

Browse files
committed
fix(expressions): avoid helper eval in literal checks
`hasLiteralsOnly()` currently evaluates helper expressions while deciding whether "{{...}}" contains unresolved variables, which makes validation paths run side-effectful helpers. Just replace that runtime eval with a `Vars()` len check so unresolved-variable detection literally stays literal (am I writing it right?), and of course side-effect free. Also make `Evaluate()` return template-authored expression compile/eval errors instead of logging and then keep continuing, so malformed helper calls still fail in the rendering path. Fixes #7320 Signed-off-by: Dwi Siswanto <git@dw1.io>
1 parent 3537030 commit a3f3c99

File tree

4 files changed

+106
-17
lines changed

4 files changed

+106
-17
lines changed

pkg/protocols/common/expressions/expressions.go

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
package expressions
22

33
import (
4+
"fmt"
45
"strings"
56

67
"github.com/Knetic/govaluate"
7-
"github.com/projectdiscovery/gologger"
88

99
"github.com/projectdiscovery/nuclei/v3/pkg/operators/common/dsl"
1010
"github.com/projectdiscovery/nuclei/v3/pkg/protocols/common/marker"
@@ -53,28 +53,40 @@ func evaluate(data string, base map[string]interface{}) (string, error) {
5353
// - complex: containing helper functions [ + variables]
5454
// literals like {{2+2}} are not considered expressions
5555
for _, expression := range expressions {
56+
originalExpression := expression
5657
// replace variable placeholders with base values
5758
expression = replacer.Replace(expression, base)
59+
5860
// turns expressions (either helper functions+base values or base values)
5961
compiled, err := govaluate.NewEvaluableExpressionWithFunctions(expression, dsl.HelperFunctions)
6062
if err != nil {
61-
gologger.Warning().Msgf("Failed to compile expression '%s': %v", expression, err)
62-
continue
63-
}
64-
// propagate unresolved {{...}} markers from variable values so the
65-
// downstream ContainsUnresolvedVariables check can detect them instead
66-
// of having encoding functions (e.g. base64) hide them
67-
if markers := unresolvedVarMarkers(compiled.Vars(), base); markers != "" {
68-
data = replacer.ReplaceOne(data, expression, markers)
69-
continue
63+
return data, fmt.Errorf("failed to compile expression %q: %w", originalExpression, err)
7064
}
65+
7166
result, err := compiled.Evaluate(base)
7267
if err != nil {
73-
gologger.Warning().Msgf("Failed to evaluate expression '%s': %v", expression, err)
74-
continue
68+
return data, fmt.Errorf("failed to evaluate expression %q: %w", originalExpression, err)
7569
}
70+
71+
replacement := result
72+
// Preserve unresolved markers only when a helper call would otherwise
73+
// hide them from downstream validation. Plain expressions such as
74+
// comparisons should evaluate normally.
75+
if markers := unresolvedVarMarkers(compiled.Vars(), base); markers != "" {
76+
usesFunctions := false
77+
for _, token := range compiled.Tokens() {
78+
if token.Kind == govaluate.FUNCTION {
79+
usesFunctions = true
80+
break
81+
}
82+
}
83+
if usesFunctions && ContainsUnresolvedVariables(fmt.Sprint(result)) == nil {
84+
replacement = markers
85+
}
86+
}
87+
7688
// replace incrementally
77-
data = replacer.ReplaceOne(data, expression, result)
89+
data = replacer.ReplaceOne(data, expression, replacement)
7890
}
7991
return data, nil
8092
}

pkg/protocols/common/expressions/expressions_test.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,3 +105,50 @@ func TestEvaluateDoesNotReinterpretResolvedValues(t *testing.T) {
105105
})
106106
}
107107
}
108+
109+
func TestEvaluateDoesNotExecuteHelpersFromResolvedValues(t *testing.T) {
110+
var calls int
111+
112+
withTestHelperFunction(t, "test_side_effect", func(args ...interface{}) (interface{}, error) {
113+
calls++
114+
return "ok", nil
115+
})
116+
117+
value, err := Evaluate("{{body}}", map[string]interface{}{
118+
"body": "{{test_side_effect(1)}}",
119+
})
120+
require.NoError(t, err)
121+
require.Equal(t, "{{test_side_effect(1)}}", value)
122+
require.Zero(t, calls)
123+
}
124+
125+
func TestEvaluateReturnsErrorForInvalidTemplateExpression(t *testing.T) {
126+
_, err := Evaluate("{{base64()}}", map[string]interface{}{})
127+
require.Error(t, err)
128+
require.ErrorContains(t, err, `failed to evaluate expression "base64()"`)
129+
}
130+
131+
func TestEvaluateErrorDoesNotLeakResolvedValues(t *testing.T) {
132+
_, err := Evaluate("{{base64('{{secret_token}}', 'extra')}}", map[string]interface{}{
133+
"secret_token": "top-secret-cia-mi6-kgb-mossad-classified",
134+
})
135+
require.Error(t, err)
136+
require.ErrorContains(t, err, `failed to evaluate expression "base64('{{secret_token}}', 'extra')"`)
137+
require.NotContains(t, err.Error(), "top-secret-cia-mi6-kgb-mossad-classified")
138+
}
139+
140+
func TestEvaluatePlainExpressionsWithMarkerLikeValues(t *testing.T) {
141+
value, err := Evaluate("{{body != ''}}", map[string]interface{}{
142+
"body": "{{contact_id}}",
143+
})
144+
require.NoError(t, err)
145+
require.Equal(t, "true", value)
146+
}
147+
148+
func TestEvaluatePreservesVisibleMarkersFromHelperResults(t *testing.T) {
149+
value, err := Evaluate("{{concat(body, '-x')}}", map[string]interface{}{
150+
"body": "{{contact_id}}",
151+
})
152+
require.NoError(t, err)
153+
require.Equal(t, "{{contact_id}}-x", value)
154+
}

pkg/protocols/common/expressions/variables.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -120,9 +120,10 @@ func hasLiteralsOnly(data string) bool {
120120
if err != nil {
121121
return false
122122
}
123-
if expr != nil {
124-
_, err = expr.Evaluate(nil)
125-
return err == nil
123+
124+
if expr == nil {
125+
return true
126126
}
127-
return true
127+
128+
return len(expr.Vars()) == 0
128129
}

pkg/protocols/common/expressions/variables_test.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,26 @@ import (
44
"errors"
55
"testing"
66

7+
"github.com/Knetic/govaluate"
8+
"github.com/projectdiscovery/nuclei/v3/pkg/operators/common/dsl"
79
"github.com/stretchr/testify/require"
810
)
911

12+
func withTestHelperFunction(t *testing.T, name string, fn govaluate.ExpressionFunction) {
13+
t.Helper()
14+
15+
originalFn, hadFn := dsl.HelperFunctions[name]
16+
dsl.HelperFunctions[name] = fn
17+
18+
t.Cleanup(func() {
19+
if hadFn {
20+
dsl.HelperFunctions[name] = originalFn
21+
return
22+
}
23+
delete(dsl.HelperFunctions, name)
24+
})
25+
}
26+
1027
func TestUnresolvedVariablesCheck(t *testing.T) {
1128
tests := []struct {
1229
data string
@@ -26,3 +43,15 @@ func TestUnresolvedVariablesCheck(t *testing.T) {
2643
require.Equal(t, test.err, err, "could not get unresolved variables")
2744
}
2845
}
46+
47+
func TestUnresolvedVariablesCheckDoesNotExecuteHelpers(t *testing.T) {
48+
var calls int
49+
withTestHelperFunction(t, "test_side_effect", func(args ...interface{}) (interface{}, error) {
50+
calls++
51+
return "ok", nil
52+
})
53+
54+
err := ContainsUnresolvedVariables("{{test_side_effect(1)}}")
55+
require.NoError(t, err)
56+
require.Zero(t, calls)
57+
}

0 commit comments

Comments
 (0)