Project

General

Profile

Bug #13399

IPAddr accepts invalid address mask

Added by rtib (Tibor Repasi) almost 2 years ago. Updated over 1 year ago.

Status:
Closed
Priority:
Normal
Target version:
-
ruby -v:
ruby 2.4.1p111 (2017-03-22 revision 58053) [x86_64-darwin15]
[ruby-core:80556]

Description

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

Associated revisions

Revision c2db917b
Added by knu (Akinori MUSHA) over 1 year ago

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]

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@60270 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Revision 60270
Added by knu (Akinori MUSHA) over 1 year ago

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]

Revision 60270
Added by knu (Akinori MUSHA) over 1 year ago

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]

Revision 60270
Added by knu (Akinori MUSHA) over 1 year ago

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]

History

Updated by nobu (Nobuyoshi Nakada) almost 2 years ago

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

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?

Updated by rtib (Tibor Repasi) almost 2 years ago

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

Thank you.

Updated by knu (Akinori MUSHA) over 1 year ago

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?

Updated by knu (Akinori MUSHA) over 1 year ago

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]
#5

Updated by knu (Akinori MUSHA) over 1 year ago

  • 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]

Also available in: Atom PDF