Reject negative manual ban durations
This commit is contained in:
committed by
Brandon McGinty
parent
0e9e495a8f
commit
51d62c3acb
@@ -880,17 +880,30 @@ func (b *Barnard) addManualBan(text string) {
|
|||||||
b.AddOutputLine("Admin: ban address must be CIDR, for example 192.0.2.1/32")
|
b.AddOutputLine("Admin: ban address must be CIDR, for example 192.0.2.1/32")
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
minutes, err := strconv.Atoi(parts[1])
|
duration, err := manualBanDuration(parts[1])
|
||||||
if err != nil {
|
if err != nil {
|
||||||
b.AddOutputLine("Admin: ban minutes must be a number")
|
b.AddOutputLine("Admin: " + err.Error())
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
reason := strings.Join(parts[2:], " ")
|
reason := strings.Join(parts[2:], " ")
|
||||||
b.adminBanList.Add(ip, network.Mask, reason, time.Duration(minutes)*time.Minute)
|
b.adminBanList.Add(ip, network.Mask, reason, duration)
|
||||||
b.Client.Send(b.adminBanList)
|
b.Client.Send(b.adminBanList)
|
||||||
b.AddOutputLine("Admin: manual ban sent")
|
b.AddOutputLine("Admin: manual ban sent")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// manualBanDuration validates user input before it reaches the unsigned
|
||||||
|
// protocol duration field.
|
||||||
|
func manualBanDuration(text string) (time.Duration, error) {
|
||||||
|
minutes, err := strconv.Atoi(text)
|
||||||
|
if err != nil {
|
||||||
|
return 0, fmt.Errorf("ban minutes must be a number")
|
||||||
|
}
|
||||||
|
if minutes < 0 {
|
||||||
|
return 0, fmt.Errorf("ban minutes must not be negative")
|
||||||
|
}
|
||||||
|
return time.Duration(minutes) * time.Minute, nil
|
||||||
|
}
|
||||||
|
|
||||||
func (b *Barnard) unbanIndex(index int) {
|
func (b *Barnard) unbanIndex(index int) {
|
||||||
if index < 0 || index >= len(b.adminBanList) {
|
if index < 0 || index >= len(b.adminBanList) {
|
||||||
b.AddOutputLine("Admin: ban index out of range")
|
b.AddOutputLine("Admin: ban index out of range")
|
||||||
|
|||||||
@@ -65,6 +65,18 @@ func TestPermissionList(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Regression: negative manual-ban minutes were cast to an unsigned protocol
|
||||||
|
// duration, turning a rejected short ban into an extremely long one.
|
||||||
|
func TestManualBanDurationRejectsNegativeMinutes(t *testing.T) {
|
||||||
|
if _, err := manualBanDuration("-1"); err == nil {
|
||||||
|
t.Fatal("negative duration was accepted")
|
||||||
|
}
|
||||||
|
got, err := manualBanDuration("15")
|
||||||
|
if err != nil || got != 15*60*1000000000 {
|
||||||
|
t.Fatalf("got %v, %v", got, err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestAdminEscapeInputs(t *testing.T) {
|
func TestAdminEscapeInputs(t *testing.T) {
|
||||||
if !isAdminEscapeKey(uiterm.KeyEsc) {
|
if !isAdminEscapeKey(uiterm.KeyEsc) {
|
||||||
t.Fatal("expected escape key to close admin menu")
|
t.Fatal("expected escape key to close admin menu")
|
||||||
|
|||||||
@@ -266,7 +266,7 @@ Priority 3: configuration, UI, and binding hardening
|
|||||||
Both helpers panic on exec failure. Return/log an error or remove unused
|
Both helpers panic on exec failure. Return/log an error or remove unused
|
||||||
helpers; a missing optional beep binary must not terminate Barnard.
|
helpers; a missing optional beep binary must not terminate Barnard.
|
||||||
|
|
||||||
35. Admin manual-ban duration accepts negative values
|
[x] 35. Admin manual-ban duration accepts negative values
|
||||||
Files: admin.go, gumble/gumble/bans.go
|
Files: admin.go, gumble/gumble/bans.go
|
||||||
Negative minutes become a negative duration, then are cast to uint32
|
Negative minutes become a negative duration, then are cast to uint32
|
||||||
seconds for the protocol, creating a huge ban duration. Reject negative
|
seconds for the protocol, creating a huge ban duration. Reject negative
|
||||||
|
|||||||
Reference in New Issue
Block a user