denoland / deno

A modern runtime for JavaScript and TypeScript.
https://deno.com
MIT License
98.08k stars 5.4k forks source link

Insecure localStorage access #25437

Open artemik opened 2 months ago

artemik commented 2 months ago

How is Deno "secure by default" if it allows access to localStorage and moreover preserves data across separate "deno run" executions?

And I can't seem to find a way to restrict/remove localStorage access?

It seems there should be a way to control it. This was very unexpected behavior when without any "--allow-read" Deno allowed localStorage writes which is really technically a disk file write.

Version: Deno 1.46.1

rojvv commented 2 months ago

While this is not necessarily a solution to your issue, you can override the place it writes to using a new --location flag, which that makes it forget everything.

lucacasonato commented 2 months ago

localStorage and Deno.Kv access are local to the specific file / project. Multiple different Deno scripts do not share a storage key, and thus do not have access to each others' data. This also applies to the Web Cache API. This is documented here: https://docs.deno.com/runtime/reference/web_platform_apis/#web-storage

If you would like to ensure that two scripts do not share a storage key, you can use the --location flag to adjust it. Invoking the same script, once with --location=https://key1.local and once with --location=https://key2.local would result in the two invocations not seeing each others' data.

BlackAsLight commented 2 months ago

And I can't seem to find a way to restrict/remove localStorage access?

Would something like this at the start of your code not work? delete localStorage

lucacasonato commented 2 months ago

No that is not good enough

artemik commented 2 months ago

@BlackAsLight thanks, it seems delete localStorage worked now (before opening the ticket I did try that and I think I saw it fail, but apparently I was wrong and it does work now).

So I also deleted sessionStorage.

@lucacasonato do you think such approach (as a workaround) would be safe enough to guarantee localStorage couldn't be accessed by user scripts somehow differently?

Regarding different --location values suggestion, the data would be different indeed, but it's still preserved across different invocations, I'm worried about disk usage growing uncontrollably. As I understand each such individual storage is allowed up to 10MB, so in theory it can get out of control.

Philosophically, to follow Deno security idea, it seems it shouldn't allow localStorage/sessionStorage by default. I also saw a ticket request for indexedDb. So even if I delete localStorage now, with such "allowed by default" approach, some day in future indexedDb may suddenly become available to the code.

I must admit I'm trying to use Deno as a sandbox solution for untrusted JS scripts, therefore I have quite high security requirements, which other use cases might not require, but it seems I'm anyway not asking more than Deno`s "secure by default" idea.

lucacasonato commented 2 months ago

@artemik delete localStorage is not good enough if you want to disallow all persistence. You would also have to disable caches (and KV if you have --unstable-kv set).

Deno does not consider storage explosion attacks as part of it's threat model. You can already explode storage by just opening a bunch of large TS files through data or blob URLs. Every TS file will create a local cached file for the JS file, and will also create a V8 code cache file that is up to 10x larger than the source file.

If you want to protect against storage explosion attacks, I suggest you run each Deno instance inside of a different (virtual) storage volume that is size limited by your operating system.

artemik commented 2 months ago

@lucacasonato thanks for caches reminder, I suppose I can do delete caches as well.

Could you clarify the large TS files potential threat? I suppose if the size of the executing script is controlled and limited and internet access is disabled, there is no way a user could import anything at all? Even if it's a blob (I think you're referring to something like that: import 'data:text/javascript,console.log("hello!");'; ?) then it would fall under the same whole script size limit?

lucacasonato commented 2 months ago
for (let i = 0; i < 100; i++ {
  await import(`data:text/typescript,${btoa(`export const x = "${String(Math.random()).repeat(10000)}"`)}`);
}
artemik commented 2 months ago

Oh my. Though, wouldn't it be prevented by --max-heap-size and --max-old-space-size flags?

Or, would the problem still be there even if "x" is small, but imported many times causing millions of files (temp file per import?) giving a hard time anyway?

for (let i = 0; i < 1000000... ; i++) {
  await import(`data:text/typescript,${btoa(`export const x = 1`)}`);
}

Update: I did some tests, import(data:text/typescript ... don't cause any files to be created in Deno cache folder. Are they rather in memory? If yes, is it all limited by the above memory flags? It seems yes, because after certain iterations the script fails with out of memory.

artemik commented 2 months ago

@lucacasonato So if back to my original question, regardless of my sandboxing requirements, it still seems it would be safer to restrict access to local storage by default, not because of disk space worries, but just because two unrelated Deno scripts would share local storages (unless they specifically use different "location" params, hacky). Generally speaking, such easy sharing is not even something that exists in other programming languages.

Anyway, leaving this ticket for you and community.

lucacasonato commented 2 months ago

because two unrelated Deno scripts would share local storages

They would not. I mentioned this earlier in the thread. The default key is based on file path / config file path. See the docs I linked above

artemik commented 2 months ago

Apps started from the same module would? (no location/config specified) Though it might be a rare scenario.

lucacasonato commented 2 months ago

The same entrypoint, yes. I really urge you to read the linked documentation.

artemik commented 2 months ago

I did, yes, hence had commented about the same entrypoint without location/config specified.

lucacasonato commented 2 months ago

Multiple apps started from the same module, are by definition the same app, right? How do you distinguish between two apps that use the same entrypoint module?


If there is no configuration or location specifier, Deno uses the absolute path to the main module to determine what storage is shared. The Deno REPL generates a "synthetic" main module that is based off the current working directory where deno is started from. This means that multiple invocations of the REPL from the same path will share the persisted localStorage data.

From https://docs.deno.com/runtime/reference/web_platform_apis/#web-storage