Skip to content

Commit

Permalink
fix IPv6Prefix bug
Browse files Browse the repository at this point in the history
  • Loading branch information
Tim Cooper committed Dec 13, 2023
1 parent 6579be8 commit 1006025
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 5 deletions.
13 changes: 8 additions & 5 deletions attribute.go
Original file line number Diff line number Diff line change
Expand Up @@ -467,18 +467,21 @@ func IPv6Prefix(a Attribute) (*net.IPNet, error) {
}

prefixLength := int(a[1])
if (len(a)-2)*8 < prefixLength {
if prefixLength > net.IPv6len*8 {
return nil, errors.New("invalid prefix length")
}

ip := make(net.IP, net.IPv6len)
copy(ip, a[2:])

// clear final non-mask bits
if i := uint(prefixLength % 8); i != 0 {
for ; i < 8; i++ {
ip[prefixLength/8] &^= 1 << (7 - i)
bit := uint(prefixLength % 8)
for octet := prefixLength / 8; octet < len(ip); octet++ {

This comment has been minimized.

Copy link
@dav-komarek

dav-komarek Dec 13, 2023

I don't think that this will work correctly. Let's have following IPv6 with prefix: 2001:1530:100e::/47.

  • bit value will be 7
  • octet (in the first iteration) will be 5
  • ip[octet]&(1<<(7-bit)) == ip[5]&(1<<(7-7)) == 0xe&1 != 0
  • This results in error, but it shouldn't. The provided IPv6 is valid.

From my point of view the original code was almost correct. You just need to iterate over rest of bytes to zero them.

Please correct me if I'm wrong.

This comment has been minimized.

Copy link
@dav-komarek

dav-komarek Dec 13, 2023

Just a question. Is it even required to zero all masked bits?

for ; bit < 8; bit++ {
if ip[octet]&(1<<(7-bit)) != 0 {
return nil, errors.New("invalid prefix data")
}
}
bit = 0
}

return &net.IPNet{
Expand Down
13 changes: 13 additions & 0 deletions attribute_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,19 @@ func TestIPv6Prefix(t *testing.T) {
}
}

func TestIPv6Prefix_issue118(t *testing.T) {
ipNet, err := IPv6Prefix([]byte{0x00, 0x40, 0x20, 0x01, 0x15, 0x30, 0x10, 0x0e})
if err != nil {
t.Fatalf("unexpected error: %s", err)
}
if expected := net.ParseIP("2001:1530:100e::"); !ipNet.IP.Equal(expected) {
t.Fatalf("got %v, expected %v", ipNet.IP, expected)
}
if ones, size := ipNet.Mask.Size(); ones != 64 || size != 128 {
t.Fatalf("got %v:%v, expected 64, 128", ones, size)
}
}

func ipNetEquals(a, b *net.IPNet) bool {
if a == nil && b == nil {
return true
Expand Down

0 comments on commit 1006025

Please sign in to comment.