fix(permission): Address permissions issue for UNC paths on Windows

[yazan-abdalrahman]

fix #24703

[yazan-abdalrahman]

@dsherret please can review this change?

[yazan-abdalrahman]

@dsherret @bartlomieju
Hello, could you help verify why it failed? It works on my local.

deno.runtime_permissions_lib.rs.2024-08-21.17-24-59.mp4

And when I ran the script, it didn't set failed to true, therefore it didn't output "One or more tests failed" and it didn't return 321.

deno.special_main.js.2024-08-21.17-27-31.mp4

[dsherret]

This might be a security issue. I think there's a reason why this isn't allowed without -A, but I can't remember at the moment.

[yazan-abdalrahman]

@dsherret this #23208 prevent some paths if they don't have all permissions, so we need to exclude UNC paths from that pattern with this code or close issue it if it's for security reasons.

[yazan-abdalrahman]

@dsherret this #23208 prevent some paths if they don't have all permissions, so we need to exclude UNC paths from that pattern with this code or close issue it if it's for security reasons.

Regarding #23208. I realize it's not a security problem, thus can accept this change?

[yazan-abdalrahman]

@mmastrac Please check this change.

[yazan-abdalrahman]

@dsherret @mmastrac
Can we accept it, or should we close the issue and consider it unsolvable?

[dsherret]

I looked into this a bit and I think it's more complicated than it seems. Here's some points from an old internal document:

• If users are using \\.\ paths to access things like named pipes, raw devices, those should trigger --allow-sys .
• Network resources should perform DNS lookup in permission checks, and the fully-resolved permission check object should include the the IP address. When we make the outgoing network requests, we must use the resolved IP.

That said, I'm not sure about what the right solution is here. At the moment, this just requires --allow-all permissions.

[yazan-abdalrahman]

I looked into this a bit and I think it's more complicated than it seems. Here's some points from an old internal document:

• If users are using \\.\ paths to access things like named pipes, raw devices, those should trigger --allow-sys .
• Network resources should perform DNS lookup in permission checks, and the fully-resolved permission check object should include the the IP address. When we make the outgoing network requests, we must use the resolved IP.

That said, I'm not sure about what the right solution is here. At the moment, this just requires --allow-all permissions.

but I assume if it requires extra permission, it will prompt to ask it to grant the permission, thus actually at this moment it grants all the permission needed, no need to verify all permissions.

[yazan-abdalrahman]

This might be a security issue. I think there's a reason why this isn't allowed without -A, but I can't remember at the moment.

This might be a security issue. I think there's a reason why this isn't allowed without -A, but I can't remember at the moment.

@dsherret
What about this? I don't think it's a security issue, but why do sheared files require all permissions? Just some permissions like read, write, and net? and why it didn't prompt to give it all permission?

[rivy]

@dsherret , I'm running into this issue while trying to open any UNC path or DOS device (eg, '\.\CONIN$') with non---allow-all permissions.

Is there any thoughts on what permissions (besides just defaulting to --allow-all) that could/should be used for this fairly common CLI and network script usage?

Requiring --allow-all is especially problematic as it can't be tested using the permissions API.

Should anyone else be included in the discussion?

Thanks for the attention.