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

/32 subnet mask erroneously matches all IP addresses #43360

Closed
supriyo-biswas opened this issue Jun 9, 2022 · 8 comments · Fixed by #43381
Closed

/32 subnet mask erroneously matches all IP addresses #43360

supriyo-biswas opened this issue Jun 9, 2022 · 8 comments · Fixed by #43381
Labels
confirmed-bug Issues with confirmed bugs. good first issue Issues that are suitable for first-time contributors.

Comments

@supriyo-biswas
Copy link
Contributor

supriyo-biswas commented Jun 9, 2022

Version

v18.3.0

Platform

Darwin xxx 21.5.0 Darwin Kernel Version 21.5.0: Tue Apr 26 21:08:22 PDT 2022; root:xnu-8020.121.3~4/RELEASE_X86_64 x86_64

Subsystem

net

What steps will reproduce the bug?

A /32 subnet mask such as 1.1.1.1/32 should match only the IP address 1.1.1.1 as all 32 bits should be compared. However, if a /32 subnet mask is used with net.Blocklist(), it erroneously matches all IP addresses.

The issue can be reproduced by running the following code snippet:

const net = require('net')

const reservedIPBlocklist = new net.BlockList()
reservedIPBlocklist.addSubnet('1.1.1.1', 32, 'ipv4')

for (const ip of ["10.0.0.1", "1.1.1.1", "4.3.2.4", "27.192.11.1", 
"192.168.1.9"]) {
    console.log(ip, reservedIPBlocklist.check(ip, 'ipv4'))    
}

Even though only 1.1.1.1 should match, true is printed for all the IPs:

10.0.0.1 true
1.1.1.1 true
4.3.2.4 true
27.192.11.1 true
192.168.1.9 true

If the subnet mask is changed to < 32, Node behaves correctly.

How often does it reproduce? Is there a required condition?

The bug can always be reproduced.

What is the expected behavior?

See above.

What do you see instead?

See above.

Additional information

This bug is present across other versions of Node too, for example see https://www.mycompiler.io/view/1JuZ56bnWhl where the Node version is Node 16.15.0.

@Trott
Copy link
Member

Trott commented Jun 9, 2022

@nodejs/net

@mcollina mcollina added confirmed-bug Issues with confirmed bugs. good first issue Issues that are suitable for first-time contributors. labels Jun 9, 2022
@mcollina
Copy link
Member

mcollina commented Jun 9, 2022

@supriyo-biswas would you like to send a PR to address this?

@supriyo-biswas
Copy link
Contributor Author

@mcollina yes, I will. If I have questions, is it fine to ask in this issue itself?

@ShogunPanda
Copy link
Contributor

@supriyo-biswas Yes, go for it! ^^

supriyo-biswas added a commit to supriyo-biswas/node that referenced this issue Jun 11, 2022
supriyo-biswas added a commit to supriyo-biswas/node that referenced this issue Jun 11, 2022
supriyo-biswas added a commit to supriyo-biswas/node that referenced this issue Jun 11, 2022
supriyo-biswas added a commit to supriyo-biswas/node that referenced this issue Jun 11, 2022
supriyo-biswas added a commit to supriyo-biswas/node that referenced this issue Jun 11, 2022
supriyo-biswas added a commit to supriyo-biswas/node that referenced this issue Jun 15, 2022
@JONATHANTSIFERANA

This comment was marked as spam.

@Adambenmabrok

This comment was marked as spam.

@Trott

This comment was marked as resolved.

@Trott

This comment was marked as resolved.

nodejs-github-bot pushed a commit that referenced this issue Jun 25, 2022
Fixes: #43360

PR-URL: #43381
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos pushed a commit that referenced this issue Jul 12, 2022
Fixes: #43360

PR-URL: #43381
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos pushed a commit that referenced this issue Jul 20, 2022
Fixes: #43360

PR-URL: #43381
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos pushed a commit that referenced this issue Jul 31, 2022
Fixes: #43360

PR-URL: #43381
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
guangwong pushed a commit to noslate-project/node that referenced this issue Oct 10, 2022
Fixes: nodejs/node#43360

PR-URL: nodejs/node#43381
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. good first issue Issues that are suitable for first-time contributors.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants