From 5d7ebea99f67219e763d661a5e04d7c75af31878 Mon Sep 17 00:00:00 2001 From: Matt Mastracci Date: Wed, 3 Jan 2024 16:31:39 -0700 Subject: [PATCH] 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) --- cli/tests/integration/run_tests.rs | 14 + .../testdata/run/permission_request_long.ts | 1 + runtime/permissions/prompter.rs | 283 ++++++++++-------- 3 files changed, 180 insertions(+), 118 deletions(-) create mode 100644 cli/tests/testdata/run/permission_request_long.ts diff --git a/cli/tests/integration/run_tests.rs b/cli/tests/integration/run_tests.rs index 43bc212c66..36eee1100c 100644 --- a/cli/tests/integration/run_tests.rs +++ b/cli/tests/integration/run_tests.rs @@ -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", diff --git a/cli/tests/testdata/run/permission_request_long.ts b/cli/tests/testdata/run/permission_request_long.ts new file mode 100644 index 0000000000..05937e95a1 --- /dev/null +++ b/cli/tests/testdata/run/permission_request_long.ts @@ -0,0 +1 @@ +Deno.open("a".repeat(1e5)); diff --git a/runtime/permissions/prompter.rs b/runtime/permissions/prompter.rs index f0c3bbad32..145e0a82e8 100644 --- a/runtime/permissions/prompter.rs +++ b/runtime/permissions/prompter.rs @@ -21,6 +21,9 @@ fn strip_ansi_codes_and_ascii_control(s: &str) -> std::borrow::Cow { 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,6 +80,140 @@ pub trait PermissionPrompter: Send + Sync { pub struct TtyPrompter; +#[cfg(unix)] +fn clear_stdin( + _stdin_lock: &mut StdinLock, + _stderr_lock: &mut StderrLock, +) -> Result<(), AnyError> { + // TODO(bartlomieju): + #[allow(clippy::undocumented_unsafe_blocks)] + let r = unsafe { libc::tcflush(0, libc::TCIFLUSH) }; + assert_eq!(r, 0); + Ok(()) +} + +#[cfg(not(unix))] +fn clear_stdin( + stdin_lock: &mut StdinLock, + stderr_lock: &mut StderrLock, +) -> Result<(), AnyError> { + use deno_core::anyhow::bail; + use winapi::shared::minwindef::TRUE; + use winapi::shared::minwindef::UINT; + use winapi::shared::minwindef::WORD; + use winapi::shared::ntdef::WCHAR; + use winapi::um::processenv::GetStdHandle; + use winapi::um::winbase::STD_INPUT_HANDLE; + use winapi::um::wincon::FlushConsoleInputBuffer; + use winapi::um::wincon::PeekConsoleInputW; + use winapi::um::wincon::WriteConsoleInputW; + use winapi::um::wincontypes::INPUT_RECORD; + use winapi::um::wincontypes::KEY_EVENT; + use winapi::um::winnt::HANDLE; + use winapi::um::winuser::MapVirtualKeyW; + use winapi::um::winuser::MAPVK_VK_TO_VSC; + use winapi::um::winuser::VK_RETURN; + + // SAFETY: winapi calls + unsafe { + let stdin = GetStdHandle(STD_INPUT_HANDLE); + // emulate an enter key press to clear any line buffered console characters + emulate_enter_key_press(stdin)?; + // read the buffered line or enter key press + read_stdin_line(stdin_lock)?; + // check if our emulated key press was executed + if is_input_buffer_empty(stdin)? { + // if so, move the cursor up to prevent a blank line + move_cursor_up(stderr_lock)?; + } else { + // the emulated key press is still pending, so a buffered line was read + // and we can flush the emulated key press + flush_input_buffer(stdin)?; + } + } + + return Ok(()); + + unsafe fn flush_input_buffer(stdin: HANDLE) -> Result<(), AnyError> { + let success = FlushConsoleInputBuffer(stdin); + if success != TRUE { + bail!( + "Could not flush the console input buffer: {}", + std::io::Error::last_os_error() + ) + } + Ok(()) + } + + unsafe fn emulate_enter_key_press(stdin: HANDLE) -> Result<(), AnyError> { + // https://github.com/libuv/libuv/blob/a39009a5a9252a566ca0704d02df8dabc4ce328f/src/win/tty.c#L1121-L1131 + let mut input_record: INPUT_RECORD = std::mem::zeroed(); + input_record.EventType = KEY_EVENT; + input_record.Event.KeyEvent_mut().bKeyDown = TRUE; + input_record.Event.KeyEvent_mut().wRepeatCount = 1; + 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; + + let mut record_written = 0; + let success = + WriteConsoleInputW(stdin, &input_record, 1, &mut record_written); + if success != TRUE { + bail!( + "Could not emulate enter key press: {}", + std::io::Error::last_os_error() + ); + } + Ok(()) + } + + unsafe fn is_input_buffer_empty(stdin: HANDLE) -> Result { + let mut buffer = Vec::with_capacity(1); + let mut events_read = 0; + let success = + PeekConsoleInputW(stdin, buffer.as_mut_ptr(), 1, &mut events_read); + if success != TRUE { + bail!( + "Could not peek the console input buffer: {}", + std::io::Error::last_os_error() + ) + } + Ok(events_read == 0) + } + + fn move_cursor_up(stderr_lock: &mut StderrLock) -> Result<(), AnyError> { + write!(stderr_lock, "\x1B[1A")?; + Ok(()) + } + + fn read_stdin_line(stdin_lock: &mut StdinLock) -> Result<(), AnyError> { + let mut input = String::new(); + stdin_lock.read_line(&mut input)?; + Ok(()) + } +} + +// Clear n-lines in terminal and move cursor to the beginning of the line. +fn clear_n_lines(stderr_lock: &mut StderrLock, n: usize) { + write!(stderr_lock, "\x1B[{n}A\x1B[0J").unwrap(); +} + +#[cfg(unix)] +fn get_stdin_metadata() -> std::io::Result { + 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, @@ -89,125 +226,15 @@ impl PermissionPrompter for TtyPrompter { 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)] - fn clear_stdin( - _stdin_lock: &mut StdinLock, - _stderr_lock: &mut StderrLock, - ) -> Result<(), AnyError> { - // TODO(bartlomieju): - #[allow(clippy::undocumented_unsafe_blocks)] - let r = unsafe { libc::tcflush(0, libc::TCIFLUSH) }; - assert_eq!(r, 0); - Ok(()) - } - - #[cfg(not(unix))] - fn clear_stdin( - stdin_lock: &mut StdinLock, - stderr_lock: &mut StderrLock, - ) -> Result<(), AnyError> { - use deno_core::anyhow::bail; - use winapi::shared::minwindef::TRUE; - use winapi::shared::minwindef::UINT; - use winapi::shared::minwindef::WORD; - use winapi::shared::ntdef::WCHAR; - use winapi::um::processenv::GetStdHandle; - use winapi::um::winbase::STD_INPUT_HANDLE; - use winapi::um::wincon::FlushConsoleInputBuffer; - use winapi::um::wincon::PeekConsoleInputW; - use winapi::um::wincon::WriteConsoleInputW; - use winapi::um::wincontypes::INPUT_RECORD; - use winapi::um::wincontypes::KEY_EVENT; - use winapi::um::winnt::HANDLE; - use winapi::um::winuser::MapVirtualKeyW; - use winapi::um::winuser::MAPVK_VK_TO_VSC; - use winapi::um::winuser::VK_RETURN; - - // SAFETY: winapi calls - unsafe { - let stdin = GetStdHandle(STD_INPUT_HANDLE); - // emulate an enter key press to clear any line buffered console characters - emulate_enter_key_press(stdin)?; - // read the buffered line or enter key press - read_stdin_line(stdin_lock)?; - // check if our emulated key press was executed - if is_input_buffer_empty(stdin)? { - // if so, move the cursor up to prevent a blank line - move_cursor_up(stderr_lock)?; - } else { - // the emulated key press is still pending, so a buffered line was read - // and we can flush the emulated key press - flush_input_buffer(stdin)?; - } - } - - return Ok(()); - - unsafe fn flush_input_buffer(stdin: HANDLE) -> Result<(), AnyError> { - let success = FlushConsoleInputBuffer(stdin); - if success != TRUE { - bail!( - "Could not flush the console input buffer: {}", - std::io::Error::last_os_error() - ) - } - Ok(()) - } - - unsafe fn emulate_enter_key_press(stdin: HANDLE) -> Result<(), AnyError> { - // https://github.com/libuv/libuv/blob/a39009a5a9252a566ca0704d02df8dabc4ce328f/src/win/tty.c#L1121-L1131 - let mut input_record: INPUT_RECORD = std::mem::zeroed(); - input_record.EventType = KEY_EVENT; - input_record.Event.KeyEvent_mut().bKeyDown = TRUE; - input_record.Event.KeyEvent_mut().wRepeatCount = 1; - 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; - - let mut record_written = 0; - let success = - WriteConsoleInputW(stdin, &input_record, 1, &mut record_written); - if success != TRUE { - bail!( - "Could not emulate enter key press: {}", - std::io::Error::last_os_error() - ); - } - Ok(()) - } - - unsafe fn is_input_buffer_empty(stdin: HANDLE) -> Result { - let mut buffer = Vec::with_capacity(1); - let mut events_read = 0; - let success = - PeekConsoleInputW(stdin, buffer.as_mut_ptr(), 1, &mut events_read); - if success != TRUE { - bail!( - "Could not peek the console input buffer: {}", - std::io::Error::last_os_error() - ) - } - Ok(events_read == 0) - } - - fn move_cursor_up(stderr_lock: &mut StderrLock) -> Result<(), AnyError> { - write!(stderr_lock, "\x1B[1A")?; - Ok(()) - } - - fn read_stdin_line(stdin_lock: &mut StdinLock) -> Result<(), AnyError> { - let mut input = String::new(); - stdin_lock.read_line(&mut input)?; - Ok(()) - } - } - - // Clear n-lines in terminal and move cursor to the beginning of the line. - fn clear_n_lines(stderr_lock: &mut StderrLock, n: usize) { - write!(stderr_lock, "\x1B[{n}A\x1B[0J").unwrap(); - } + let metadata_before = get_stdin_metadata().unwrap(); // Lock stdio streams, so no other output is written while the prompt is // displayed. @@ -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 } }