Prevent redirecting internal processes to file descriptors above 2

The user may write for example:

    echo foo >&5

and fish would try to output to file descriptor 5, within the fish process
itself. This has unpredictable effects and isn't useful. Make this an
error.

Note that the reverse is "allowed" but ignored:

    echo foo 5>&1

this conceptually dup2s stdout to fd 5, but since no builtin writes to fd
5 we ignore it.

CHANGELOG.rst

@@ -185,6 +185,7 @@ Scripting improvements
 -  ``fish --profile`` now only starts profiling after fish is ready to execute commands (all configuration is completed). There is a new ``--profile-startup`` option that only profiles the startup and configuration process (:issue:`7648`).
 -  Builtins return a maximum exit status of 255, rather than potentially overflowing. In particular, this affects ``exit``, ``return``, ``functions --query``, and ``set --query`` (:issue:`7698`, :issue:`7702`).
 - It is no longer an error to run builtin with closed stdin. For example ``count <&-`` now prints 0, instead of failing.
+- Blocks, functions, and builtins no longer permit redirecting to fds above 2. For example ``echo hello >&5`` is now an error. External commands may redirect to any fd below 10.
 
 
 Interactive improvements

doc_src/index.rst

@@ -269,6 +269,8 @@ Any arbitrary file descriptor can used in a redirection by prefixing the redirec
 
 For example, ``echo hello 2> output.stderr`` writes the standard error (file descriptor 2) to ``output.stderr``.
 
+It is an error to redirect a builtin, function, or block to a file descriptor above 2. However this is supported for external commands.
+
 .. [#] Previous versions of fish also allowed specifying this as ``^DESTINATION``, but that made another character special so it was deprecated and will be removed in the future. See :ref:`feature flags<featureflags>`.
 
 .. _pipes:

src/parse_execution.cpp

@@ -900,14 +900,19 @@ end_execution_reason_t parse_execution_context_t::populate_plain_process(
             return arg_result;
         }
 
+        // Determine the process type.
+        process_type = process_type_for_command(statement, cmd);
+
+        // Only external commands and exec may redirect arbitrary fds.
+        bool allow_high_fds =
+            (process_type == process_type_t::external || process_type == process_type_t::exec);
+
         // The set of IO redirections that we construct for the process.
-        auto reason = this->determine_redirections(statement.args_or_redirs, &redirections);
+        auto reason =
+            this->determine_redirections(statement.args_or_redirs, allow_high_fds, &redirections);
         if (reason != end_execution_reason_t::ok) {
             return reason;
         }
-
-        // Determine the process type.
-        process_type = process_type_for_command(statement, cmd);
     }
 
     // Populate the process.
@@ -980,7 +985,8 @@ end_execution_reason_t parse_execution_context_t::expand_arguments_from_nodes(
 }
 
 end_execution_reason_t parse_execution_context_t::determine_redirections(
-    const ast::argument_or_redirection_list_t &list, redirection_spec_list_t *out_redirections) {
+    const ast::argument_or_redirection_list_t &list, bool allow_high_fds,
+    redirection_spec_list_t *out_redirections) {
     // Get all redirection nodes underneath the statement.
     for (const ast::argument_or_redirection_t &arg_or_redir : list) {
         if (!arg_or_redir.is_redirection()) continue;
@@ -1008,10 +1014,18 @@ end_execution_reason_t parse_execution_context_t::determine_redirections(
         redirection_spec_t spec{oper->fd, oper->mode, std::move(target)};
 
         // Validate this spec.
-        if (spec.mode == redirection_mode_t::fd && !spec.is_close() && !spec.get_target_as_fd()) {
-            const wchar_t *fmt =
-                _(L"Requested redirection to '%ls', which is not a valid file descriptor");
-            return report_error(STATUS_INVALID_ARGS, redir_node, fmt, spec.target.c_str());
+        if (spec.mode == redirection_mode_t::fd && !spec.is_close()) {
+            maybe_t<int> target_fd = spec.get_target_as_fd();
+            if (!target_fd.has_value()) {
+                // Like `cmd >&gibberish`.
+                const wchar_t *fmt =
+                    _(L"Requested redirection to '%ls', which is not a valid file descriptor");
+                return report_error(STATUS_INVALID_ARGS, redir_node, fmt, spec.target.c_str());
+            } else if (*target_fd > STDERR_FILENO && !allow_high_fds) {
+                // Like `echo foo 2>&5`. Don't allow internal procs to write to internal fish fds.
+                const wchar_t *fmt = _(L"Redirection to fd %d is only valid for external commands");
+                return report_error(STATUS_INVALID_ARGS, redir_node, fmt, *target_fd);
+            }
         }
         out_redirections->push_back(std::move(spec));
 
@@ -1066,7 +1080,8 @@ end_execution_reason_t parse_execution_context_t::populate_block_process(
     assert(args_or_redirs && "Should have args_or_redirs");
 
     redirection_spec_list_t redirections;
-    auto reason = this->determine_redirections(*args_or_redirs, &redirections);
+    auto reason =
+        this->determine_redirections(*args_or_redirs, false /* no high fds */, &redirections);
     if (reason == end_execution_reason_t::ok) {
         proc->type = process_type_t::block_node;
         proc->block_node_source = pstree;

src/parse_execution.h

@@ -130,7 +130,11 @@ class parse_execution_context_t {
                                                        globspec_t glob_behavior);
 
     // Determines the list of redirections for a node.
+    // If \p allow_high_fds is false, then report an error for an fd redirection other than to
+    // in/out/err. This is set when constructing an internal process and prevents writing random
+    // data to internal fish fds.
     end_execution_reason_t determine_redirections(const ast::argument_or_redirection_list_t &list,
+                                                  bool allow_high_fds,
                                                   redirection_spec_list_t *out_redirections);
 
     end_execution_reason_t run_1_job(const ast::job_t &job, const block_t *associated_block);

tests/checks/fds.fish

@@ -23,6 +23,9 @@ $helper print_fds </dev/null
 $helper print_fds 3</dev/null
 # CHECK: 0 1 2 3
 
+$helper print_fds 5>&2
+# CHECK: 0 1 2 5
+
 # This attempts to trip a case where the file opened in fish
 # has the same fd as the redirection. In this case, the dup2
 # does not clear the CLO_EXEC bit.

tests/checks/redirect.fish

@@ -100,10 +100,35 @@ echo $status
 read abc <&-
 #CHECKERR: read: stdin is closed
 
+# This one should output nothing.
+echo derp >&-
 
+# Verify that builtins, blocks, and functions may not write to arbitrary fds.
+echo derp >&12
+#CHECKERR: {{.*}} Redirection to fd 12 is only valid for external commands
+#CHECKERR: echo derp >&12
+#CHECKERR:           ^
+
+begin
+    echo derp
+end <&42
+#CHECKERR: {{.*}} Redirection to fd 42 is only valid for external commands
+#CHECKERR: end <&42
+#CHECKERR:     ^
+
+outnerr 2>&7
+#CHECKERR: {{.*}} Redirection to fd 7 is only valid for external commands
+#CHECKERR: outnerr 2>&7
+#CHECKERR:          ^
+
+# Redirection to 0, 1, 2 is allowed. We don't test 0 since writing to stdin is weird and unpredictable.
+echo hooray1 >&1
+echo hooray2 >&2
+#CHECK: hooray1
+#CHECKERR: hooray2
 
 # "Verify that pipes don't conflict with fd redirections"
-# This code is very similar to eval. We go over a bunch of fads
+# This code is very similar to eval. We go over a bunch of fds
 # to make it likely that we will nominally conflict with a pipe
 # fish is supposed to detect this case and dup the pipe to something else
 echo "/bin/echo pipe 3 <&3 3<&-" | source 3<&0
@@ -113,9 +138,6 @@ echo "/bin/echo pipe 6 <&6 6<&-" | source 6<&0
 echo "/bin/echo pipe 7 <&7 7<&-" | source 7<&0
 echo "/bin/echo pipe 8 <&8 8<&-" | source 8<&0
 echo "/bin/echo pipe 9 <&9 9<&-" | source 9<&0
-echo "/bin/echo pipe 10 <&10 10<&-" | source 10<&0
-echo "/bin/echo pipe 11 <&11 11<&-" | source 11<&0
-echo "/bin/echo pipe 12 <&12 12<&-" | source 12<&0
 #CHECK: pipe 3
 #CHECK: pipe 4
 #CHECK: pipe 5
@@ -123,6 +145,3 @@ echo "/bin/echo pipe 12 <&12 12<&-" | source 12<&0
 #CHECK: pipe 7
 #CHECK: pipe 8
 #CHECK: pipe 9
-#CHECK: pipe 10
-#CHECK: pipe 11
-#CHECK: pipe 12