Add AST check to catch calls, remove regex for block and template

server/errors.go

@@ -136,7 +136,7 @@ var (
 	errHTTPBadRequestTemplateMessageTooLarge         = &errHTTP{40041, http.StatusBadRequest, "invalid request: message or title is too large after replacing template", "https://ntfy.sh/docs/publish/#message-templating", nil}
 	errHTTPBadRequestTemplateMessageNotJSON          = &errHTTP{40042, http.StatusBadRequest, "invalid request: message body must be JSON if templating is enabled", "https://ntfy.sh/docs/publish/#message-templating", nil}
 	errHTTPBadRequestTemplateInvalid                 = &errHTTP{40043, http.StatusBadRequest, "invalid request: could not parse template", "https://ntfy.sh/docs/publish/#message-templating", nil}
-	errHTTPBadRequestTemplateDisallowedFunctionCalls = &errHTTP{40044, http.StatusBadRequest, "invalid request: template contains disallowed function calls, e.g. template, call, or define", "https://ntfy.sh/docs/publish/#message-templating", nil}
+	errHTTPBadRequestTemplateDisallowedFunctionCalls = &errHTTP{40044, http.StatusBadRequest, "invalid request: template contains disallowed function calls, e.g. template, call, define, or block", "https://ntfy.sh/docs/publish/#message-templating", nil}
 	errHTTPBadRequestTemplateExecuteFailed           = &errHTTP{40045, http.StatusBadRequest, "invalid request: template execution failed", "https://ntfy.sh/docs/publish/#message-templating", nil}
 	errHTTPBadRequestTemplateExecutionTimeout        = &errHTTP{40055, http.StatusBadRequest, "invalid request: template execution timed out", "https://ntfy.sh/docs/publish/#message-templating", nil}
 	errHTTPBadRequestInvalidUsername                 = &errHTTP{40046, http.StatusBadRequest, "invalid request: invalid username", "", nil}

server/server.go

@@ -150,10 +150,7 @@ var (
 	templatesFs  embed.FS // Contains template config files (e.g. grafana.yml, github.yml, ...)
 	templatesDir = "templates"
 
-	// templateDisallowedRegex tests a template for disallowed expressions. While not really dangerous, they
-	// are not useful, and seem potentially troublesome.
-	templateDisallowedRegex = regexp.MustCompile(`(?m)\{\{-?\s*(call|template|define)\b`)
-	templateNameRegex       = regexp.MustCompile(`^[-_A-Za-z0-9]+$`)
+	templateNameRegex = regexp.MustCompile(`^[-_A-Za-z0-9]+$`)
 )
 
 const (

server/server_template.go

@@ -7,6 +7,7 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
+	"text/template/parse"
 	"time"
 
 	"gopkg.in/yaml.v2"
@@ -105,9 +106,6 @@ func (s *Server) renderTemplateFromParams(m *model.Message, peekedBody string, p
 
 // renderTemplate renders a template with the given JSON source data.
 func (s *Server) renderTemplate(name, tpl, source string) (string, error) {
-	if templateDisallowedRegex.MatchString(tpl) {
-		return "", errHTTPBadRequestTemplateDisallowedFunctionCalls
-	}
 	var data any
 	if err := json.Unmarshal([]byte(source), &data); err != nil {
 		return "", errHTTPBadRequestTemplateMessageNotJSON
@@ -116,6 +114,9 @@ func (s *Server) renderTemplate(name, tpl, source string) (string, error) {
 	if err != nil {
 		return "", errHTTPBadRequestTemplateInvalid.Wrap("%s", err.Error())
 	}
+	if templateUsesDisallowedFeatures(t) {
+		return "", errHTTPBadRequestTemplateDisallowedFunctionCalls
+	}
 	t.SetExecutionDeadline(time.Now().Add(templateMaxExecutionTime)) // Bail out of runaway templates (GHSA-rhwf-xgc9-m9fp)
 	var buf bytes.Buffer
 	limitWriter := util.NewLimitWriter(&buf, util.NewFixedLimiter(templateMaxOutputBytes))
@@ -127,3 +128,59 @@ func (s *Server) renderTemplate(name, tpl, source string) (string, error) {
 	}
 	return strings.TrimSpace(strings.ReplaceAll(buf.String(), "\\n", "\n")), nil // replace any remaining "\n" (those outside of template curly braces) with newlines
 }
+
+// templateUsesDisallowedFeatures reports whether the parsed template defines or invokes a
+// sub-template ({{define}}/{{block}}/{{template}}) or uses the {{call}} builtin. None are useful for
+// ntfy's JSON-data templates. Checking the parse tree (rather than the raw string) catches every
+// syntactic form -- e.g. {{if call .x}} or {{$y := call .x}} -- that a regex would miss.
+func templateUsesDisallowedFeatures(t *gotext.Template) bool {
+	if len(t.Templates()) > 1 { // {{define}}/{{block}} create additional associated templates
+		return true
+	}
+	return treeContainsDisallowedNode(t.Root)
+}
+
+// treeContainsDisallowedNode reports whether the parse tree contains a {{template}}/{{block}}
+// invocation or a {{call}} builtin, descending into pipes and command arguments (where {{call}} can
+// appear anywhere a function is allowed).
+func treeContainsDisallowedNode(node parse.Node) bool {
+	switch n := node.(type) {
+	case *parse.ListNode:
+		if n == nil {
+			return false
+		}
+		for _, child := range n.Nodes {
+			if treeContainsDisallowedNode(child) {
+				return true
+			}
+		}
+	case *parse.ActionNode:
+		return treeContainsDisallowedNode(n.Pipe)
+	case *parse.RangeNode:
+		return treeContainsDisallowedNode(n.Pipe) || treeContainsDisallowedNode(n.List) || treeContainsDisallowedNode(n.ElseList)
+	case *parse.IfNode:
+		return treeContainsDisallowedNode(n.Pipe) || treeContainsDisallowedNode(n.List) || treeContainsDisallowedNode(n.ElseList)
+	case *parse.WithNode:
+		return treeContainsDisallowedNode(n.Pipe) || treeContainsDisallowedNode(n.List) || treeContainsDisallowedNode(n.ElseList)
+	case *parse.TemplateNode: // {{template}} or {{block}} invocation
+		return true
+	case *parse.PipeNode:
+		if n == nil {
+			return false
+		}
+		for _, cmd := range n.Cmds {
+			if treeContainsDisallowedNode(cmd) {
+				return true
+			}
+		}
+	case *parse.CommandNode:
+		for _, arg := range n.Args {
+			if treeContainsDisallowedNode(arg) {
+				return true
+			}
+		}
+	case *parse.IdentifierNode: // a function name; {{call}} is the disallowed builtin
+		return n.Ident == "call"
+	}
+	return false
+}

server/server_test.go

@@ -3660,6 +3660,70 @@ func TestServer_MessageTemplate_ExecutionTimeout(t *testing.T) {
 	})
 }
 
+// TestServer_MessageTemplate_DataDrivenNestedRange_TimesOut is the regression for the exact hole the
+// old write-triggered TimeoutWriter missed: a nested {{range}} over a JSON array field with a
+// no-output body calls no function, so only the executor's wall-clock deadline can stop it
+// (GHSA-rhwf-xgc9-m9fp).
+func TestServer_MessageTemplate_DataDrivenNestedRange_TimesOut(t *testing.T) {
+	forEachBackend(t, func(t *testing.T, databaseURL string) {
+		t.Parallel()
+		s := newTestServer(t, newTestConfig(t, databaseURL))
+		elems := make([]string, 1000)
+		for i := range elems {
+			elems[i] = "0"
+		}
+		jsonBody := `{"a":[` + strings.Join(elems, ",") + `]}`
+		msg := `{{range .a}}{{range $.a}}{{range $.a}}{{$x := .}}{{end}}{{end}}{{end}}done`
+		start := time.Now()
+		response := request(t, s, "POST", "/mytopic", jsonBody, map[string]string{
+			"X-Message":  msg,
+			"X-Template": "1",
+		})
+		elapsed := time.Since(start)
+		require.Equal(t, 400, response.Code)
+		require.Equal(t, 40055, toHTTPError(t, response.Body.String()).Code)
+		require.Less(t, elapsed, 500*time.Millisecond, "data-driven nested range should be cut off by the deadline (took %s)", elapsed)
+	})
+}
+
+// TestServer_MessageTemplate_ExpensiveFunctionLoop_TimesOut ensures the deadline also bounds loops
+// whose body calls an expensive function (hashing a large string), where a single call between
+// deadline checks could otherwise overshoot (GHSA-rhwf-xgc9-m9fp).
+func TestServer_MessageTemplate_ExpensiveFunctionLoop_TimesOut(t *testing.T) {
+	forEachBackend(t, func(t *testing.T, databaseURL string) {
+		t.Parallel()
+		s := newTestServer(t, newTestConfig(t, databaseURL))
+		msg := `{{$big := repeat 990 "0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789"}}{{range until 1000}}{{range until 1000}}{{$h := sha512sum $big}}{{end}}{{end}}`
+		start := time.Now()
+		response := request(t, s, "POST", "/mytopic", `{}`, map[string]string{
+			"X-Message":  msg,
+			"X-Template": "1",
+		})
+		elapsed := time.Since(start)
+		require.Equal(t, 400, response.Code)
+		require.Equal(t, 40055, toHTTPError(t, response.Body.String()).Code)
+		require.Less(t, elapsed, 1500*time.Millisecond, "expensive-function loop should be cut off by the deadline (took %s)", elapsed)
+	})
+}
+
+// TestServer_MessageTemplate_NestedLoopPoC_TimesOut is the exact proof-of-concept from the advisory:
+// a range over a runtime-computed slice, nested, must be bounded by the deadline (GHSA-rhwf-xgc9-m9fp).
+func TestServer_MessageTemplate_NestedLoopPoC_TimesOut(t *testing.T) {
+	forEachBackend(t, func(t *testing.T, databaseURL string) {
+		t.Parallel()
+		s := newTestServer(t, newTestConfig(t, databaseURL))
+		start := time.Now()
+		response := request(t, s, "POST", "/mytopic", `{}`, map[string]string{
+			"X-Message":  `{{$x := until 10000}}{{range $x}}{{range $x}}{{end}}{{end}}done`,
+			"X-Template": "1",
+		})
+		elapsed := time.Since(start)
+		require.Equal(t, 400, response.Code)
+		require.Equal(t, 40055, toHTTPError(t, response.Body.String()).Code)
+		require.Less(t, elapsed, 500*time.Millisecond, "advisory PoC should be cut off by the deadline (took %s)", elapsed)
+	})
+}
+
 func TestServer_MessageTemplate_GenuineError_NotTimeout(t *testing.T) {
 	forEachBackend(t, func(t *testing.T, databaseURL string) {
 		t.Parallel()
@@ -3810,11 +3874,18 @@ func TestServer_MessageTemplate_DisallowedCalls(t *testing.T) {
 			`{{- template ""}}`,
 			`{{-
 template ""}}`,
-			`{{      call abc}}`,
-			`{{      define "aa"}}`,
-			`We cannot {{define "aa"}}`,
+			`{{ call "aa"}}`,
+			`{{define "aa"}}hi{{end}}`,
+			`We cannot {{define "aa"}}hi{{end}}`,
 			`We cannot {{ call "aa"}}`,
 			`We cannot {{- template "aa"}}`,
+			`{{block "aa" .}}hi{{end}}`,
+			`We cannot {{- block "aa" .}}hi{{end}}`,
+			// call is a function, not a keyword, so it can hide in non-leading positions that a
+			// raw-string regex misses -- the parse-tree walk catches all of them.
+			`{{if call .x}}x{{end}}`,
+			`{{$y := call .x}}`,
+			`{{index (call .x) 0}}`,
 		}
 		for _, disallowedTemplate := range disallowedTemplates {
 			messageTemplate := disallowedTemplate