Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WebIDL symbols shouldn't be exposed to user code #11720

Open
ghost opened this issue Aug 15, 2021 · 5 comments
Open

WebIDL symbols shouldn't be exposed to user code #11720

ghost opened this issue Aug 15, 2021 · 5 comments
Labels
cli related to cli/ dir suggestion suggestions for new features (yet to be agreed)

Comments

@ghost
Copy link

ghost commented Aug 15, 2021

The webidl.brand symbol is a symbol on Deno's WebIDL API instances. Symbols are observable to any user script.

This allows anyone to simply take it, and bypass any checks, e.g.

// gets symbols and strings
// in browsers, this is an empty array, in Deno this is an array with a length of one
const webidl_brand = Reflect.ownKeys(performance)[0];

// not a host object, but functions as a subclass in Deno's implementation
const my_performance = {
  __proto__: Performance.prototype,
  [webidl_brand]: webidl_brand,
};

// should throw a TypeError
console.log(my_performance.now());

Should this be allowed?

A webidl.branded WeakSet would be much more secure, in that it would ensure encapsulation.

@crowlKats
Copy link
Member

This applies for any other symbol that we use for internal slots as well, not just webidl brands

@ghost
Copy link
Author

ghost commented Aug 16, 2021

This applies for any other symbol that we use for internal slots as well, not just webidl brands

Absolutely, this was just an example of a particular symbol. But is this intended to be a "feature," or is it more of a limitation of the implementation?

@ghost ghost changed the title WebIDL brand symbol shouldn't be exposed to user code WebIDL symbols shouldn't be exposed to user code Aug 16, 2021
@stale
Copy link

stale bot commented Oct 15, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Oct 15, 2021
@kitsonk kitsonk added the suggestion suggestions for new features (yet to be agreed) label Oct 15, 2021
@stale stale bot removed the stale label Oct 15, 2021
@kitsonk kitsonk added cli related to cli/ dir stale labels Oct 15, 2021
@stale stale bot removed the stale label Oct 15, 2021
@kitsonk
Copy link
Contributor

kitsonk commented Oct 15, 2021

Sorry I missed this before, but I tend to agree... I ran into a challenge where I created something that extended EventTarget and logging to the console leaked a whole load if "internal" stuff.

Feels like we should have an internal symbol filter feature on inspect. @lucacasonato thoughts?

@ghost
Copy link
Author

ghost commented Oct 15, 2021

Feels like we should have an internal symbol filter feature on inspect.

Now, I do fully agree: having the internal implementation values revealed in Console functions does look ugly, and is quite misleading, but I'm talking from a purely functional standpoint, i.e. just code operating on these values.

Do you think it's worth making the implementation not show the extra properties?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli related to cli/ dir suggestion suggestions for new features (yet to be agreed)
Projects
None yet
Development

No branches or pull requests

2 participants