Summary
js-toml's interpreter checks whether a key already exists in a parser-built container with if (object[key]) instead of if (key in object). When the prior value is a falsy primitive — false, 0, 0n, 0.0, -0, or "" — the duplicate-key branch is skipped and the value is silently overwritten by a later sub-table, dotted-key sub-table, or array-of-tables sharing the same name. Per the TOML 1.0.0 spec ("Defining a key multiple times is invalid"; "You cannot define any key or table more than once"), this should be a parse error.
The result is structural type confusion of attacker-named keys in the value returned by load(). A boolean-typed false (or numeric 0) becomes a truthy object. Host applications that gate behavior on if (config.flag), if (!user.banned), if (config.allowDelete), or if (config.publicMode) will silently take the truthy branch.
This is distinct from GHSA-65fc-cr5f-v7r2 (the 1.0.2 prototype-pollution fix). Object.prototype is not polluted. The Object.create(null) mitigation from 1.0.2 is intact; the bug here is in the duplicate-key state machine, not in container construction.
Details
Two truthy checks are wrong:
src/load/interpreter.ts:214 — Interpreter.tryCreatingObject
if (object[key]) { // falsy primitives slip through
// duplicate-key logic
} else {
object[key] = createSafeObject(); // silently overwrites the prior falsy value
...
}
src/load/interpreter.ts:278 — Interpreter.getOrCreateArray
if (object[first] && !Array.isArray(object[first])) { // same flaw
throw new DuplicateKeyError();
}
object[first] = object[first] || []; // overwrites the prior falsy value
Both should use the in operator. Containers are created via Object.create(null), so in is unambiguous (no inherited keys to worry about).
The bug is reachable through every parent-walking interpreter path:
assignValue — dotted keys in key = value
createTable — [stdTable] headers
getOrCreateArray — [[arrayOfTables]] headers
PoC
isAdmin = false
[isAdmin]
forced = "yes"
import { load } from 'js-toml';
const config = load(`
isAdmin = false
[isAdmin]
forced = "yes"
`);
console.log(JSON.stringify(config));
// {"isAdmin":{"forced":"yes"}}
console.log(config.isAdmin ? 'BYPASS' : 'safe');
// BYPASS
if (config.isAdmin) {
// attacker reaches admin-only code
}
Impact
Spec-violating input acceptance leading to structural type confusion. (CWE-697)
Suggested fix
in src/load/interpreter.ts
export class Interpreter extends BaseCstVisitor {
ignoreImplicitDeclared,
ignoreExplicitDeclared
) {
- if (object[key]) {
+ if (key in object) {
if (
!isPlainObject(object[key]) ||
(!ignoreExplicitDeclared &&
export class Interpreter extends BaseCstVisitor {
return this.getOrCreateArray(keys, object[first], idx + 1);
}
- if (object[first] && !Array.isArray(object[first])) {
+ if (first in object && !Array.isArray(object[first])) {
throw new DuplicateKeyError();
}
object[first] = object[first] || [];
References
Summary
js-toml's interpreter checks whether a key already exists in a parser-built container withif (object[key])instead ofif (key in object). When the prior value is a falsy primitive —false,0,0n,0.0,-0, or""— the duplicate-key branch is skipped and the value is silently overwritten by a later sub-table, dotted-key sub-table, or array-of-tables sharing the same name. Per the TOML 1.0.0 spec ("Defining a key multiple times is invalid"; "You cannot define any key or table more than once"), this should be a parse error.The result is structural type confusion of attacker-named keys in the value returned by
load(). A boolean-typedfalse(or numeric0) becomes a truthy object. Host applications that gate behavior onif (config.flag),if (!user.banned),if (config.allowDelete), orif (config.publicMode)will silently take the truthy branch.This is distinct from GHSA-65fc-cr5f-v7r2 (the 1.0.2 prototype-pollution fix).
Object.prototypeis not polluted. TheObject.create(null)mitigation from 1.0.2 is intact; the bug here is in the duplicate-key state machine, not in container construction.Details
Two truthy checks are wrong:
src/load/interpreter.ts:214—Interpreter.tryCreatingObjectsrc/load/interpreter.ts:278—Interpreter.getOrCreateArrayBoth should use the
inoperator. Containers are created viaObject.create(null), soinis unambiguous (no inherited keys to worry about).The bug is reachable through every parent-walking interpreter path:
assignValue— dotted keys inkey = valuecreateTable—[stdTable]headersgetOrCreateArray—[[arrayOfTables]]headersPoC
Impact
Spec-violating input acceptance leading to structural type confusion. (CWE-697)
Suggested fix
in
src/load/interpreter.tsexport class Interpreter extends BaseCstVisitor { ignoreImplicitDeclared, ignoreExplicitDeclared ) { - if (object[key]) { + if (key in object) { if ( !isPlainObject(object[key]) || (!ignoreExplicitDeclared &&export class Interpreter extends BaseCstVisitor { return this.getOrCreateArray(keys, object[first], idx + 1); } - if (object[first] && !Array.isArray(object[first])) { + if (first in object && !Array.isArray(object[first])) { throw new DuplicateKeyError(); } object[first] = object[first] || [];References