From 9ccddc7b10e202c46471a5ec12d947b5d77561ca Mon Sep 17 00:00:00 2001 From: binwiederhier Date: Mon, 2 Nov 2026 01:13:18 +0000 Subject: [PATCH] Cap ban weights and the threshold, so counting in thousandths cannot overflow A weight like 1e16 overflowed to a negative token count, which the limiter always allows, so the code never banned. Weights above 1,000,000 and thresholds above 1,000,000,000 are now rejected at startup. --- ban/service.go | 7 +++++++ ban/weights.go | 2 ++ ban/weights_test.go | 2 ++ cmd/serve.go | 2 ++ cmd/serve_test.go | 11 +++++++++++ 5 files changed, 24 insertions(+) diff --git a/ban/service.go b/ban/service.go index fd97e9e2..ae0c7a9e 100644 --- a/ban/service.go +++ b/ban/service.go @@ -25,6 +25,13 @@ const ( strikeUnits = 1000 ) +// Strikes are counted in thousandths of a strike, so these bounds keep the counts far from +// overflowing; any weight above the threshold bans on the first hit anyway +const ( + MaxWeight = 1_000_000 + MaxThreshold = 1_000_000_000 +) + // Config is the Service's config, kept separate from server.Config to avoid an import cycle. type Config struct { File string // Ban file that fail2ban tails (must be non-empty; the caller decides whether the feature is enabled) diff --git a/ban/weights.go b/ban/weights.go index f587a5f0..4b8e4a0a 100644 --- a/ban/weights.go +++ b/ban/weights.go @@ -25,6 +25,8 @@ func ParseWeights(entries []string) (Weights, error) { return nil, fmt.Errorf("invalid ban-weight value in %q, want a non-negative number", entry) } else if weight > 0 && weight < 1.0/strikeUnits { return nil, fmt.Errorf("invalid ban-weight value in %q, want 0 or at least %g", entry, 1.0/strikeUnits) + } else if weight > MaxWeight { + return nil, fmt.Errorf("invalid ban-weight value in %q, want at most %d", entry, MaxWeight) } key = strings.TrimSpace(key) if !validWeightKey(key) { diff --git a/ban/weights_test.go b/ban/weights_test.go index 5094cb3a..81926a30 100644 --- a/ban/weights_test.go +++ b/ban/weights_test.go @@ -49,6 +49,8 @@ func TestParseWeights_Fractional(t *testing.T) { require.Equal(t, 0.001, weights.WeightFor(50701)) _, err = ParseWeights([]string{"50701:0.0001"}) // Below the 0.001 resolution require.Error(t, err) + _, err = ParseWeights([]string{"42909:1e16"}) // Would overflow once counted in thousandths + require.Error(t, err) } func TestWeights_WeightFor(t *testing.T) { diff --git a/cmd/serve.go b/cmd/serve.go index 8ad0107c..b2c0338e 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -439,6 +439,8 @@ func execServe(c *cli.Context) error { return errors.New("if ban-file is set, ban-window must be greater than zero") } else if banFile != "" && banThreshold <= 0 { return errors.New("if ban-file is set, ban-threshold must be greater than zero") + } else if banFile != "" && banThreshold > ban.MaxThreshold { + return fmt.Errorf("if ban-file is set, ban-threshold must be at most %d", ban.MaxThreshold) } else if banFile != "" && len(banWeights) == 0 { return errors.New("if ban-file is set, ban-weights must not be empty") } else if banFile != "" && !util.FileExists(filepath.Dir(banFile)) { diff --git a/cmd/serve_test.go b/cmd/serve_test.go index b116332e..869130c8 100644 --- a/cmd/serve_test.go +++ b/cmd/serve_test.go @@ -741,3 +741,14 @@ func newEmptyFile(t *testing.T) string { require.Nil(t, os.WriteFile(filename, []byte{}, 0600)) return filename } + +func TestCLI_Serve_BanThresholdTooLarge(t *testing.T) { + // Strikes are counted in thousandths, so a huge threshold would overflow and never ban; an + // invalid auth-default-access fails right after the checks, so the server never starts + configFile := newEmptyFile(t) + app, _, _, _ := newTestApp() + banFile := filepath.Join(t.TempDir(), "ban.log") + err := app.Run([]string{"ntfy", "serve", "--config=" + configFile, "--ban-file=" + banFile, "--ban-threshold=10000000000000000", "--auth-default-access=invalid"}) + require.Error(t, err) + require.Contains(t, err.Error(), "ban-threshold") +}