GHSA-7648-jmwf-vw7j

## Summary `networkStats()` keeps a per interface cache for computing `rx_sec` / `tx_sec` between calls. The cache is a plain JavaScript object and is indexed with the interface name the caller supplied. Passing the name `__proto__` makes every cache write land on `Object.prototype` instead of the cache, polluting every object in the Node.js process. An application is affected if it passes untrusted input as the `ifaces` argument of `si.networkStats()` and runs on FreeBSD, OpenBSD or NetBSD. ## Details ### The vulnerable code `lib/network.js` declares the cache as a plain object: ```js const _network = {}; ``` `calcNetworkSpeed()` writes into it, keyed by the interface name: ```js } else { if (!_network[iface]) { _network[iface] = {}; } _network[iface].rx_bytes = rx_bytes; _network[iface].tx_bytes = tx_bytes; _network[iface].rx_sec = null; _network[iface].tx_sec = null; _network[iface].ms = Date.now(); _network[iface].last_ms = 0; _network[iface].operstate = operstate; } ``` With `iface === '__proto__'` this misbehaves twice: 1. `_network['__proto__']` does not read a missing own property — it invokes the inherited `__proto__` getter and returns `Object.prototype`, which is truthy. The `if (!_network[iface])` guard therefore never creates the `{}` placeholder. 2. Each following assignment writes onto `Object.prototype`. After one call, `rx_bytes`, `tx_bytes`, `rx_sec`, `tx_sec`, `ms`, `last_ms` and `operstate` are visible on **every** object in the process. A second call takes the other branch (`_network['__proto__'].ms` is now truthy) and keeps them updated. ### Why the existing sanitizing does not help The interface name does pass through `util.sanitizeString(iface, true)` before it is used as the cache key. That function is a **shell metacharacter** filter, built to stop command injection in the `netstat` / `ifconfig` invocations — a different threat. Property names made of letters and underscores are legitimate interface names and pass through untouched: ``` "a;id" -> "aid" "a$(id)" -> "aid" "a|id" -> "aid" "a`id`" -> "aid" "__proto__" -> "__proto__" <-- unchanged "constructor" -> "constructor" <-- unchanged ``` The sanitizer is doing its job; the gap is that no layer treats the value as an *object key*. ### Reachability The cache write is guarded incidentally on three of four platform branches. These guards exist for unrelated reasons and are not a deliberate defence: | Platform | Code path | Reachable with an arbitrary name? | | --- | --- | --- | | Linux / Android | `fs.existsSync('/sys/class/net/' + ifaceSanitized)` | **No** — a non-existent interface returns early | | macOS | requires `netstat` to emit a data row beyond the header | **No** — verified on macOS 27, `Object.prototype` stays clean | | Windows | `if (rx_bytes && tx_bytes)` — requires a real adapter match | **No** | | **FreeBSD / OpenBSD / NetBSD** | `if (!error) { …; result = calcNetworkSpeed(ifaceSanitized, …) }` | **Yes** — the call sits outside the parsing loop, so any successful `netstat` exit reaches it, and `netstat -ibndI <unknown>` prints only its header and exits `0` | The important part is not that three platforms happen to be safe, but that nothing in the design prevents the write. Any future platform branch without such a guard reopens the issue, which is why the fix is applied at the cache rather than at the call sites. ### Age The pattern predates the v5 line — `git log -S "_network[iface] = {}"` traces it back to a refactoring before `v3.42.5`. The v3 and v4 lines are end of life; the advisory covers v5. ## Impact The attacker controls which attributes appear on `Object.prototype` but not their values — those come from the interface statistics (`rx_bytes`, `ms`, `operstate` …). The attribute *names* are fixed by the library, so this is not an arbitrary-key pollution primitive. The consequences are therefore application specific and arise from name collisions: - Any application code that tests `if (obj.operstate)`, `if (obj.ms)` or similar on an object that should not have those properties now takes the wrong branch. - `for…in` loops over plain objects gain seven extra enumerable keys, which can corrupt serialisation, configuration merging or validation logic. - Libraries that use `{}` as a lookup map and check membership with `in` or truthiness can return values for keys that were never inserted. No path to remote code execution is known, and the values cannot be chosen by the attacker. ## Proof of concept ### The underlying primitive (any platform) ```js const { calcNetworkSpeed } = require('systeminformation/lib/network.js'); // internal const cache = {}; // the declaration used by the library calcNetworkSpeed('__proto__', 1, 2, 'up', 0, 0, 0, 0); console.log({}.rx_bytes); // 1 console.log({}.operstate); // 'up' console.log(Object.prototype.hasOwnProperty('rx_bytes')); // true ``` ### End to end through the public API (FreeBSD / OpenBSD / NetBSD) ```js const si = require('systeminformation'); (async () => { console.log('before:', {}.operstate); // undefined await si.networkStats('__proto__'); // an attacker supplied interface name console.log('after :', {}.operstate); // 'up' -> Object.prototype is polluted })(); ``` On Linux, macOS and Windows the same call returns an empty result set and leaves `Object.prototype` untouched, for the reasons in the *Reachability* table. ## Patch `lib/network.js` — give the cache a null prototype, so `__proto__` becomes an ordinary own property instead of a setter into the prototype chain: ```diff -const _network = {}; +const _network = Object.create(null); ``` This is the smallest change that fixes the class of problem rather than one reachable path: - It is independent of the platform branches, so no guard has to be added or maintained. - It keeps behaviour identical for every legitimate interface name — `Object.keys()`, member reads and writes all work unchanged on a null prototype object. - It matches the pattern already used elsewhere in the codebase (`lib/filesystem.js:114-115`, `lib/graphics.js:1060`, `lib/graphics.js:1087`). - `Object.create` is ES5.1, well below the declared `engines.node >= 10.0.0`. After the patch the hostile name is stored as a normal cache entry: ```js const cache = Object.create(null); // ... cache['__proto__'] holds the stats object, Object.prototype is untouched ``` ## Workarounds For users who cannot upgrade immediately, either of these is sufficient: 1. **Validate the interface name before calling** — accept only names returned by `si.networkInterfaces()`, or reject the names `__proto__`, `constructor` and `prototype`. 2. **Never pass untrusted input** as the `ifaces` argument. Passing `'*'` or omitting the argument is unaffected, because those names come from the system rather than the caller. Freezing `Object.prototype` (`Object.freeze(Object.prototype)`) also prevents the write, but it has wide-reaching side effects on other dependencies and is not recommended as a general mitigation. ## Affected versions All `5.x` releases up to and including `5.33.12`. Earlier major lines contain the same pattern but are end of life. ### What was verified, and what was not Verified on this machine (macOS 27, Node 24): - the pollution primitive, by calling the cache write with `iface = '__proto__'`; - that `util.sanitizeString('__proto__', true)` returns the string unchanged, while shell metacharacters are stripped; - that unpatched v5 on macOS does **not** pollute through the public API; - that `netstat -bdnI <unknown-interface>` prints only its header and exits `0`; - that the patch stores the hostile key as an own property and leaves `Object.prototype` clean. Not verified, because no such host was available: - end to end exploitation on FreeBSD, OpenBSD or NetBSD. That conclusion is derived from reading the BSD branch of `networkStatsSingle()` — where the `calcNetworkSpeed()` call sits outside the row parsing loop and is reached whenever `netstat` exits successfully — combined with the observed `netstat` exit behaviour above. Worth confirming on a BSD host before publishing, since the affected-platform list depends on it.