From 4b27601dfdc8afa44aa93fe4f34985079c0119d5 Mon Sep 17 00:00:00 2001 From: Chet Ramey Date: Tue, 30 Dec 2025 16:06:40 -0500 Subject: [PATCH] fixes for several bugs found via fuzzing: overflow in $'...' hex expansion; fix for accessing freed memory when reading input from stdin and using multiple redirections to stdin; fix for pointer aliasing problem after a syntax error while executing a command string --- CWRU/CWRU.chlog | 41 ++++++++++++++++++ MANIFEST | 1 + builtins/evalstring.c | 7 +-- examples/shellmath/README.md | 6 ++- execute_cmd.c | 4 ++ input.c | 84 ++++++++++++++++++++++++++++++++++-- lib/sh/strtrans.c | 13 +++++- tests/history.right | 36 ++++++++++++++++ tests/history.tests | 1 + tests/history10.sub | 53 +++++++++++++++++++++++ 10 files changed, 235 insertions(+), 11 deletions(-) create mode 100644 tests/history10.sub diff --git a/CWRU/CWRU.chlog b/CWRU/CWRU.chlog index 18463f6a7..15553f703 100644 --- a/CWRU/CWRU.chlog +++ b/CWRU/CWRU.chlog @@ -12406,3 +12406,44 @@ bashline.c doc/bash.1,doc/bashref.texi - fc: documented new -D option + + 12/19 + ----- +lib/sh/strtrans.c + - ansicstr: handle int overflow in \x{...} and break out of the loop + if we overflow + Fuzzing report from Александр Ушаков + + 12/23 + ----- +input.c + - duplicate_buffered_stream: if we are duplicating the current input + buffered stream, make sure to mark both the current and new + buffered streams as B_SHAREDBUF, since they both share the buffer + and underlying file descriptor + - sync_shared_buffers: new function, after syncing the underlying + file descriptor, if the buffered stream is marked B_SHAREDBUF, make + sure to update b_used and b_inputp on other (saved) buffered streams + - sync_buffered_stream: call sync_shared_buffers + - count_shared_buffers: return a count of how many other buffered + streams are sharing the same buffer with the buffered stream passed + as an argument + - unshare_shared_buffers: unset the B_SHAREDBUF flag on the other + buffered stream (the caller guarantees there is only 1) sharing a + buffer with the buffered stream passed as an argument + - save_bash_input,duplicate_buffered_stream,close_buffered_stream: + if the buffered stream we're going to free up is marked B_SHAREDBUF, + count the number of buffered streams sharing the buffer, and + if there is only one other buffered stream sharing it, unset the + B_SHAREDBUF flag on that buffered stream + + From a fuzzing report from Александр Ушаков + +builtins/evalstring.c + - parse_and_execute: save and restore currently_executing_command to + avoid pointer aliasing problems with local COMMAND variable + From a fuzzing report from Александр Ушаков + +execute_cmd.c + - execute_command_internal: set currently_executing_command to NULL + in places where we return from this function diff --git a/MANIFEST b/MANIFEST index 1e00729f8..3bf2fa240 100644 --- a/MANIFEST +++ b/MANIFEST @@ -1300,6 +1300,7 @@ tests/history6.sub f tests/history7.sub f tests/history8.sub f tests/history9.sub f +tests/history10.sub f tests/ifs.tests f tests/ifs.right f tests/ifs1.sub f diff --git a/builtins/evalstring.c b/builtins/evalstring.c index 31ece9ffe..b21cfba4d 100644 --- a/builtins/evalstring.c +++ b/builtins/evalstring.c @@ -412,6 +412,7 @@ parse_and_execute (char *string, const char *from_file, int flags) protects installed by the string we're evaluating, so it will undo the current function scope. */ dispose_command (command); + currently_executing_command = NULL; discard_unwind_frame ("pe_dispose"); } else @@ -491,6 +492,7 @@ parse_and_execute (char *string, const char *from_file, int flags) begin_unwind_frame ("pe_dispose"); add_unwind_protect (uw_dispose_fd_bitmap, bitmap); add_unwind_protect (uw_dispose_command, command); /* XXX */ + unwind_protect_pointer (currently_executing_command); global_command = (COMMAND *)NULL; @@ -567,9 +569,8 @@ INTERNAL_DEBUG(("parse_and_execute: calling cat_file, parse_and_execute_level = #endif last_result = execute_command_internal (command, 0, NO_PIPE, NO_PIPE, bitmap); - dispose_command (command); - dispose_fd_bitmap (bitmap); - discard_unwind_frame ("pe_dispose"); + + run_unwind_frame ("pe_dispose"); /* If the global value didn't change, we restore what we had. */ if (((subshell_environment & SUBSHELL_COMSUB) || executing_funsub) && local_alflag == expaliases_flag) diff --git a/examples/shellmath/README.md b/examples/shellmath/README.md index 1b4725611..2aa139893 100644 --- a/examples/shellmath/README.md +++ b/examples/shellmath/README.md @@ -1,5 +1,5 @@ # Shellmath -Introducing decimal arithmetic libraries for the Bash shell, because +Introducing floating-point arithmetic libraries for the Bash shell, because they said it couldn't be done... and because: . @@ -90,7 +90,7 @@ script is exercising the shellmath arithmetic subroutines 31 times.___) The comment header in `faster_e_demo.sh` explains the optimization and shows how to put this faster version to work for you. -## Runtime efficiency competitive with awk and bc +## Competitive with awk and bc in runtime efficiency The file `timingData.txt` captures the results of some timing experiments that compare `shellmath` against the GNU versions of the calculators `awk` and `bc`. The experiments exercised each of the arithmetic operations and captured the results in a shell variable. @@ -164,3 +164,5 @@ You can run your floating-point calculations directly in Bash! ## Please see also: [A short discussion on arbitrary precision and shellmath](https://github.com/clarity20/shellmath/wiki/Shellmath-and-arbitrary-precision-arithmetic) + +-- Michael Wood diff --git a/execute_cmd.c b/execute_cmd.c index c139b3e0e..34d10ba92 100644 --- a/execute_cmd.c +++ b/execute_cmd.c @@ -808,6 +808,7 @@ execute_command_internal (COMMAND *command, int asynchronous, int pipe_in, int p jump_to_top_level (ERREXIT); } + currently_executing_command = (COMMAND *)NULL; return (last_command_exit_value); } else @@ -816,6 +817,7 @@ execute_command_internal (COMMAND *command, int asynchronous, int pipe_in, int p run_pending_traps (); + currently_executing_command = (COMMAND *)NULL; /* Posix 2013 2.9.3.1: "the exit status of an asynchronous list shall be zero." */ last_command_exit_value = 0; @@ -903,6 +905,8 @@ execute_command_internal (COMMAND *command, int asynchronous, int pipe_in, int p jump_to_top_level (ERREXIT); } } + + currently_executing_command = (COMMAND *)NULL; return (last_command_exit_value); } diff --git a/input.c b/input.c index 5de6b7ca7..6d038f2c5 100644 --- a/input.c +++ b/input.c @@ -162,6 +162,10 @@ stream_setsize (size_t size) int bash_input_fd_changed; +static inline int count_shared_buffers (BUFFERED_STREAM *); +static inline void unshare_shared_buffers (BUFFERED_STREAM *); +static inline void sync_shared_buffers (BUFFERED_STREAM *);;;; + /* This provides a way to map from a file descriptor to the buffer associated with that file descriptor, rather than just the other way around. This is needed so that buffers are managed properly @@ -276,7 +280,13 @@ save_bash_input (int fd, int new_fd) descriptor? Free up the buffer and report the error. */ internal_error (_("save_bash_input: buffer already exists for new fd %d"), nfd); if (buffers[nfd]->b_flag & B_SHAREDBUF) - buffers[nfd]->b_buffer = (char *)NULL; + { + /* If this buffer is shared by only one other buffered stream, mark + it as no longer shared. */ + if (count_shared_buffers (buffers[nfd]) == 1) + unshare_shared_buffers (buffers[nfd]); + buffers[nfd]->b_buffer = (char *)NULL; + } free_buffered_stream (buffers[nfd]); } @@ -362,6 +372,10 @@ duplicate_buffered_stream (int fd1, int fd2) /* If this buffer is shared with another fd, don't free the buffer */ else if (buffers[fd2]->b_flag & B_SHAREDBUF) { + /* If this buffer is shared by only one other buffered stream, mark + it as no longer shared. */ + if (count_shared_buffers (buffers[fd2]) == 1) + unshare_shared_buffers (buffers[fd2]); buffers[fd2]->b_buffer = (char *)NULL; free_buffered_stream (buffers[fd2]); } @@ -380,7 +394,11 @@ duplicate_buffered_stream (int fd1, int fd2) } if (buffers[fd2] && (fd_is_bash_input (fd1) || (buffers[fd1] && (buffers[fd1]->b_flag & B_SHAREDBUF)))) - buffers[fd2]->b_flag |= B_SHAREDBUF; + { + /* Make sure both buffered streams are marked as sharing an input buffer */ + buffers[fd1]->b_flag |= B_SHAREDBUF; + buffers[fd2]->b_flag |= B_SHAREDBUF; + } return (fd2); } @@ -446,11 +464,17 @@ close_buffered_stream (BUFFERED_STREAM *bp) { int fd; - if (!bp) + if (bp == NULL) return (0); fd = bp->b_fd; if (bp->b_flag & B_SHAREDBUF) - bp->b_buffer = (char *)NULL; + { + /* If this buffer is shared by only one other buffered stream, mark + it as no longer shared. */ + if (count_shared_buffers (buffers[fd]) == 1) + unshare_shared_buffers (buffers[fd]); + bp->b_buffer = (char *)NULL; + } free_buffered_stream (bp); return (close (fd)); } @@ -581,6 +605,49 @@ bufstream_ungetc(int c, BUFFERED_STREAM *bp) return (c); } +/* Return the number of buffered streams sharing a buffer with BP */ +static inline int +count_shared_buffers (BUFFERED_STREAM *bp) +{ + size_t i; + int n; + + n = 0; + for (i = 0; i < nbuffers; i++) + if (buffers[i] && (buffers[i]->b_flag & B_SHAREDBUF) && buffers[i] != bp && buffers[i]->b_buffer == bp->b_buffer) + n++; + return n; +} + +/* Mark the buffered stream sharing a buffer with BP as no longer B_SHAREDBUF */ +static inline void +unshare_shared_buffers (BUFFERED_STREAM *bp) +{ + size_t i; + + for (i = 0; i < nbuffers; i++) + if (buffers[i] && (buffers[i]->b_flag & B_SHAREDBUF) && buffers[i] != bp && buffers[i]->b_buffer == bp->b_buffer) + { + buffers[i]->b_flag &= ~B_SHAREDBUF; + return; + } +} + +/* Sync the input and read offsets on all buffered streams that share a buffer + with BP */ +static inline void +sync_shared_buffers (BUFFERED_STREAM *bp) +{ + size_t i; + + for (i = 0; i < nbuffers; i++) + if (buffers[i] && (buffers[i]->b_flag & B_SHAREDBUF) && buffers[i] != bp && buffers[i]->b_buffer == bp->b_buffer) + { + buffers[i]->b_used = bp->b_used; + buffers[i]->b_inputp = bp->b_inputp; + } +} + /* Seek backwards on file BFD to synchronize what we've read so far with the underlying file pointer. */ int @@ -596,6 +663,15 @@ sync_buffered_stream (int bfd) if (chars_left) lseek (bp->b_fd, -chars_left, SEEK_CUR); bp->b_used = bp->b_inputp = 0; + + /* If we are sharing the buffer between buffered streams, we must have + duplicated the file descriptor and called duplicate_buffered_stream(). + In this case, the buffers share the underlying fd and all of them + need to be updated to force a read on the next call, since we've changed + the file offset. */ + if (bp->b_flag & B_SHAREDBUF) + sync_shared_buffers (bp); + return (0); } diff --git a/lib/sh/strtrans.c b/lib/sh/strtrans.c index e3d67b6a3..16ca16dd2 100644 --- a/lib/sh/strtrans.c +++ b/lib/sh/strtrans.c @@ -130,7 +130,7 @@ ansicstr (const char *string, size_t len, int flags, int *sawc, size_t *rlen) c &= 0xFF; break; case 'x': /* Hex digit -- non-ANSI */ - if ((flags & 2) && *s == '{') + if ((flags & 2) && *s == '{') /* undocumented */ { flags |= 16; /* internal flag value */ s++; @@ -143,8 +143,17 @@ ansicstr (const char *string, size_t len, int flags, int *sawc, size_t *rlen) chars are consumed. */ if (flags & 16) { + v = c; for ( ; ISXDIGIT ((unsigned char)*s); s++) - c = (c * 16) + HEXVALUE (*s); + { + v = (v * 16) + HEXVALUE (*s); + if (v > INT_MAX) /* abandon on overflow */ + { + v &= INT_MAX; + break; + } + } + c = v; flags &= ~16; if (*s == '}') s++; diff --git a/tests/history.right b/tests/history.right index 54ed23b08..235903bc1 100644 --- a/tests/history.right +++ b/tests/history.right @@ -403,3 +403,39 @@ EOF 5 echo three + history10.sub + 1 echo first + 2 echo one + 3 echo two + 4 echo three + 5 echo x + 6 echo y + 7 echo z + 8 echo last +echo one append +one append +echo two append +two append +echo three append +three append + 1 echo first + 2 echo x + 3 echo y + 4 echo z + 5 echo last + 6 echo one append + 7 echo two append + 8 echo three append +edit +echo edit append +edit append + 1 echo first + 2 echo x + 3 echo y + 4 echo z + 5 echo last + 6 echo one append + 7 echo two append + 8 echo three append + 9 # simulate readline edit_and_execute_command + 10 echo edit append diff --git a/tests/history.tests b/tests/history.tests index 213c75c4b..15c6b0108 100644 --- a/tests/history.tests +++ b/tests/history.tests @@ -137,3 +137,4 @@ test_runsub ./history6.sub test_runsub ./history7.sub test_runsub ./history8.sub test_runsub ./history9.sub +test_runsub ./history10.sub diff --git a/tests/history10.sub b/tests/history10.sub new file mode 100644 index 000000000..100971dba --- /dev/null +++ b/tests/history10.sub @@ -0,0 +1,53 @@ +# +# Copyright 2025 Free Software Foundation, Inc. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# + +# basic test for fc -D +: ${TMPDIR:=/tmp} + +stream() +{ + # something you can see + sed 's/$/ append/' < $1 >$TMPF && mv $TMPF $1 + rm -f $TMPF +} + +HISTFILE=$TMPDIR/fchist-$BASHPID +TMPF=$TMPDIR/fctmp-$BASHPID + +trap 'rm -f "$HISTFILE" "$TMPF"' 0 1 2 3 6 15 + +cat > $HISTFILE <