Skip to content

Commit

Permalink
Fix: Refactor network parameter to pointer in brute force checks
Browse files Browse the repository at this point in the history
Modified the `network` parameter to a double pointer in `checkBucketOverLimit` and `checkRepeatingBruteForcer` functions to ensure consistency and correct parameter passing. This change helps in properly utilizing references within these functions, avoiding issues related to network object manipulation.

Signed-off-by: Christian Roessner <c@roessner.co>
  • Loading branch information
Christian Roessner committed Oct 25, 2024
1 parent 9b849c3 commit 53c9da9
Showing 1 changed file with 8 additions and 11 deletions.
19 changes: 8 additions & 11 deletions server/core/bruteforce.go
Original file line number Diff line number Diff line change
Expand Up @@ -659,12 +659,12 @@ func logBucketMatchingRule(auth *AuthState, network *net.IPNet, rule *config.Bru

// checkBucketOverLimit checks if the given network exceeds the brute force allowed thresholds based on predefined rules.
// It verifies the current network against the specified brute force rules and returns if any rule is triggered.
func checkBucketOverLimit(auth *AuthState, rules []config.BruteForceRule, network *net.IPNet, message *string) (withError bool, ruleTriggered bool, ruleNumber int) {
func checkBucketOverLimit(auth *AuthState, rules []config.BruteForceRule, network **net.IPNet, message *string) (withError bool, ruleTriggered bool, ruleNumber int) {
var err error

for ruleNumber = range rules {
// Skip, where the current IP address does not match the current rule
if network, err = auth.getNetwork(&rules[ruleNumber]); err != nil {
if *network, err = auth.getNetwork(&rules[ruleNumber]); err != nil {
level.Error(log.Logger).Log(global.LogKeyGUID, auth.GUID, global.LogKeyError, err)

return true, false, ruleNumber
Expand Down Expand Up @@ -795,14 +795,14 @@ func processBruteForce(auth *AuthState, ruleTriggered, alreadyTriggered bool, ru

// checkRepeatingBruteForcer analyzes if a network partakes in repeated brute force attempts according to specified rules.
// It returns a boolean indicating an error, whether a brute force rule already triggered, and the rule number.
func checkRepeatingBruteForcer(auth *AuthState, rules []config.BruteForceRule, network *net.IPNet, message *string) (withError bool, alreadyTriggered bool, ruleNumber int) {
func checkRepeatingBruteForcer(auth *AuthState, rules []config.BruteForceRule, network **net.IPNet, message *string) (withError bool, alreadyTriggered bool, ruleNumber int) {
var (
ruleName string
err error
)

for ruleNumber = range rules {
if network, err = auth.getNetwork(&rules[ruleNumber]); err != nil {
if *network, err = auth.getNetwork(&rules[ruleNumber]); err != nil {
level.Error(log.Logger).Log(global.LogKeyGUID, auth.GUID, global.LogKeyError, err)

return true, false, ruleNumber
Expand All @@ -811,10 +811,6 @@ func checkRepeatingBruteForcer(auth *AuthState, rules []config.BruteForceRule, n
}

if ruleName, err = auth.getPreResultBruteForceRedis(&rules[ruleNumber]); ruleName != "" && err == nil {
if _, network, err = net.ParseCIDR(fmt.Sprintf("%s/%d", auth.ClientIP, rules[ruleNumber].CIDR)); err != nil {
withError = true
}

alreadyTriggered = true
*message = "Brute force attack detected (cached result)"
stats.BruteForceRejected.WithLabelValues(ruleName).Inc()
Expand Down Expand Up @@ -847,7 +843,6 @@ func (a *AuthState) checkBruteForce() (blockClientIP bool) {
var (
ruleTriggered bool
message string
network *net.IPNet
)

if a.NoAuth || a.ListAccounts {
Expand Down Expand Up @@ -906,13 +901,15 @@ func (a *AuthState) checkBruteForce() (blockClientIP bool) {
return false
}

abort, alreadyTriggered, ruleNumber := checkRepeatingBruteForcer(a, rules, network, &message)
network := &net.IPNet{}

abort, alreadyTriggered, ruleNumber := checkRepeatingBruteForcer(a, rules, &network, &message)
if abort {
return false
}

if !alreadyTriggered {
abort, ruleTriggered, ruleNumber = checkBucketOverLimit(a, rules, network, &message)
abort, ruleTriggered, ruleNumber = checkBucketOverLimit(a, rules, &network, &message)
if abort {
return false
}
Expand Down

0 comments on commit 53c9da9

Please sign in to comment.