diff --git a/CHANGELOG.rst b/CHANGELOG.rst index e1ac96ea7..6dcafd4b8 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -13,6 +13,10 @@ Scripting improvements Interactive improvements ------------------------ - Autosuggestions are now also provided in multi-line command lines. Like `ctrl-r`, autosuggestions operate only on the current line. +- New feature flag ``buffered-enter-noexec`` with the following effect: + when typing a command and :kbd:`enter` while the previous one is still running, the new one will no longer execute immediately. Similarly, keys that are bound to shell commands will be ignored. + This mitigates a security issue where a command like ``cat malicious-file.txt`` could write terminal escape codes prompting the terminal to write arbitrary text to fish's standard input. + Such a malicious file can still potentially insert arbitrary text into the command line but can no longer execute it directly (:issue:`10987`). New or improved bindings ^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/doc_src/language.rst b/doc_src/language.rst index 7f1cc37d8..5fbee421e 100644 --- a/doc_src/language.rst +++ b/doc_src/language.rst @@ -2024,6 +2024,7 @@ You can see the current list of features via ``status features``:: ampersand-nobg-in-token on 3.4 & only backgrounds if followed by a separating character remove-percent-self off 3.8 %self is no longer expanded (use $fish_pid) test-require-arg off 3.8 builtin test requires an argument + buffered-enter-noexec off 4.1 enter typed while executing will not execute Here is what they mean: @@ -2033,6 +2034,7 @@ Here is what they mean: - ``ampersand-nobg-in-token`` was introduced in fish 3.4. It makes it so a ``&`` i no longer interpreted as the backgrounding operator in the middle of a token, so dealing with URLs becomes easier. Either put spaces or a semicolon after the ``&``. This is recommended formatting anyway, and ``fish_indent`` will have done it for you already. - ``remove-percent-self`` turns off the special ``%self`` expansion. It was introduced in 3.8. To get fish's pid, you can use the :envvar:`fish_pid` variable. - ``test-require-arg`` removes :doc:`builtin test `'s one-argument form (``test "string"``. It was introduced in 3.8. To test if a string is non-empty, use ``test -n "string"``. If disabled, any call to ``test`` that would change sends a :ref:`debug message ` of category "deprecated-test", so starting fish with ``fish --debug=deprecated-test`` can be used to find offending calls. +- ``buffered-enter-noexec`` typing enter during command execution will insert a newline into the next commandline instead of executing it. These changes are introduced off by default. They can be enabled on a per session basis:: diff --git a/src/future_feature_flags.rs b/src/future_feature_flags.rs index 429baa8c9..33f9ff748 100644 --- a/src/future_feature_flags.rs +++ b/src/future_feature_flags.rs @@ -27,6 +27,9 @@ pub enum FeatureFlag { /// Remove `test`'s one and zero arg mode (make `test -n` return false etc) test_require_arg, + + /// Buffered enter (typed wile running a command) does not execute. + buffered_enter_noexec, } struct Features { @@ -107,6 +110,14 @@ pub struct FeatureMetadata { default_value: false, read_only: false, }, + FeatureMetadata { + flag: FeatureFlag::buffered_enter_noexec, + name: L!("buffered-enter-noexec"), + groups: L!("4.1"), + description: L!("enter typed while executing will not execute"), + default_value: false, + read_only: false, + }, ]; thread_local!( @@ -168,6 +179,7 @@ const fn new() -> Self { AtomicBool::new(METADATA[3].default_value), AtomicBool::new(METADATA[4].default_value), AtomicBool::new(METADATA[5].default_value), + AtomicBool::new(METADATA[6].default_value), ], } } diff --git a/src/input.rs b/src/input.rs index 1f011e2b7..9731f68ca 100644 --- a/src/input.rs +++ b/src/input.rs @@ -6,7 +6,7 @@ use crate::input_common::CursorPositionBlockingWait::MouseLeft; use crate::input_common::{ CharEvent, CharInputStyle, CursorPositionWait, ImplicitEvent, InputData, InputEventQueuer, - ReadlineCmd, R_END_INPUT_FUNCTIONS, + ReadlineCmd, ReadlineCmdEvent, READING_BUFFERED_INPUT, R_END_INPUT_FUNCTIONS, }; use crate::key::ViewportPosition; use crate::key::{self, canonicalize_raw_escapes, ctrl, Key, Modifiers}; @@ -886,6 +886,12 @@ fn mapping_execute(&mut self, m: &InputMapping) { } for cmd in m.commands.iter().rev() { let evt = match input_function_get_code(cmd) { + Some(ReadlineCmd::Execute) if READING_BUFFERED_INPUT.load() => { + CharEvent::Readline(ReadlineCmdEvent { + cmd: ReadlineCmd::SelfInsert, + seq: WString::from_chars([key::Enter]), + }) + } Some(code) => { self.function_push_args(code); // At this point, the sequence is only used for reinserting the keys into @@ -899,7 +905,12 @@ fn mapping_execute(&mut self, m: &InputMapping) { .collect(), ) } - None => CharEvent::Command(cmd.clone()), + None => { + if READING_BUFFERED_INPUT.load() { + continue; + } + CharEvent::Command(cmd.clone()) + } }; self.push_front(evt); } diff --git a/src/input_common.rs b/src/input_common.rs index e671a6f29..18c2edf62 100644 --- a/src/input_common.rs +++ b/src/input_common.rs @@ -291,6 +291,8 @@ pub fn from_check_exit() -> CharEvent { const WAIT_ON_SEQUENCE_KEY_INFINITE: usize = usize::MAX; static WAIT_ON_SEQUENCE_KEY_MS: AtomicUsize = AtomicUsize::new(WAIT_ON_SEQUENCE_KEY_INFINITE); +pub(crate) static READING_BUFFERED_INPUT: RelaxedAtomicBool = RelaxedAtomicBool::new(false); + /// Internal function used by readch to read one byte. /// This calls select() on three fds: input (e.g. stdin), the ioport notifier fd (for main thread /// requests), and the uvar notifier. This returns either the byte which was read, or one of the @@ -333,7 +335,7 @@ fn readb(in_fd: RawFd, blocking: bool) -> ReadbResult { } // Here's where we call select(). - let select_res = fdset.check_readable(if blocking { + let select_res = fdset.check_readable(if blocking && !READING_BUFFERED_INPUT.load() { Timeout::Forever } else { Timeout::ZERO @@ -372,6 +374,8 @@ fn readb(in_fd: RawFd, blocking: bool) -> ReadbResult { FLOG!(reader, "Read byte", char_to_symbol(char::from(c))); // The common path is to return a u8. return ReadbResult::Byte(c); + } else { + READING_BUFFERED_INPUT.store(false); } if !blocking { return ReadbResult::NothingToRead; diff --git a/src/reader.rs b/src/reader.rs index 0ac22f4d2..7c974deb4 100644 --- a/src/reader.rs +++ b/src/reader.rs @@ -68,6 +68,8 @@ use crate::flog::{FLOG, FLOGF}; #[allow(unused_imports)] use crate::future::IsSomeAnd; +use crate::future_feature_flags::feature_test; +use crate::future_feature_flags::FeatureFlag; use crate::global_safety::RelaxedAtomicBool; use crate::highlight::{ autosuggest_validate_from_history, highlight_shell, HighlightRole, HighlightSpec, @@ -77,7 +79,6 @@ SearchType, }; use crate::input::init_input; -use crate::input_common::terminal_protocols_disable_ifn; use crate::input_common::CursorPositionBlockingWait; use crate::input_common::CursorPositionWait; use crate::input_common::ImplicitEvent; @@ -88,6 +89,7 @@ terminal_protocol_hacks, terminal_protocols_enable_ifn, CharEvent, CharInputStyle, InputData, ReadlineCmd, }; +use crate::input_common::{terminal_protocols_disable_ifn, READING_BUFFERED_INPUT}; use crate::input_common::{CURSOR_UP_SUPPORTED, SCROLL_FORWARD_SUPPORTED}; use crate::io::IoChain; use crate::key::ViewportPosition; @@ -4111,6 +4113,9 @@ fn term_steal(copy_modes: bool) { break; } } + if feature_test(FeatureFlag::buffered_enter_noexec) { + READING_BUFFERED_INPUT.store(true); + } termsize_invalidate_tty(); } diff --git a/tests/checks/status.fish b/tests/checks/status.fish index 0a1089fae..e64155fec 100644 --- a/tests/checks/status.fish +++ b/tests/checks/status.fish @@ -59,6 +59,7 @@ status features #CHECK: ampersand-nobg-in-token on 3.4 & only backgrounds if followed by a separator #CHECK: remove-percent-self off 3.8 %self is no longer expanded (use $fish_pid) #CHECK: test-require-arg off 3.8 builtin test requires an argument +#CHECK: buffered-enter-noexec off 4.1 enter typed while executing will not execute status test-feature stderr-nocaret echo $status #CHECK: 0