Repository navigation
Typescript ignores prototype of object literals assigned to index signatures #37963
Description
TypeScript Version: 3.8.3 & nightly 2020-04-14
Search Terms:
- Object.create
- index signatures
- prototype
Code
// This should not be possible but is:
const unsafeStringMap: { [key: string]: string } = {};
const notActuallyAString: string | undefined = unsafeStringMap['constructor'];
console.log(
`Typescript thinks this is a string (or undefined), but it's actually a ${typeof notActuallyAString}.`
);
// This is a correct implementation of the same:
const safeStringMap: { [key: string]: string } = Object.create(null);
const aString: string | undefined = safeStringMap['constructor'];
console.log(
`Typescript thinks this is a string (or undefined), and indeed it's ${typeof aString}.`
);
// Typescript seems to correctly handle this when instantiating the `Object` constructor.
const correctlyHandledStringMap: { [key: string]: string } = new Object();
// Typescript also seems to be aware of an object literal's prototype chain for other purposes.
const foo: { constructor: typeof Object.prototype.constructor } = {}Expected behavior:
Assigning {} to { [key: string]: string } should fail, because it does not have a compatible signature. In javascript, an "object literal" {} is equivalent to new Object() and TS should treat it accordingly.
Actual behavior:
TS allows the assigning of {} to { [key: string]: string }, leading to runtime crashes and potential security issues (including prototype pollution) when a key with the same name as a member of Object.prototype (constructor, __proto__, etc) is accessed or set.
Playground Link:
Link
Related Issues:
#13778
Activity
RyanCavanaugh commentedon Apr 17, 2020
This is a potential problem, but the type system was designed prior to the existence of
Object.create(null)and many correct programs use{ }in places where the keys, by construction, don't overlap with any built-in names (or are always set as own properties first).mike-marcacci commentedon Apr 17, 2020
Thanks for the note!
Ya you're definitely right that there will be use-cases that happen to be safe (for example where we know from some external guarantee that key names are UUIDs), but it isn't really provable via the type system, making it painfully easy to introduce type errors or worse when that assumption gets broken.
I've personally messed this up enough to suspect that there are very real safety issues with the vast majority of cases that would become invalid with this fix.
From an ergonomics perspective this fix would ideally be accompanied by my suggestion over here, to support
__proto__in object literal declarations only, since object spreads have gained considerable popularity and the rules here are quite straightforward.You mention that you suspect there is a design limitation here, but the last two examples in my code block suggest that this information is being tracked, and just not enforced in the cases of indexed objects (although I certainly don't know how it's implemented). I'm curious to know what the limitation might be?
EDIT: I have turned the suggestion referenced above into a more thoroughly constructed proposal in #38385.
RyanCavanaugh commentedon Apr 17, 2020
The "limitation" is "we don't want to break a ton of existing correct code" 😉
mike-marcacci commentedon Apr 17, 2020
Ah haha, makes sense :) I suppose we could make this behavior suppressible via an option (maybe
supressIndexedObjectPrototypeErrors), or possibly "opt-in" via a new "strict" option (maybestrictIntexedObjects). (Although this probably needs to be weighed against adding more complexity to the configuration.)mike-marcacci commentedon May 8, 2020
While working on #38385 I discovered that
Object.create(null)was part of the 2009 ES5.0 spec, so I'd argue that this behavior has never been correct...The problem is quite prevalent.
I quickly searched github for indexed object signatures in typescript files, and found an astonishing number of potentially serious issues that would have been caught by correct type checking here.
I also took a moment to look through the documentation and external resources (blog posts, etc) that describe this feature, and I have yet to see this implemented safely, with either
Object.create(null)or{ __proto__: null }.Unless I missed a whole contingent of the internet, this means that the VAST majority of cases where this feature is being used, it is giving a false sense of safety...
EDIT: Below is part of my original comment here, which I'm removing from the scope of this issue, per Ryan Cavanaugh (@RyanCavanaugh)'s comment below.
This appears to be part of a broader problem with index signatures.
Further experimentation has brought me to the opinion that unconstrained indexed object signatures are severely safe in TypeScript at the moment. (I'm ignoring scenarios where index signatures are constrained by another type, like
{[k in keyof Foo}: null | F[k]}.) For example:The problem isn't just that the prototype of object literals is ignored, but that more broadly, index signatures describe an exhaustive set of properties. Accordingly, it's really never safe to "refine" one from a type that only defines a partial set of properties.
Both problems should probably be addressed together.
It's possible that these are two different issues, but I'd like to treat them as one, since the initial pushback was against introducing errors into codebases that currently (although incorrectly) compile. Based on my initial reasoning, both fixes expose the same kind of safety issue, and should be released together to avoid unnecessary developer toil.
mike-marcacci commentedon May 8, 2020
Property exhaustiveness problem relates to #12936.
RyanCavanaugh commentedon May 12, 2020
Mike Marcacci (@mike-marcacci) this is a known soundness hole; optional properties have the same problem. While it's sometimes possible to construct an adversarial repro to cause an issue to occur, in practice this doesn't cause problems and it's an intentional usability trade-off that we allow it.
mike-marcacci commentedon May 12, 2020
Ryan Cavanaugh (@RyanCavanaugh) thanks for the reply. That makes sense. I've updated my comment to retract the "exhaustiveness" problem from the scope of this issue, which can continue to focus on the original problem.
Is there a place that lists these soundness caveats? Having them is certainly an acceptable call, but it would be nice to know the intended limits of type safety that TS can provide.