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

BlockList addRange appears to match addresses incorrectly #39074

Closed
EricHripko opened this issue Jun 18, 2021 · 4 comments
Closed

BlockList addRange appears to match addresses incorrectly #39074

EricHripko opened this issue Jun 18, 2021 · 4 comments
Labels
net Issues and PRs related to the net subsystem.

Comments

@EricHripko
Copy link

  • Version: v16.3.0
  • Platform: Darwin <redacted> 20.4.0 Darwin Kernel Version 20.4.0: Thu Apr 22 21:46:41 PDT 2021; root:xnu-7195.101.2~1/RELEASE_ARM64_T8101 arm64
  • Subsystem: net

What steps will reproduce the bug?

I've used the following code snippet from Node.JS Net docs to try to block IPs based on ranges:

> const blockList = new net.BlockList();
undefined
> blockList.addRange('10.0.0.1', '10.0.0.10');
undefined
> blockList.check('10.0.0.3')
true
> blockList.check('192.168.0.3')
true

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

Always reproduces.

What is the expected behavior?

I expect the block list with above-mentioned configuration to block 10.0.0.3 but allow 192.168.0.3 (i.e., check to return false).

What do you see instead?

It looks as if BlockList matches only the last element of the IP address. As a result, check returns true for 192.168.0.3 even though it's not in the range:

> blockList.rules
[ 'Range: IPv4 10.0.0.1-10.0.0.10' ]

Additional information

addAddress and (more importantly) addSubnet appears to work fine:

> const blockList = new net.BlockList(); 
undefined
> blockList.addSubnet("10.0.0.0", 29)  # 10.0.0.0/29 = 10.0.0.0-10.0.0.7
undefined
> blockList.check('10.0.0.3')
true
> blockList.check('192.168.0.3')
false
@targos
Copy link
Member

targos commented Jun 18, 2021

@jasnell

@targos targos added the net Issues and PRs related to the net subsystem. label Jun 18, 2021
@EladKeyshawn
Copy link

@targos Is this a relevant bug ?

@jasnell
Copy link
Member

jasnell commented Jun 18, 2021

Will investigate!

@EricHripko
Copy link
Author

Thank you for looking into this 👍

targos pushed a commit that referenced this issue Jul 11, 2021
This commit updates compare_ipv4() to use the host byte
order.

PR-URL: #39096
Fixes: #39074
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos pushed a commit that referenced this issue Aug 8, 2021
This commit updates compare_ipv4() to use the host byte
order.

PR-URL: #39096
Fixes: #39074
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
BethGriggs pushed a commit that referenced this issue Aug 12, 2021
This commit updates compare_ipv4() to use the host byte
order.

PR-URL: #39096
Fixes: #39074
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
MylesBorins pushed a commit that referenced this issue Aug 31, 2021
This commit updates compare_ipv4() to use the host byte
order.

PR-URL: #39096
Fixes: #39074
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
foxxyz pushed a commit to foxxyz/node that referenced this issue Oct 18, 2021
This commit updates compare_ipv4() to use the host byte
order.

PR-URL: nodejs#39096
Fixes: nodejs#39074
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
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
net Issues and PRs related to the net subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants