Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 [Bug]: utils.IsIPv4 and net.ParseIP have inconsistent results #2735

Closed
3 tasks done
itswcg opened this issue Nov 21, 2023 · 5 comments · Fixed by #2736
Closed
3 tasks done

🐛 [Bug]: utils.IsIPv4 and net.ParseIP have inconsistent results #2735

itswcg opened this issue Nov 21, 2023 · 5 comments · Fixed by #2736

Comments

@itswcg
Copy link
Contributor

itswcg commented Nov 21, 2023

Bug Description

if config EnableIPValidation set true, The correct IP is filtered.

cause:
utils.IsIPv4 and net.ParseIP have inconsistent results

utils.IsIPv4("123.117.56.255") // return fasle

ip := net.ParseIP("123.117.56.255")
ip.To4() != nil // return true

How to Reproduce

IP address ending with .255, utils.IsIPv4 always returns false.

Expected Behavior

utils.IsIPv4("123.117.56.255") return true

Fiber Version

v2.49.2

Code Snippet (optional)

app := fiber.New(fiber.Config{
	EnableIPValidation: true,
})

ips = ctx.IPs()

Checklist:

  • I agree to follow Fiber's Code of Conduct.
  • I have checked for existing issues that describe my problem prior to opening this one.
  • I understand that improperly formatted bug reports may be closed without explanation.
Copy link

welcome bot commented Nov 21, 2023

Thanks for opening your first issue here! 🎉 Be sure to follow the issue template! If you need help or want to chat with us, join us on Discord https://gofiber.io/discord

@ReneWerner87
Copy link
Member

fiber/utils/ips.go

Lines 9 to 39 in 2374cad

func IsIPv4(s string) bool {
for i := 0; i < net.IPv4len; i++ {
if len(s) == 0 {
return false
}
if i > 0 {
if s[0] != '.' {
return false
}
s = s[1:]
}
n, ci := 0, 0
for ci = 0; ci < len(s) && '0' <= s[ci] && s[ci] <= '9'; ci++ {
n = n*10 + int(s[ci]-'0')
if n >= 0xFF {
return false
}
}
if ci == 0 || (ci > 1 && s[0] == '0') {
return false
}
s = s[ci:]
}
return len(s) == 0
}

func IsIPv4(s string) bool {
    for i := 0; i < net.IPv4len; i++ {
        if len(s) == 0 {
            return false
        }

        if i > 0 {
            if s[0] != '.' {
                return false
            }
            s = s[1:]
        }

        n, ci := 0, 0

        for ci = 0; ci < len(s) && '0' <= s[ci] && s[ci] <= '9'; ci++ {
            n = n*10 + int(s[ci]-'0')
            if n > 0xFF { // change
                return false
            }
        }

        if ci == 0 || (ci > 1 && s[0] == '0') {
            return false
        }

        s = s[ci:]
    }

    return len(s) == 0
}

could you test this ?

@ReneWerner87
Copy link
Member

0xFF is 255

@itswcg
Copy link
Contributor Author

itswcg commented Nov 21, 2023

fiber/utils/ips.go

Lines 9 to 39 in 2374cad

func IsIPv4(s string) bool {
for i := 0; i < net.IPv4len; i++ {
if len(s) == 0 {
return false
}
if i > 0 {
if s[0] != '.' {
return false
}
s = s[1:]
}
n, ci := 0, 0
for ci = 0; ci < len(s) && '0' <= s[ci] && s[ci] <= '9'; ci++ {
n = n*10 + int(s[ci]-'0')
if n >= 0xFF {
return false
}
}
if ci == 0 || (ci > 1 && s[0] == '0') {
return false
}
s = s[ci:]
}
return len(s) == 0
}

func IsIPv4(s string) bool {
    for i := 0; i < net.IPv4len; i++ {
        if len(s) == 0 {
            return false
        }

        if i > 0 {
            if s[0] != '.' {
                return false
            }
            s = s[1:]
        }

        n, ci := 0, 0

        for ci = 0; ci < len(s) && '0' <= s[ci] && s[ci] <= '9'; ci++ {
            n = n*10 + int(s[ci]-'0')
            if n > 0xFF { // change
                return false
            }
        }

        if ci == 0 || (ci > 1 && s[0] == '0') {
            return false
        }

        s = s[ci:]
    }

    return len(s) == 0
}

could you test this ?

it was passed.

utils.IsIPv4("123.117.56.255") // return true

@ReneWerner87
Copy link
Member

ReneWerner87 commented Nov 21, 2023

ok, can you create a pull request that contains the fix and other unittest cases or should I do that?
https://github.com/gofiber/fiber/blob/master/utils/ips_test.go#L15-L32

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants