Repository: nodejs/node
Pull request: os: fix netmask format check condition in getCIDR function

Modified to check the format of the netmask instead of just checking that each part of the netmask is even number.
(Ref: https://www.rfc-editor.org/rfc/rfc1878)

The result of the previous run was like this.

```js
...
if ((binary & 1) !== 0) {
  return null;
}
...

console.log(getCIDR('127.0.0.1', '242.0.0.0', 'IPv4')); // 127.0.0.1/5
// 242 = 11110010(2)
```

This does not satisfy the condition in netmask, but it passes the conditional statement and returns an invalid value.

I'm not sure if that conditional is necessary(if the correct value is always passed, etc), but I think it's doing the wrong thing, so I've modified it to look like this.

```js
...
if (StringPrototypeIncludes(binary.toString(2), '01')) {
  return null;
}

console.log(getCIDR('127.0.0.1', '242.0.0.0', 'IPv4')); // null
...
```

I modified the netmask to check if it satisfies the condition of containing '01', such as 11110010, since a 0 cannot be followed by a 1 according to the rules of netmask.

The benchmark results for changing conditions are shown below.
```
                                confidence improvement accuracy (*)   (**)  (***)
os/networkInterfaces.js n=10000                 0.19 %       ±1.17% ±1.56% ±2.03%

Be aware that when doing many comparisons the risk of a false-positive result increases.
In this case, there are 1 comparisons, you can thus expect the following amount of false-positive results:
  0.05 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.01 false positives, when considering a   1% risk acceptance (**, ***),
  0.00 false positives, when considering a 0.1% risk acceptance (***)
```

Changed files:
filename: lib/internal/util.js
patch: @@ -8,6 +8,7 @@ const {
   Error,
   ErrorCaptureStackTrace,
   FunctionPrototypeCall,
+  NumberParseInt,
   ObjectDefineProperties,
   ObjectDefineProperty,
   ObjectFreeze,
@@ -33,7 +34,9 @@ const {
   SafeSet,
   SafeWeakMap,
   SafeWeakRef,
+  StringPrototypeIncludes,
   StringPrototypeReplace,
+  StringPrototypeSlice,
   StringPrototypeToLowerCase,
   StringPrototypeToUpperCase,
   Symbol,
@@ -806,6 +809,59 @@ function setupCoverageHooks(dir) {
   return coverageDirectory;
 }
 
+// Returns the number of ones in the binary representation of the decimal
+// number.
+function countBinaryOnes(n) {
+  // Count the number of bits set in parallel, which is faster than looping
+  n = n - ((n >>> 1) & 0x55555555);
+  n = (n & 0x33333333) + ((n >>> 2) & 0x33333333);
+  return ((n + (n >>> 4) & 0xF0F0F0F) * 0x1010101) >>> 24;
+}
+
+function getCIDR(address, netmask, family) {
+  let ones = 0;
+  let split = '.';
+  let range = 10;
+  let groupLength = 8;
+  let hasZeros = false;
+  let lastPos = 0;
+
+  if (family === 'IPv6') {
+    split = ':';
+    range = 16;
+    groupLength = 16;
+  }
+
+  for (let i = 0; i < netmask.length; i++) {
+    if (netmask[i] !== split) {
+      if (i + 1 < netmask.length) {
+        continue;
+      }
+      i++;
+    }
+    const part = StringPrototypeSlice(netmask, lastPos, i);
+    lastPos = i + 1;
+    if (part !== '') {
+      if (hasZeros) {
+        if (part !== '0') {
+          return null;
+        }
+      } else {
+        const binary = NumberParseInt(part, range);
+        const binaryOnes = countBinaryOnes(binary);
+        ones += binaryOnes;
+        if (binaryOnes !== groupLength) {
+          if (StringPrototypeIncludes(binary.toString(2), '01')) {
+            return null;
+          }
+          hasZeros = true;
+        }
+      }
+    }
+  }
+
+  return `${address}/${ones}`;
+}
 
 const handleTypes = ['TCP', 'TTY', 'UDP', 'FILE', 'PIPE', 'UNKNOWN'];
 function guessHandleType(fd) {
@@ -872,6 +928,7 @@ module.exports = {
   filterDuplicateStrings,
   filterOwnProperties,
   getConstructorOf,
+  getCIDR,
   getCWDURL,
   getInternalGlobal,
   getStructuredStack,

filename: lib/os.js
patch: @@ -24,7 +24,6 @@
 const {
   ArrayPrototypePush,
   Float64Array,
-  NumberParseInt,
   ObjectDefineProperties,
   StringPrototypeSlice,
   SymbolToPrimitive,
@@ -40,6 +39,7 @@ const {
   },
   hideStackFrames,
 } = require('internal/errors');
+const { getCIDR } = require('internal/util');
 const { validateInt32 } = require('internal/validators');
 
 const {
@@ -202,60 +202,6 @@ function endianness() {
 }
 endianness[SymbolToPrimitive] = () => kEndianness;
 
-// Returns the number of ones in the binary representation of the decimal
-// number.
-function countBinaryOnes(n) {
-  // Count the number of bits set in parallel, which is faster than looping
-  n = n - ((n >>> 1) & 0x55555555);
-  n = (n & 0x33333333) + ((n >>> 2) & 0x33333333);
-  return ((n + (n >>> 4) & 0xF0F0F0F) * 0x1010101) >>> 24;
-}
-
-function getCIDR(address, netmask, family) {
-  let ones = 0;
-  let split = '.';
-  let range = 10;
-  let groupLength = 8;
-  let hasZeros = false;
-  let lastPos = 0;
-
-  if (family === 'IPv6') {
-    split = ':';
-    range = 16;
-    groupLength = 16;
-  }
-
-  for (let i = 0; i < netmask.length; i++) {
-    if (netmask[i] !== split) {
-      if (i + 1 < netmask.length) {
-        continue;
-      }
-      i++;
-    }
-    const part = StringPrototypeSlice(netmask, lastPos, i);
-    lastPos = i + 1;
-    if (part !== '') {
-      if (hasZeros) {
-        if (part !== '0') {
-          return null;
-        }
-      } else {
-        const binary = NumberParseInt(part, range);
-        const binaryOnes = countBinaryOnes(binary);
-        ones += binaryOnes;
-        if (binaryOnes !== groupLength) {
-          if ((binary & 1) !== 0) {
-            return null;
-          }
-          hasZeros = true;
-        }
-      }
-    }
-  }
-
-  return `${address}/${ones}`;
-}
-
 /**
  * @returns {Record<string, Array<{
  *  address: string,

filename: test/parallel/test-internal-util-getCIDR.js
patch: @@ -0,0 +1,23 @@
+// Flags: --expose-internals
+'use strict';
+require('../common');
+
+// These are tests that verify that the subnetmask is used
+// to create the correct CIDR address.
+// Tests that it returns null if the subnetmask is not in the correct format.
+// (ref: https://www.rfc-editor.org/rfc/rfc1878)
+
+const assert = require('node:assert');
+const { getCIDR } = require('internal/util');
+
+assert.strictEqual(getCIDR('127.0.0.1', '255.0.0.0', 'IPv4'), '127.0.0.1/8');
+assert.strictEqual(getCIDR('127.0.0.1', '255.255.0.0', 'IPv4'), '127.0.0.1/16');
+
+// 242 = 11110010(2)
+assert.strictEqual(getCIDR('127.0.0.1', '242.0.0.0', 'IPv4'), null);
+
+assert.strictEqual(getCIDR('::1', 'ffff:ffff:ffff:ffff::', 'IPv6'), '::1/64');
+assert.strictEqual(getCIDR('::1', 'ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff', 'IPv6'), '::1/128');
+
+// ff00:ffff = 11111111 00000000 : 11111111 11111111(2)
+assert.strictEqual(getCIDR('::1', 'ffff:ff00:ffff::', 'IPv6'), null);