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.
This commit is contained in:
binwiederhier committed 2026-11-02 01:13:18 +00:00
1 parent 093012dc8d
commit 9ccddc7b10
5 files changed
+24

No files matched your search

+7
View File
@@ -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)
+2
View File
@@ -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) {
+2
View File
@@ -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) {
+2
View File
@@ -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)) {
+11
View File
@@ -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")
}