From 7ed7fea081e54743f5b3a836c4bffddf0ad8588f Mon Sep 17 00:00:00 2001 From: binwiederhier Date: Mon, 3 Aug 2026 18:34:29 +0200 Subject: [PATCH] Limit memory usage in templates --- docs/publish.md | 5 +- docs/releases.md | 1 + server/errors.go | 1 + server/server.go | 13 ---- server/server_template.go | 52 ++++++++++++- server/server_template_test.go | 131 +++++++++++++++++++++++++++++++++ util/sprig/functions.go | 1 + util/sprig/strings.go | 7 ++ util/sprig/strings_test.go | 20 +++++ 9 files changed, 216 insertions(+), 15 deletions(-) create mode 100644 server/server_template_test.go diff --git a/docs/publish.md b/docs/publish.md index e66356d6..a95b5458 100644 --- a/docs/publish.md +++ b/docs/publish.md @@ -3227,7 +3227,10 @@ your templates there first ([example for Grafana alert](https://repeatit.io/#/sh !!! info A few Go template features are disabled for user-supplied templates: `{{define}}`, `{{template}}`, `{{block}}`, and `{{call}}` are not allowed. Templates also run with a short execution time limit -- - a template that loops too long is stopped and rejected with an HTTP 400 error. + a template that loops too long is stopped and rejected with an HTTP 400 error. Templates are + limited to 32 KB in size, `printf` widths and precisions must be below 1000 (`%999d` is + allowed, `%1000d` is not), including the `%*d` form that takes the width from an argument, and + `indent`/`nindent` are limited to 100 spaces. ### Template functions ntfy supports a subset of the **[Sprig template functions](publish/template-functions.md)** (originally copied from [Sprig](https://github.com/Masterminds/sprig), diff --git a/docs/releases.md b/docs/releases.md index dbab6f5e..4ae45592 100644 --- a/docs/releases.md +++ b/docs/releases.md @@ -2056,6 +2056,7 @@ and the [ntfy Android app](https://github.com/binwiederhier/ntfy-android/release **Security:** * Exclude secrets (Stripe/Twilio/web push keys, SMTP password, provisioned users and tokens) from the config hash served to the web app +* Limit message templates (`Template: yes`) to 32 KB, limit `printf` widths and precisions to below 1000, and limit `indent`/`nindent` to 100 spaces, preventing excessive memory use from a single small template **Features:** diff --git a/server/errors.go b/server/errors.go index a97df25a..3e03894f 100644 --- a/server/errors.go +++ b/server/errors.go @@ -148,6 +148,7 @@ var ( errHTTPBadRequestEmailAddressNotVerified = &errHTTP{40052, http.StatusBadRequest, "invalid request: email address not verified", "https://ntfy.sh/docs/publish/#e-mail-notifications", nil} errHTTPBadRequestAnonymousEmailNotAllowed = &errHTTP{40053, http.StatusBadRequest, "invalid request: anonymous email sending is not allowed", "https://ntfy.sh/docs/publish/#e-mail-notifications", nil} errHTTPBadRequestResetLinkInvalid = &errHTTP{40054, http.StatusBadRequest, "invalid request: password reset link invalid or expired", "", nil} + errHTTPBadRequestTemplateTooLarge = &errHTTP{40056, http.StatusBadRequest, "invalid request: template too large", "https://ntfy.sh/docs/publish/#message-templating", nil} errHTTPNotFound = &errHTTP{40401, http.StatusNotFound, "page not found", "", nil} errHTTPUnauthorized = &errHTTP{40101, http.StatusUnauthorized, "unauthorized", "https://ntfy.sh/docs/publish/#authentication", nil} errHTTPForbidden = &errHTTP{40301, http.StatusForbidden, "forbidden", "https://ntfy.sh/docs/publish/#authentication", nil} diff --git a/server/server.go b/server/server.go index c8476dda..eab007ab 100644 --- a/server/server.go +++ b/server/server.go @@ -150,17 +150,6 @@ var ( //go:embed docs docsStaticFs embed.FS docsStaticCached = &util.CachingEmbedFS{ModTime: time.Now(), FS: docsStaticFs} - - //go:embed templates - templatesFs embed.FS // Contains template config files (e.g. grafana.yml, github.yml, ...) - templatesDir = "templates" - - templateNameRegex = regexp.MustCompile(`^[-_A-Za-z0-9]+$`) - - // templateMaxExecutionTime is the wall-clock deadline for a single template render, a DoS guard - // (GHSA-rhwf-xgc9-m9fp). It is a var (not a const) solely so tests can raise it; it is never - // mutated in production. - templateMaxExecutionTime = 100 * time.Millisecond ) const ( @@ -174,8 +163,6 @@ const ( unifiedPushTopicPrefix = "up" // Temporarily, we rate limit all "up*" topics based on the subscriber unifiedPushTopicLength = 14 // Length of UnifiedPush topics, including the "up" part messagesHistoryMax = 10 // Number of message count values to keep in memory - templateMaxOutputBytes = 1024 * 1024 // Maximum number of bytes a template can output, used to prevent DoS attacks - templateFileExtension = ".yml" // Template files must end with this extension ) // WebSocket constants diff --git a/server/server_template.go b/server/server_template.go index bed6e9f1..76639b58 100644 --- a/server/server_template.go +++ b/server/server_template.go @@ -3,12 +3,16 @@ package server import ( "bytes" "context" + "embed" "encoding/json" "errors" + "fmt" "os" "path/filepath" + "regexp" "strings" "text/template/parse" + "time" "gopkg.in/yaml.v2" "heckel.io/ntfy/v2/model" @@ -17,6 +21,32 @@ import ( "heckel.io/ntfy/v2/util/sprig" ) +var ( + //go:embed templates + templatesFs embed.FS // Contains template config files (e.g. grafana.yml, github.yml, ...) + templatesDir = "templates" + + templateNameRegex = regexp.MustCompile(`^[-_A-Za-z0-9]+$`) + + // templatePrintfLargeSizeRegex matches a printf directive whose width or precision is a star + // (taken from an argument) or has four or more digits, i.e. is at least 1000. It deliberately + // scans the flag/width/precision characters after a % without requiring a well-formed + // directive: fmt pads even malformed ones (e.g. "%000 9999999#" emits 10 MB), so anything + // unrecognized must still be caught. + templatePrintfLargeSizeRegex = regexp.MustCompile(`%[-+# 0-9.*\[\]]*(\*|[0-9]{4})`) + + // templateMaxExecutionTime is the wall-clock deadline for a single template render, a DoS guard + // (GHSA-rhwf-xgc9-m9fp). It is a var (not a const) solely so tests can raise it; it is never + // mutated in production. + templateMaxExecutionTime = 100 * time.Millisecond +) + +const ( + templateMaxOutputBytes = 1024 * 1024 // Maximum number of bytes a template can output, used to prevent DoS attacks + templateMaxTemplateBytes = 32 * 1024 // Maximum size of a template (inline or from a template file), used to prevent DoS attacks + templateFileExtension = ".yml" // Template files must end with this extension +) + func (s *Server) handleBodyAsTemplatedTextMessage(ctx context.Context, m *model.Message, template templateMode, body *util.PeekedReadCloser, priorityStr string) error { body, err := util.Peek(body, max(s.config.MessageSizeLimit, jsonBodyBytesLimit)) if err != nil { @@ -106,11 +136,14 @@ func (s *Server) renderTemplateFromParams(ctx context.Context, m *model.Message, // renderTemplate renders a template with the given JSON source data. func (s *Server) renderTemplate(ctx context.Context, name, tpl, source string) (string, error) { + if len(tpl) > templateMaxTemplateBytes { + return "", errHTTPBadRequestTemplateTooLarge + } var data any if err := json.Unmarshal([]byte(source), &data); err != nil { return "", errHTTPBadRequestTemplateMessageNotJSON } - t, err := gotext.New("").Funcs(sprig.TxtFuncMap()).Parse(tpl) + t, err := gotext.New("").Funcs(sprig.TxtFuncMap()).Funcs(gotext.FuncMap{"printf": templatePrintf}).Parse(tpl) if err != nil { return "", errHTTPBadRequestTemplateInvalid.Wrap("%s", err.Error()) } @@ -168,6 +201,8 @@ func treeContainsDisallowedNode(node parse.Node) bool { return treeContainsDisallowedNode(n.Pipe) || treeContainsDisallowedNode(n.List) || treeContainsDisallowedNode(n.ElseList) case *parse.TemplateNode: // {{template}} or {{block}} invocation return true + case *parse.ChainNode: // A term followed by field accesses, e.g. (call .x).y + return treeContainsDisallowedNode(n.Node) case *parse.PipeNode: if n == nil { return false @@ -188,3 +223,18 @@ func treeContainsDisallowedNode(node parse.Node) bool { } return false } + +// templatePrintf is the template builtin printf, guarded against memory amplification: fmt +// allows widths and precisions up to 1e6 per verb, so a small template like +// {{printf "%999999d%999999d..." ...}} can allocate gigabytes inside a single fmt call -- and the +// executor's cancellation context is only checked between template nodes, never inside one. +// Widths and precisions of 1000 or more are therefore rejected, as is the star (*) form, which +// takes the width from an argument. Combined with the template size limit, this bounds a single +// render to a few MB. Registered via Funcs, which takes precedence over the builtin, and checked +// at call time so a format string assembled during execution is covered too. +func templatePrintf(format string, args ...any) (string, error) { + if templatePrintfLargeSizeRegex.MatchString(strings.ReplaceAll(format, "%%", "")) { // Strip escaped percent signs, they take no width + return "", errors.New("printf width or precision too large") + } + return fmt.Sprintf(format, args...), nil +} diff --git a/server/server_template_test.go b/server/server_template_test.go new file mode 100644 index 00000000..61e69b9b --- /dev/null +++ b/server/server_template_test.go @@ -0,0 +1,131 @@ +package server + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestServer_MessageTemplate_TooLarge(t *testing.T) { + forEachBackend(t, func(t *testing.T, databaseURL string) { + t.Parallel() + s := newTestServer(t, newTestConfig(t, databaseURL)) + response := request(t, s, "PUT", "/mytopic", `{"foo":"bar"}`, map[string]string{ + "X-Message": "{{.foo}}" + strings.Repeat("x", 33*1024), + "X-Template": "1", + }) + require.Equal(t, 400, response.Code) + require.Equal(t, 40056, toHTTPError(t, response.Body.String()).Code) + }) +} + +func TestServer_MessageTemplate_PrintfWidthTooLarge(t *testing.T) { + forEachBackend(t, func(t *testing.T, databaseURL string) { + t.Parallel() + s := newTestServer(t, newTestConfig(t, databaseURL)) + // A handful of 1MB-wide verbs would allocate several MB inside a single fmt call, where + // the executor's context is never checked; the printf guard must reject the call before + // fmt runs, not after the limit writer sees the output + response := request(t, s, "PUT", "/mytopic", `{"n":1}`, map[string]string{ + "X-Message": `{{printf "%1000000d%1000000d%1000000d" .n .n .n}}`, + "X-Template": "1", + }) + require.Equal(t, 400, response.Code) + require.Equal(t, 40045, toHTTPError(t, response.Body.String()).Code) + require.Contains(t, response.Body.String(), "printf width or precision too large") + }) +} + +func TestServer_MessageTemplate_PrintfWidthTooLarge_DynamicFormat(t *testing.T) { + forEachBackend(t, func(t *testing.T, databaseURL string) { + t.Parallel() + s := newTestServer(t, newTestConfig(t, databaseURL)) + // The format string is assembled at execution time, so the guard must inspect the actual + // argument, not the template source + response := request(t, s, "PUT", "/mytopic", `{"n":1}`, map[string]string{ + "X-Message": `{{$f := print "%" "999999" "d" "%" "999999" "d"}}{{printf $f .n .n}}`, + "X-Template": "1", + }) + require.Equal(t, 400, response.Code) + require.Contains(t, response.Body.String(), "printf width or precision too large") + }) +} + +func TestServer_MessageTemplate_PrintfStarWidthTooLarge(t *testing.T) { + forEachBackend(t, func(t *testing.T, databaseURL string) { + t.Parallel() + s := newTestServer(t, newTestConfig(t, databaseURL)) + // Star widths take the width from an argument; sprig's math functions (int64 results) + // make large integer arguments reachable from a template + response := request(t, s, "PUT", "/mytopic", `{"n":1}`, map[string]string{ + "X-Message": `{{printf "%*d" (mul 1000 2000) 1}}`, + "X-Template": "1", + }) + require.Equal(t, 400, response.Code) + require.Contains(t, response.Body.String(), "printf width or precision too large") + }) +} + +func TestServer_MessageTemplate_PrintfSmallWidthStillWorks(t *testing.T) { + forEachBackend(t, func(t *testing.T, databaseURL string) { + t.Parallel() + s := newTestServer(t, newTestConfig(t, databaseURL)) + response := request(t, s, "PUT", "/mytopic", `{"n":7}`, map[string]string{ + "X-Message": `{{printf "%05d" 7}}`, + "X-Template": "1", + }) + require.Equal(t, 200, response.Code) + require.Equal(t, "00007", toMessage(t, response.Body.String()).Message) + }) +} + +func Test_templatePrintf(t *testing.T) { + tests := []struct { + format string + args []any + want string // Empty means the call must be rejected + }{ + {"%d", []any{5}, "5"}, + {"%05d", []any{5}, "00005"}, + {"%-8.3f|", []any{1.5}, "1.500 |"}, + {"%1000d", []any{1}, ""}, // Rejected: four digits + {"%.1000s", []any{"x"}, ""}, + {"%*d", []any{500, 1}, ""}, // Rejected: star width + {"%.*s", []any{400, "x"}, ""}, // Rejected: star precision + {"%[1]1000000d", []any{1}, ""}, // Rejected: explicit arg index does not hide the width + {"%[2]*[1]d", []any{6, 12}, ""}, // Rejected: star width behind an arg index + {"100%% of 2024 values", nil, "100% of 2024 values"}, // Literal digits are not a width + } + for _, test := range tests { + out, err := templatePrintf(test.format, test.args...) + if test.want == "" { + require.Error(t, err, "format %q must be rejected", test.format) + require.Contains(t, err.Error(), "too large") + } else { + require.Nil(t, err, "format %q", test.format) + require.Equal(t, test.want, out) + } + } + + // The largest allowed width still produces bounded output + out, err := templatePrintf("%999d", 1) + require.Nil(t, err) + require.Len(t, out, 999) +} + +func TestServer_MessageTemplate_DisallowedCallInChain(t *testing.T) { + forEachBackend(t, func(t *testing.T, databaseURL string) { + t.Parallel() + s := newTestServer(t, newTestConfig(t, databaseURL)) + // {{call}} behind a field access parses into a ChainNode. JSON data cannot produce a + // function value, so this cannot be exploited today, but the ban must catch every + // syntactic form rather than relying on the call failing at runtime. + response := request(t, s, "PUT", "/mytopic", `{"fn":1}`, map[string]string{ + "X-Message": `{{(call .fn).x}}`, + "X-Template": "1", + }) + require.Equal(t, 400, response.Code) + require.Equal(t, 40044, toHTTPError(t, response.Body.String()).Code) + }) +} diff --git a/util/sprig/functions.go b/util/sprig/functions.go index 72b46aaf..bd52c254 100644 --- a/util/sprig/functions.go +++ b/util/sprig/functions.go @@ -12,6 +12,7 @@ const ( loopExecutionLimit = 10_000 // Limit the number of loop executions to prevent execution from taking too long stringLengthLimit = 100_000 // Limit the length of strings to prevent memory issues sliceSizeLimit = 10_000 // Limit the size of slices to prevent memory issues + indentSpacesLimit = 100 // Limit indentation width to prevent memory issues; indent allocates spaces*lines bytes ) // TxtFuncMap produces the function map. diff --git a/util/sprig/strings.go b/util/sprig/strings.go index e64f82d9..f14eb8af 100644 --- a/util/sprig/strings.go +++ b/util/sprig/strings.go @@ -116,6 +116,7 @@ func cat(v ...any) string { } // indent adds a specified number of spaces at the beginning of each line in a string. +// It has a safety limit to prevent excessive memory usage. // // Parameters: // - spaces: The number of spaces to add @@ -123,7 +124,13 @@ func cat(v ...any) string { // // Returns: // - string: The indented string +// +// Panics: +// - If spaces exceeds indentSpacesLimit func indent(spaces int, v string) string { + if spaces > indentSpacesLimit { + panic(fmt.Sprintf("indent %d exceeds limit of %d", spaces, indentSpacesLimit)) + } pad := strings.Repeat(" ", spaces) return pad + strings.Replace(v, "\n", "\n"+pad, -1) } diff --git a/util/sprig/strings_test.go b/util/sprig/strings_test.go index 1e91d9b2..40ce38df 100644 --- a/util/sprig/strings_test.go +++ b/util/sprig/strings_test.go @@ -4,6 +4,7 @@ import ( "encoding/base32" "encoding/base64" "fmt" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -214,6 +215,25 @@ func TestNindent(t *testing.T) { } } +func TestIndentLimit(t *testing.T) { + // Indentation beyond a sane width is an amplification attempt: indent allocates + // spaces * lines bytes in a single uninterruptible call + if err := runt(`{{indent 100 "a"}}`, strings.Repeat(" ", 100)+"a"); err != nil { + t.Error(err) + } + for _, tpl := range []string{ + `{{indent 101 "a"}}`, + `{{nindent 101 "a"}}`, + `{{indent 1000000000 "a"}}`, + } { + if _, err := runRaw(tpl, nil); err == nil { + t.Errorf("expected %s to be rejected", tpl) + } else if !strings.Contains(err.Error(), "exceeds limit") { + t.Errorf("expected limit error for %s, got: %v", tpl, err) + } + } +} + func TestReplace(t *testing.T) { tpl := `{{"I Am Henry VIII" | replace " " "-"}}` if err := runt(tpl, "I-Am-Henry-VIII"); err != nil {