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

fix: use Node.js net lib and reject malformed addresses #1

Merged
merged 4 commits into from
Jun 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
name: Release

on:
push:
branches: [ master ]

jobs:
release:
name: Node.js
uses: eggjs/github-actions/.github/workflows/node-release.yml@master
secrets:
NPM_TOKEN: ${{ secrets.NPM_TOKEN }}
GIT_TOKEN: ${{ secrets.GIT_TOKEN }}
8 changes: 4 additions & 4 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
node: [ 12, 14, 16, 18 ]
node: [ 14, 16, 18, 20, 22 ]
name: Node ${{ matrix.node }} sample
steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4
- name: Setup node
uses: actions/setup-node@v3
uses: actions/setup-node@v4
with:
node-version: ${{ matrix.node }}
- run: npm ci
- run: npm install
- run: npm test
30 changes: 20 additions & 10 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,22 +1,21 @@
# IP
[![](https://badge.fury.io/js/ip.svg)](https://www.npmjs.com/package/ip)
# IP

[![](https://badge.fury.io/js/@eggjs/ip.svg)](https://www.npmjs.com/package/@eggjs/ip)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add alt text to the image for accessibility.

- [![](https://badge.fury.io/js/@eggjs/ip.svg)](https://www.npmjs.com/package/@eggjs/ip)
+ [![npm package version](https://badge.fury.io/js/@eggjs/ip.svg)](https://www.npmjs.com/package/@eggjs/ip)
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
[![](https://badge.fury.io/js/@eggjs/ip.svg)](https://www.npmjs.com/package/@eggjs/ip)
[![npm package version](https://badge.fury.io/js/@eggjs/ip.svg)](https://www.npmjs.com/package/@eggjs/ip)
Tools
Markdownlint

3-3: null (MD045, no-alt-text)
Images should have alternate text (alt text)


IP address utilities for node.js

## Installation
Security fix fork, merge https://github.com/indutny/node-ip/pull/144
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using a markdown link instead of a bare URL to comply with best practices.

- Security fix fork, merge https://github.com/indutny/node-ip/pull/144
+ Security fix fork, merge [node-ip pull request #144](https://github.com/indutny/node-ip/pull/144)
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Security fix fork, merge https://github.com/indutny/node-ip/pull/144
Security fix fork, merge [node-ip pull request #144](https://github.com/indutny/node-ip/pull/144)
Tools
Markdownlint

7-7: null (MD034, no-bare-urls)
Bare URL used


### npm
```shell
npm install ip
```
## Installation

### git
### npm

```shell
git clone https://github.com/indutny/node-ip.git
npm install ip
```

## Usage

Get your ip address, compare ip addresses, validate ip addresses, etc.

```js
Expand All @@ -34,6 +33,7 @@ ip.or('192.168.1.134', '0.0.0.255') // 192.168.1.255
ip.isPrivate('127.0.0.1') // true
ip.isV4Format('127.0.0.1'); // true
ip.isV6Format('::ffff:127.0.0.1'); // true
ip.isValid('127.0.0.1'); // true

// operate on buffers in-place
var buf = new Buffer(128);
Expand All @@ -58,16 +58,25 @@ ip.cidrSubnet('192.168.1.134/26')
// range checking
ip.cidrSubnet('192.168.1.134/26').contains('192.168.1.190') // true


// ipv4 long conversion
ip.toLong('127.0.0.1'); // 2130706433
ip.fromLong(2130706433); // '127.0.0.1'

// malformed addresses and normalization
ip.normalizeStrict('0::01'); // '::1'
ip.isPrivate('0x7f.1'); // throw error
ip.isValidAndPrivate('0x7f.1'); // false
ip.normalizeStrict('0x7f.1'); // throw error
var normalized = ip.normalizeLax('0x7f.1'); // 127.0.0.1
ip.isPrivate(normalized); // true
```

### License

```txt
This software is licensed under the MIT License.
Copyright (c) 2024-present eggjs and other contributors.
Copyright Fedor Indutny, 2012.
Permission is hereby granted, free of charge, to any person obtaining a
Expand All @@ -88,3 +97,4 @@ NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
USE OR OTHER DEALINGS IN THE SOFTWARE.
```
88 changes: 57 additions & 31 deletions lib/ip.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const ip = exports;
const { Buffer } = require('buffer');
const os = require('os');
const net = require('net');

ip.toBuffer = function (ip, buff, offset) {
offset = ~~offset;
Expand Down Expand Up @@ -82,15 +83,28 @@ ip.toString = function (buff, offset, length) {
return result;
};

const ipv4Regex = /^(\d{1,3}\.){3,3}\d{1,3}$/;
const ipv6Regex = /^(::)?(((\d{1,3}\.){3}(\d{1,3}){1})?([0-9a-f]){0,4}:{0,2}){1,8}(::)?$/i;
ip.isV4Format = net.isIPv4;

ip.isV4Format = function (ip) {
return ipv4Regex.test(ip);
ip.isV6Format = net.isIPv6;

ip.isValid = function (addr) {
return net.isIP(addr) !== 0;
};

ip.isV6Format = function (ip) {
return ipv6Regex.test(ip);
ip.normalizeStrict = function (addr) {
let family;
switch (net.isIP(addr)) {
case 4:
family = 'ipv4';
break;
case 6:
family = 'ipv6';
break;
default:
throw new Error(`Invalid ip address: ${addr}`);
}
const { address } = new net.SocketAddress({ address: addr, family });
return address;
Comment on lines +90 to +107
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ip.isValid and ip.normalizeStrict functions are well implemented using the net library. Consider converting these to arrow functions for consistency and to reduce function complexity.

- ip.isValid = function (addr) {
+ ip.isValid = (addr) => {
- ip.normalizeStrict = function (addr) {
+ ip.normalizeStrict = (addr) => {
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
ip.isValid = function (addr) {
return net.isIP(addr) !== 0;
};
ip.isV6Format = function (ip) {
return ipv6Regex.test(ip);
ip.normalizeStrict = function (addr) {
let family;
switch (net.isIP(addr)) {
case 4:
family = 'ipv4';
break;
case 6:
family = 'ipv6';
break;
default:
throw new Error(`Invalid ip address: ${addr}`);
}
const { address } = new net.SocketAddress({ address: addr, family });
return address;
ip.isValid = (addr) => {
return net.isIP(addr) !== 0;
};
ip.normalizeStrict = (addr) => {
let family;
switch (net.isIP(addr)) {
case 4:
family = 'ipv4';
break;
case 6:
family = 'ipv6';
break;
default:
throw new Error(`Invalid ip address: ${addr}`);
}
const { address } = new net.SocketAddress({ address: addr, family });
return address;
Tools
Biome

[error] 90-92: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.

};

function _normalizeFamily(family) {
Expand Down Expand Up @@ -306,26 +320,13 @@ ip.isEqual = function (a, b) {
};

ip.isPrivate = function (addr) {
// check loopback addresses first
if (ip.isLoopback(addr)) {
return true;
}

// ensure the ipv4 address is valid
if (!ip.isV6Format(addr)) {
const ipl = ip.normalizeToLong(addr);
if (ipl < 0) {
throw new Error('invalid ipv4 address');
}
// normalize the address for the private range checks that follow
addr = ip.fromLong(ipl);
}
addr = ip.normalizeStrict(addr);

// check private ranges
return /^(::f{4}:)?10\.([0-9]{1,3})\.([0-9]{1,3})\.([0-9]{1,3})$/i.test(addr)
return /^(::f{4}:)?127\.([0-9]{1,3})\.([0-9]{1,3})\.([0-9]{1,3})$/i.test(addr)
|| /^(::f{4}:)?10\.([0-9]{1,3})\.([0-9]{1,3})\.([0-9]{1,3})$/i.test(addr)
|| /^(::f{4}:)?192\.168\.([0-9]{1,3})\.([0-9]{1,3})$/i.test(addr)
|| /^(::f{4}:)?172\.(1[6-9]|2\d|30|31)\.([0-9]{1,3})\.([0-9]{1,3})$/i
.test(addr)
|| /^(::f{4}:)?172\.(1[6-9]|2\d|30|31)\.([0-9]{1,3})\.([0-9]{1,3})$/i.test(addr)
|| /^(::f{4}:)?169\.254\.([0-9]{1,3})\.([0-9]{1,3})$/i.test(addr)
|| /^f[cd][0-9a-f]{2}:/i.test(addr)
|| /^fe80:/i.test(addr)
Expand All @@ -337,16 +338,26 @@ ip.isPublic = function (addr) {
return !ip.isPrivate(addr);
};

ip.isLoopback = function (addr) {
// If addr is an IPv4 address in long integer form (no dots and no colons), convert it
if (!/\./.test(addr) && !/:/.test(addr)) {
addr = ip.fromLong(Number(addr));
ip.isValidAndPrivate = function (addr) {
try {
return ip.isPrivate(addr);
} catch {
return false;
}
};

ip.isValidAndPublic = function (addr) {
try {
return ip.isPublic(addr);
} catch {
return false;
}
};

ip.isLoopback = function (addr) {
addr = ip.normalizeStrict(addr);

return /^(::f{4}:)?127\.([0-9]{1,3})\.([0-9]{1,3})\.([0-9]{1,3})/
.test(addr)
|| /^0177\./.test(addr)
|| /^0x7f\./i.test(addr)
return /^(::f{4}:)?127\.([0-9]{1,3})\.([0-9]{1,3})\.([0-9]{1,3})/.test(addr)
|| /^fe80::1$/i.test(addr)
|| /^::1$/.test(addr)
|| /^::$/.test(addr);
Expand Down Expand Up @@ -443,6 +454,8 @@ ip.fromLong = function (ipl) {
};

ip.normalizeToLong = function (addr) {
if (typeof addr !== 'string') return -1;

const parts = addr.split('.').map(part => {
// Handle hexadecimal format
if (part.startsWith('0x') || part.startsWith('0X')) {
Expand All @@ -469,6 +482,7 @@ ip.normalizeToLong = function (addr) {

switch (n) {
case 1:
if (parts[0] > 0xffffffff) return -1;
val = parts[0];
break;
case 2:
Expand All @@ -489,3 +503,15 @@ ip.normalizeToLong = function (addr) {

return val >>> 0;
};

ip.normalizeLax = function(addr) {
if (ip.isV6Format(addr)) {
const { address } = new net.SocketAddress({ address: addr, family: 'ipv6' });
return address;
}
const ipl = ip.normalizeToLong(addr);
if (ipl < 0) {
throw Error(`Invalid ip address: ${addr}`);
}
return ip.fromLong(ipl);
};
Loading