choojs / choojs/nanoquery

Security: query-string keys (toString/constructor/hasOwnProperty) overwrite methods on parsed object

Open Beginner friendly
#6 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
49
Forks
2
PR merge metrics
No merged PRs in 30d

Description

## Security: query-string keys (toString/constructor/hasOwnProperty) overwrite methods on parsed object

**Package:** nanoquery
**Severity:** Medium (Denial of Service in consumers, direct from user-controlled URL)
**Type:** Special key handling — unfiltered key assignment into plain object

### Summary

`nanoquery` parses a URL query string into a plain object via `obj[key] = value` without filtering special keys. A query string containing keys like `toString`, `valueOf`, `hasOwnProperty`, or `constructor` overwrites those built-in methods on the returned object with string values. Any consumer that stringifies the object (template interpolation, logging) or calls `hasOwnProperty` (config checks) crashes.

Unlike `.env`-parser cases, the attacker controls the input **directly via a URL** — no supply-chain step needed.

### Location

`browser.js:17`:

```js
} else {
obj[key] = value // ← no filter for __proto__/constructor/toString/hasOwnProperty
}
```

Full parse loop (`browser.js:9-19`):

```js
var obj = {}
url.replace(/^.*\?/, '').replace(reg, function (a0, a1, a2, a3) {
var value = decodeURIComponent(a3)
var key = decodeURIComponent(a1)
if (obj.hasOwnProperty(key)) {
if (Array.isArray(obj[key])) obj[key].push(value)
else obj[key] = [obj[key], value]
} else {
obj[key] = value // ← line 17
}
})
```

### Proof of concept

```js
const qs = require('nanoquery')

const parsed = qs('?APP=MyApp&toString=EVIL')
console.log(typeof parsed.toString) // "string" — was "function"

// Any consumer stringifying the parsed object crashes:
// `'config: ' + parsed` → TypeError: Cannot convert object to primitive value

const parsed2 = qs('?hasOwnProperty=nope')
// parsed2.hasOwnProperty('x') → TypeError: parsed2.hasOwnProperty is not a function
```

### Impact

- **DoS via user-controlled URL**: a crafted query string (`?toString=1`) passed through `nanoquery` breaks any downstream code that stringifies or introspects the result.
- **Logic corruption**: `constructor` overwrite can break `instanceof`-style checks.
- No RCE: `__proto__=string` is rejected by the JS language (non-object value), so global prototype pollution does not trigger.

### Suggested fix

```js
if (key === '__proto__' || key === 'constructor' || key === 'prototype' ||
key === 'toString' || key === 'valueOf' || key === 'hasOwnProperty') {
return
}
```

### Related

Same class of issue (unfiltered special keys into plain object) confirmed in:
- `anima-querystring` — `src/querystring.js:118` (`ret[key] = m ? [val] : val`)
- `ini-simple-parser` — `dist/index.js:32,59`
- `dotenv-parse-variables` — process.env global pollution (High)
- `env-file-parser`, `@tokey/env-file-parser`, `envfile`

This looks like a systemic gap across small query-string/INI/env parsers: special keys are rarely filtered when building result objects.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with the parse loop in browser.js:9-19, especially the assignment at line 17, and reproduce the supplied cases for special query-string keys. Check how the returned object behaves when those keys are present, then verify that they no longer overwrite built-in methods while ordinary keys and repeated values still parse correctly.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
security
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
75/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.