Threats tagged '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_b