SthephanShinkufag / bytebeat-composer

Bytebeat player with a collection of many formulas from around the internet.
https://dollchan.net/bytebeat/
MIT License
90 stars 28 forks source link

Potential to trick the parser #22

Closed ghost closed 2 years ago

ghost commented 2 years ago

A construct like unescape(escape(/* any action */)) is insecure. The parser performs the action an attacker wants. For example, the application could flood the console simply by allowing unescape(escape(console.log('Hello world!'))), overloading machine resources.

SthephanShinkufag commented 2 years ago

Not any action. The function runs in the AudioWorklet's sandbox (scope). Web Workers/Worklets cannot access the page (DOM ), you cannot communicate with window with document Object. But yes, you still have access to console.

@SArpnt writed:

The only security measure in place is only running the function in an AudioWorklet, on the global scope. As far as i can tell, this should be mostly secure, but this doesn't prevent:

  • taking advantage of browser security vulnerabilities
  • sending data to other webpages (not sure about this, XMLHttpRequest isn't avalible, but i don't know if there are other ways)
  • locking up the audio thread, making controls not work (volume still works, and the page can be refreshed)
  • anything else an AudioWorklet could do that i don't know about (i know barely anything about security) on a secure browser the website should be safe, but i can't guarantee anything.
ghost commented 2 years ago

I'm not sure, but it may be possible to run a cryptominer this way, since user code can take advantage of browser security vulnerabilities and send data to other webpages.

SthephanShinkufag commented 2 years ago

@Diicorp95 so if you created this issue, what are your suggestions for solving?

SArpnt commented 2 years ago

since user code can take advantage of browser security vulnerabilities and send data to other webpages.

as far as i know, sending any data outside of the webpage isn't possible. can you show any code that sends data to other webpages or programs? I've only said it might be possible just as a warning since browsers vary and some people take security very seriously, but i don't think it's likely.

it would be pointless to try to make a cryptominer anyways, the performance would be terrible and wouldn't even be able to mine anything until after someone leaves the page. the link would never get popular enough to ever get a useful amount because the website is niche, the link would be long and suspicious to most people, and nobody would bother sharing the link. any time spent making a cryptominer on this website would be FAR better spent making it on a different website or even just mining crypto on the device that would be used to make the bytebeat.

SArpnt commented 2 years ago

A construct like unescape(escape(/* any action */)) is insecure. The parser performs the action an attacker wants. For example, the application could flood the console simply by allowing unescape(escape(console.log('Hello world!'))), overloading machine resources.

unescape and escape aren't needed. you can just write the expression. all the code is written in javascript which is a full programming language. it's not any kind of "trick", it's entirely expected and something i regularly take advantage of when writing bytebeats. being able to write code isn't insecure just by itself, as many websites like lua.org do it without security risks.

flooding the console isn't dangerous, it's just annoying and can lag the webpage. the best it can do is lock up the tab, which isn't an issue since every modern browser sandboxes each tab individually and comes with a task manager to kill tabs if needed.

ghost commented 2 years ago

@Diicorp95 so if you created this issue, what are your suggestions for solving?

As a last resort, inherit JavaScript code parsing system and adapt it for bytebeat formulas. Otherwise ignore it.

ghost commented 2 years ago

unescape and escape aren't needed

The assumption was made by u/kOLbOSa_exe in his post.

ghost commented 2 years ago

Final solution by SArpnt:

this isn't a security vulnerability because console logs aren't dangerous escape and unescape aren't needed and aren't being used in a meaningful way here

the audioworklet scope blocks anything that actually matters like xml http requests, dom, etc.

In sum, it's not a security risk. Gratitude to @SArpnt for correcting me.