1
0
Fork 0
mirror of https://github.com/denoland/deno.git synced 2024-11-24 15:19:26 -05:00

fix(cli): harden permission stdio check (#21778)

Harden the code that does permission checks to protect against
re-opening of stdin.

Code that runs FFI is vulnerable to an attack where fd 0 is closed
during a permission check and re-opened with a file that contains a
positive response (ie: `y` or `A`). While FFI code is dangerous in
general, we can make it more difficult for FFI-enabled code to bypass
additional permission checks.

- Checks to see if the underlying file for stdin has changed from the
start to the end of the permission check (detects races)
- Checks to see if the message is excessively long (lowering the window
for races)
- Checks to see if stdin and stderr are still terminals at the end of
the function (making races more difficult)
This commit is contained in:
Matt Mastracci 2024-01-03 16:31:39 -07:00 committed by GitHub
parent 7f1c41d245
commit 00970daea2
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 180 additions and 118 deletions

View file

@ -707,6 +707,20 @@ fn permissions_prompt_allow_all_lowercase_a() {
});
}
#[test]
fn permission_request_long() {
TestContext::default()
.new_command()
.args_vec(["run", "--quiet", "run/permission_request_long.ts"])
.with_pty(|mut console| {
console.expect(concat!(
"❌ Permission prompt length (100017 bytes) was larger than the configured maximum length (10240 bytes): denying request.\r\n",
"❌ WARNING: This may indicate that code is trying to bypass or hide permission check requests.\r\n",
"❌ Run again with --allow-read to bypass this check if this is really what you want to do.\r\n",
));
});
}
itest!(deny_all_permission_args {
args: "run --deny-env --deny-read --deny-write --deny-ffi --deny-run --deny-sys --deny-net --deny-hrtime run/deny_all_permission_args.js",
output: "run/deny_all_permission_args.out",

View file

@ -0,0 +1 @@
Deno.open("a".repeat(1e5));

View file

@ -21,6 +21,9 @@ fn strip_ansi_codes_and_ascii_control(s: &str) -> std::borrow::Cow<str> {
pub const PERMISSION_EMOJI: &str = "⚠️";
// 10kB of permission prompting should be enough for anyone
const MAX_PERMISSION_PROMPT_LENGTH: usize = 10 * 1024;
#[derive(Debug, Eq, PartialEq)]
pub enum PromptResponse {
Allow,
@ -77,18 +80,6 @@ pub trait PermissionPrompter: Send + Sync {
pub struct TtyPrompter;
impl PermissionPrompter for TtyPrompter {
fn prompt(
&mut self,
message: &str,
name: &str,
api_name: Option<&str>,
is_unary: bool,
) -> PromptResponse {
if !std::io::stdin().is_terminal() || !std::io::stderr().is_terminal() {
return PromptResponse::Deny;
};
#[cfg(unix)]
fn clear_stdin(
_stdin_lock: &mut StdinLock,
@ -163,8 +154,7 @@ impl PermissionPrompter for TtyPrompter {
input_record.Event.KeyEvent_mut().wVirtualKeyCode = VK_RETURN as WORD;
input_record.Event.KeyEvent_mut().wVirtualScanCode =
MapVirtualKeyW(VK_RETURN as UINT, MAPVK_VK_TO_VSC) as WORD;
*input_record.Event.KeyEvent_mut().uChar.UnicodeChar_mut() =
'\r' as WCHAR;
*input_record.Event.KeyEvent_mut().uChar.UnicodeChar_mut() = '\r' as WCHAR;
let mut record_written = 0;
let success =
@ -209,6 +199,43 @@ impl PermissionPrompter for TtyPrompter {
write!(stderr_lock, "\x1B[{n}A\x1B[0J").unwrap();
}
#[cfg(unix)]
fn get_stdin_metadata() -> std::io::Result<std::fs::Metadata> {
use std::os::fd::FromRawFd;
use std::os::fd::IntoRawFd;
// SAFETY: we don't know if fd 0 is valid but metadata() will return an error in this case (bad file descriptor)
// and we can panic.
unsafe {
let stdin = std::fs::File::from_raw_fd(0);
let metadata = stdin.metadata().unwrap();
stdin.into_raw_fd();
Ok(metadata)
}
}
impl PermissionPrompter for TtyPrompter {
fn prompt(
&mut self,
message: &str,
name: &str,
api_name: Option<&str>,
is_unary: bool,
) -> PromptResponse {
if !std::io::stdin().is_terminal() || !std::io::stderr().is_terminal() {
return PromptResponse::Deny;
};
if message.len() > MAX_PERMISSION_PROMPT_LENGTH {
eprintln!("❌ Permission prompt length ({} bytes) was larger than the configured maximum length ({} bytes): denying request.", message.len(), MAX_PERMISSION_PROMPT_LENGTH);
eprintln!("❌ WARNING: This may indicate that code is trying to bypass or hide permission check requests.");
eprintln!("❌ Run again with --allow-{name} to bypass this check if this is really what you want to do.");
return PromptResponse::Deny;
}
#[cfg(unix)]
let metadata_before = get_stdin_metadata().unwrap();
// Lock stdio streams, so no other output is written while the prompt is
// displayed.
let stdout_lock = std::io::stdout().lock();
@ -306,6 +333,26 @@ impl PermissionPrompter for TtyPrompter {
drop(stderr_lock);
drop(stdin_lock);
// Ensure that stdin has not changed from the beginning to the end of the prompt. We consider
// it sufficient to check a subset of stat calls. We do not consider the likelihood of a stdin
// swap attack on Windows to be high enough to add this check for that platform. These checks will
// terminate the runtime as they indicate something nefarious is going on.
#[cfg(unix)]
{
use std::os::unix::fs::MetadataExt;
let metadata_after = get_stdin_metadata().unwrap();
assert_eq!(metadata_before.dev(), metadata_after.dev());
assert_eq!(metadata_before.ino(), metadata_after.ino());
assert_eq!(metadata_before.rdev(), metadata_after.rdev());
assert_eq!(metadata_before.uid(), metadata_after.uid());
assert_eq!(metadata_before.gid(), metadata_after.gid());
assert_eq!(metadata_before.mode(), metadata_after.mode());
}
// Ensure that stdin and stderr are still terminals before we yield the response.
assert!(std::io::stdin().is_terminal() && std::io::stderr().is_terminal());
value
}
}