Browse Source

fix(template): avoid DoS in truncate() when processing untrusted input

Avoid scanning and allocating entire untrusted feed titles during
template rendering just to truncate them. The previous
implementation could turn large remote titles into unnecessary CPU
and memory pressure on each page render.

Iterating only until the requested rune boundary preserves
Unicode-safe truncation while keeping the work proportional to the
visible output instead of the full input.
Frédéric Guillot 3 weeks ago
parent
commit
2d3a9aebb4
2 changed files with 76 additions and 23 deletions
  1. 14 2
      internal/template/functions.go
  2. 62 21
      internal/template/functions_test.go

+ 14 - 2
internal/template/functions.go

@@ -211,9 +211,21 @@ func dict(values ...any) (map[string]any, error) {
 }
 
 func truncate(str string, max int) string {
-	if runes := []rune(str); len(runes) > max {
-		return string(runes[:max]) + "…"
+	if max <= 0 {
+		panic("truncate: max must be greater than zero")
 	}
+
+	// Template callers pass feed titles from remote content. Scanning and
+	// allocating the entire untrusted input just to truncate it could create a
+	// denial-of-service risk, so stop as soon as we reach the requested limit.
+	runeCount := 0
+	for i := range str {
+		if runeCount == max {
+			return str[:i] + "…"
+		}
+		runeCount++
+	}
+
 	return str
 }
 

+ 62 - 21
internal/template/functions_test.go

@@ -45,33 +45,74 @@ func TestDictWithInvalidMap(t *testing.T) {
 	}
 }
 
-func TestTruncateWithShortTexts(t *testing.T) {
-	scenarios := []string{"Short text", "Короткий текст"}
-
-	for _, input := range scenarios {
-		result := truncate(input, 25)
-		if result != input {
-			t.Fatalf(`Unexpected output, got %q instead of %q`, result, input)
-		}
+func TestTruncate(t *testing.T) {
+	scenarios := []struct {
+		name     string
+		input    string
+		max      int
+		expected string
+	}{
+		{
+			name:     "short ascii",
+			input:    "Short text",
+			max:      25,
+			expected: "Short text",
+		},
+		{
+			name:     "short unicode",
+			input:    "Короткий текст",
+			max:      25,
+			expected: "Короткий текст",
+		},
+		{
+			name:     "exact ascii length",
+			input:    "Short text",
+			max:      len("Short text"),
+			expected: "Short text",
+		},
+		{
+			name:     "long ascii",
+			input:    "This is a really pretty long English text",
+			max:      25,
+			expected: "This is a really pretty l…",
+		},
+		{
+			name:     "long unicode",
+			input:    "Это реально очень длинный русский текст",
+			max:      25,
+			expected: "Это реально очень длинный…",
+		},
+	}
 
-		result = truncate(input, len(input))
-		if result != input {
-			t.Fatalf(`Unexpected output, got %q instead of %q`, result, input)
-		}
+	for _, scenario := range scenarios {
+		t.Run(scenario.name, func(t *testing.T) {
+			result := truncate(scenario.input, scenario.max)
+			if result != scenario.expected {
+				t.Fatalf(`Unexpected output, got %q instead of %q`, result, scenario.expected)
+			}
+		})
 	}
 }
 
-func TestTruncateWithLongTexts(t *testing.T) {
-	scenarios := map[string]string{
-		"This is a really pretty long English text": "This is a really pretty l…",
-		"Это реально очень длинный русский текст":   "Это реально очень длинный…",
+func TestTruncateInvalidMax(t *testing.T) {
+	scenarios := []struct {
+		name string
+		max  int
+	}{
+		{name: "zero", max: 0},
+		{name: "negative", max: -1},
 	}
 
-	for input, expected := range scenarios {
-		result := truncate(input, 25)
-		if result != expected {
-			t.Fatalf(`Unexpected output, got %q instead of %q`, result, expected)
-		}
+	for _, scenario := range scenarios {
+		t.Run(scenario.name, func(t *testing.T) {
+			defer func() {
+				if recover() == nil {
+					t.Fatal("Expected panic for non-positive max")
+				}
+			}()
+
+			_ = truncate("Short text", scenario.max)
+		})
 	}
 }