Bug #13399: IPAddr accepts invalid address mask

• Status: Closed
• Assignee: knu (Akinori MUSHA)
• Target version: -
• ruby -v: ruby 2.4.1p111 (2017-03-22 revision 58053) [x86_64-darwin15]
• Backport: 2.2: UNKNOWN, 2.3: UNKNOWN, 2.4: UNKNOWN

[rtib (Tibor Repasi)]

API Class IPAddr can be initialised with e.g. '1.2.3.4/255.255.255.1', which is an invalid mask for an IPv4 address, however, IPAddr.new won't throw ArgumentError, nor ip.ipv4? will return false.

$ cat iptest.rb
require 'ipaddr'

begin
  ip = IPAddr.new('1.2.3.4/255.255.255.1')
rescue ArgumentError
  puts 'ArgumentError was thrown'
end
puts 'IP address is valid' if ip.ipv4?

$ ruby iptest.rb
IP address is valid

$ ipcalc 1.2.3.4/255.255.255.1
INVALID NETMASK
INVALID MASK1:   255.255.255.1

Address:   1.2.3.4              00000001.00000010.00000011. 00000100
Netmask:   255.255.255.0 = 24   11111111.11111111.11111111. 00000000
Wildcard:  0.0.0.255            00000000.00000000.00000000. 11111111
=>
Network:   1.2.3.0/24           00000001.00000010.00000011. 00000000
HostMin:   1.2.3.1              00000001.00000010.00000011. 00000001
HostMax:   1.2.3.254            00000001.00000010.00000011. 11111110
Broadcast: 1.2.3.255            00000001.00000010.00000011. 11111111
Hosts/Net: 254                   Class A

[nobu (Nobuyoshi Nakada)]

• Description updated (diff)
• Status changed from Open to Assigned
• Assignee set to knu (Akinori MUSHA)

After fixing it, I found that drb/acl depends on this behavior.

diff --git a/lib/ipaddr.rb b/lib/ipaddr.rb
index 6f70ebf773..4f87738be1 100644
--- a/lib/ipaddr.rb
+++ b/lib/ipaddr.rb
@@ -422,6 +422,10 @@
           raise InvalidPrefixError, "address family is not same"
         end
         @mask_addr = m.to_i
+        n = @mask_addr ^ m.instance_variable_get(:@mask_addr)
+        unless ((n + 1) & n).zero?
+          raise InvalidPrefixError, "invalid mask #{mask}"
+        end
         @addr &= @mask_addr
         return self
       end
diff --git a/test/test_ipaddr.rb b/test/test_ipaddr.rb
index 86482a08bd..c49e1ab240 100644
--- a/test/test_ipaddr.rb
+++ b/test/test_ipaddr.rb
@@ -79,6 +79,7 @@
     assert_raise(IPAddr::InvalidPrefixError) { IPAddr.new("::1/255.255.255.0") }
     assert_raise(IPAddr::InvalidPrefixError) { IPAddr.new("::1/129") }
     assert_raise(IPAddr::InvalidPrefixError) { IPAddr.new("192.168.0.1/33") }
+    assert_raise(IPAddr::InvalidPrefixError) { IPAddr.new("192.168.0.1/255.255.255.1") }
     assert_raise(IPAddr::AddressFamilyError) { IPAddr.new(1) }
     assert_raise(IPAddr::AddressFamilyError) { IPAddr.new("::ffff:192.168.1.2/120", Socket::AF_INET) }
   end
@@ -234,7 +235,14 @@
   def test_mask
     a = @a.mask(32)
     assert_equal("3ffe:505::", a.to_s)
+    assert_equal("3ffe:505::", @a.mask("ffff:ffff::").to_s)
     assert_equal("3ffe:505:2::", @a.to_s)
+    a = IPAddr.new("192.168.2.0/24")
+    assert_equal("192.168.0.0", a.mask(16).to_s)
+    assert_equal("192.168.0.0", a.mask("255.255.0.0").to_s)
+    assert_equal("192.168.2.0", a.to_s)
+    assert_raise(IPAddr::InvalidPrefixError) {a.mask("255.255.0.255")}
+    assert_raise(IPAddr::InvalidPrefixError) {@a.mask("ffff:1::")}
   end
 
   def test_include?

[rtib (Tibor Repasi)]

I've just applied the patch locally. It works as expected.

Thank you.

[knu (Akinori MUSHA)]

nobu (Nobuyoshi Nakada) wrote:

After fixing it, I found that drb/acl depends on this behavior.

Thanks for the patch, but what do you mean by drb/acl depending on this behavior?

[knu (Akinori MUSHA)]

Now I see!

% RBENV_VERSION=ruby_2_5 ruby -Itest/lib:lib test/drb/test_acl.rb
Run options:

# Running tests:

[ 2/12] DRbTests::ACLEntryTest#test_ip = 0.00 s
  1) Failure:
DRbTests::ACLEntryTest#test_ip [test/drb/test_acl.rb:72]:
Expected #<ACL::ACLEntry:0x00007fb2d8842e60
 @pat=[:name, /\A192\.168\.0\.1\/255\.255\.0\.255\z/]> to be match ["AF_INET", 10000, "x68k.linux.or.jp", "192.168.1.1"].

[ 5/12] DRbTests::ACLListTest#test_1 = 0.00 s
  2) Failure:
DRbTests::ACLListTest#test_1 [test/drb/test_acl.rb:141]:
Expected #<ACL::ACLList:0x00007fb2d88cba08
 @list=
  [#<ACL::ACLEntry:0x00007fb2d88cb828
    @pat=[:name, /\A192\.0\.0\.1\/255\.0\.0\.255\z/]>,
   #<ACL::ACLEntry:0x00007fb2d88d1818 @pat=[:name, /\Ayum\..+\.jp\z/]>]> to be match ["AF_INET", 10000, "x68k.linux.or.jp", "192.168.1.1"].

Finished tests in 0.009075s, 1322.3140 tests/s, 8925.6198 assertions/s.
12 tests, 81 assertions, 2 failures, 0 errors, 0 skips

ruby -v: ruby 2.5.0dev (2017-10-21 trunk 60228) [x86_64-darwin17]

[knu (Akinori MUSHA)]

• Status changed from Assigned to Closed

Applied in changeset trunk|r60270.

---

Import ipaddr 1.2.0

• Add IPAddr#prefix
• Add IPAddr#loopback?
• Add IPAddr#private? [Feature #11666]
• Add IPAddr#link_local? [Feature #10912]
• Reject invalid address mask [Bug #13399]
• Warn that IPAddr#ipv4_compat and #ipv4_compat? are deprecated [#Bug 13769]