From 01084938a14b2a52aa90a829b712dbb1e5f21ce1 Mon Sep 17 00:00:00 2001 From: Chet Ramey Date: Mon, 13 Mar 2023 17:20:51 -0400 Subject: [PATCH] script read errors are now fatal errors; fixes for readline asan issues --- CWRU/CWRU.chlog | 61 +++++++++++++++++++++++++++++++++++++++ config-top.h | 5 ++-- input.c | 9 ++++++ input.h | 1 + lib/readline/display.c | 6 +++- lib/readline/histexpand.c | 11 ++++++- lib/readline/isearch.c | 6 ++-- lib/readline/kill.c | 14 ++++----- lib/readline/vi_mode.c | 35 ++++++++++++++++++---- parse.y | 21 +++++--------- 10 files changed, 135 insertions(+), 34 deletions(-) diff --git a/CWRU/CWRU.chlog b/CWRU/CWRU.chlog index b3ea96aa..28d09a32 100644 --- a/CWRU/CWRU.chlog +++ b/CWRU/CWRU.chlog @@ -5571,3 +5571,64 @@ jobs.c the foreground (J_FOREGROUND) as it would be if it had been continued in the foreground with `fg'; if it is, we want to give the terminal back to shell_pgrp + + 3/10 + ---- +input.c + - fd_berror: returns true if the file descriptor passed as an arg + corresponds to a buffered stream with an error condition + +parse.y + - shell_getc: use fd_berror instead of inline code + - shell_getc: if yy_getc returns EOF, immediately set line[0] to NULL + and set shell_input_line_terminator to READERR. Then exit immediately + with status 128 if index == 0, line[index] == NULL, and READERR. + Posix interp 1629 + +config-top.h + - FATAL_READERROR: now defined to 1 by default + +lib/readline/display.c + - rl_expand_prompt: allocate an empty string for local_prompt if prompt + is the NULL or empty string. Fixes seg fault with rl_message and + empty prompt reported by Grisha Levit + +lib/readline/histexpand.c + - history_expand: if hist_extract_single_quoted returns an index at or + past the end of the string, correct the index and break the loop, + which will cause a return. Report and fix from + Grisha Levit + + 3/13 + ---- +lib/readline/vi_mode.c + - rl_domove_motion_callback: set the MOVE_FAILED flag in the context M + if rl_dispatch returns non-zero and other movement-command-specific + conditions are met. Right now, an unsuccessful `f' or `F' command + is failure. + - _rl_vi_domove_motion_cleanup: if the command is `c' or `C', we don't + enter insert mode if the MOVE_FAILED flag is set in the context M. + This is how standalone vi works. + +lib/readline/kill.c + - rl_unix_filename_rubout: add some checks for rl_point > 0 before + looking at rl_line_buffer[rl_point - 1]. Report and fix from + Grisha Levit + +lib/readline/isearch.c + - _rl_isearch_dispatch: reset cxt->sline_index to 0 if it's past the + end of the history string (sline_len). Fixes asan bug reported by + Grisha Levit + +lib/readline/histexpand.c + - history_tokenize_word: don't increment the string index past the + NULL if the line ends in a backslash.Fixes asan bug reported by + Grisha Levit + +lib/readline/vi_mode.c + - rl_vi_yank_to,rl_vi_change_to,rl_vi_delete_to: if these are called + with an existing valid vimvcxt, save it and allocate a new one for + the current function call to use, then restore it before returning. + This prevents referencing vimvcxt after freeing it in the case that + these functions call each other recursively (e.g., y1y1, c1d1, etc.). + Fixes asan bug reported by Grisha Levit diff --git a/config-top.h b/config-top.h index f2ebc671..5a322706 100644 --- a/config-top.h +++ b/config-top.h @@ -196,8 +196,9 @@ #define ASSOC_KVPAIR_ASSIGNMENT 1 /* Define if you want read errors in non-interactive shells to be fatal - errors instead of the historical practice of treating them as EOF. */ -/* #define FATAL_READERROR 1 */ + errors instead of the historical practice of treating them as EOF. The + next version of POSIX will require this (interp 1629). */ +#define FATAL_READERROR 1 /* Define to 0 if you want the `patsub_replacement' shell option to be disabled by default. */ diff --git a/input.c b/input.c index 155fd369..f739de18 100644 --- a/input.c +++ b/input.c @@ -469,6 +469,15 @@ get_buffered_stream (int fd) return (buffers && fd < nbuffers) ? buffers[fd] : (BUFFERED_STREAM *)0; } +int +fd_berror (int fd) +{ + BUFFERED_STREAM *bp; + + bp = get_buffered_stream (default_buffered_input); + return (bp && berror (bp)); +} + /* Read a buffer full of characters from BP, a buffered stream. */ static int b_fill_buffer (BUFFERED_STREAM *bp) diff --git a/input.h b/input.h index a1fa7219..592b6cb9 100644 --- a/input.h +++ b/input.h @@ -122,6 +122,7 @@ extern void free_buffered_stream (BUFFERED_STREAM *); extern int close_buffered_stream (BUFFERED_STREAM *); extern int close_buffered_fd (int); extern int sync_buffered_stream (int); +extern int fd_berror (int); extern int buffered_getchar (void); extern int buffered_ungetchar (int); extern void with_input_from_buffered_stream (int, char *); diff --git a/lib/readline/display.c b/lib/readline/display.c index b67be305..ce9dacd8 100644 --- a/lib/readline/display.c +++ b/lib/readline/display.c @@ -620,7 +620,11 @@ rl_expand_prompt (char *prompt) local_prompt_invis_chars[0] = 0; if (prompt == 0 || *prompt == 0) - return (0); + { + local_prompt = xmalloc (sizeof (char)); + local_prompt[0] = '\0'; + return (0); + } p = strrchr (prompt, '\n'); if (p == 0) diff --git a/lib/readline/histexpand.c b/lib/readline/histexpand.c index 38a49531..25d962c2 100644 --- a/lib/readline/histexpand.c +++ b/lib/readline/histexpand.c @@ -984,6 +984,8 @@ history_expand (const char *hstring, char **output) squote = 0; if (string[i]) i++; + if (i >= l) + i = l; } for ( ; string[i]; i++) @@ -1054,6 +1056,11 @@ history_expand (const char *hstring, char **output) flag = (i > 0 && string[i - 1] == '$'); i++; hist_string_extract_single_quoted (string, &i, flag); + if (i >= l) + { + i = l; + break; + } } else if (history_quotes_inhibit_expansion && string[i] == '\\') { @@ -1064,7 +1071,7 @@ history_expand (const char *hstring, char **output) } } - + if (string[i] != history_expansion_char) { xfree (result); @@ -1563,6 +1570,8 @@ get_word: (delimiter != '"' || member (string[i], slashify_in_quotes))) { i++; + if (string[i] == 0) + break; continue; } diff --git a/lib/readline/isearch.c b/lib/readline/isearch.c index 89c93386..c99072fd 100644 --- a/lib/readline/isearch.c +++ b/lib/readline/isearch.c @@ -783,7 +783,7 @@ opcode_dispatch: else cxt->sline_index += cxt->direction; - if (cxt->sline_index < 0) + if (cxt->sline_index < 0 || cxt->sline_index > cxt->sline_len) { cxt->sline_index = 0; break; @@ -816,8 +816,8 @@ opcode_dispatch: if (cxt->sflags & SF_FAILED) { - /* XXX - reset sline_index if < 0 */ - if (cxt->sline_index < 0) + /* XXX - reset sline_index if < 0 or longer than the history line */ + if (cxt->sline_index < 0 || cxt->sline_index > cxt->sline_len) cxt->sline_index = 0; break; } diff --git a/lib/readline/kill.c b/lib/readline/kill.c index 659e57fd..1dfe3c57 100644 --- a/lib/readline/kill.c +++ b/lib/readline/kill.c @@ -348,15 +348,15 @@ rl_unix_filename_rubout (int count, int key) if (count <= 0) count = 1; - while (count--) + while (rl_point > 0 && count--) { c = rl_line_buffer[rl_point - 1]; /* First move backwards through whitespace */ while (rl_point && whitespace (c)) { - rl_point--; - c = rl_line_buffer[rl_point - 1]; + if (--rl_point) + c = rl_line_buffer[rl_point - 1]; } /* Consume one or more slashes. */ @@ -377,14 +377,14 @@ rl_unix_filename_rubout (int count, int key) while (rl_point && (whitespace (c) || c == '/')) { - rl_point--; - c = rl_line_buffer[rl_point - 1]; + if (--rl_point) + c = rl_line_buffer[rl_point - 1]; } while (rl_point && (whitespace (c) == 0) && c != '/') { - rl_point--; /* XXX - multibyte? */ - c = rl_line_buffer[rl_point - 1]; + if (--rl_point) /* XXX - multibyte? */ + c = rl_line_buffer[rl_point - 1]; } } diff --git a/lib/readline/vi_mode.c b/lib/readline/vi_mode.c index 96523c73..e85fd101 100644 --- a/lib/readline/vi_mode.c +++ b/lib/readline/vi_mode.c @@ -76,6 +76,10 @@ #define INCREMENT_POS(start) (start)++ #endif /* !HANDLE_MULTIBYTE */ +/* Flags for the motion context */ +#define MOVE_SUCCESS 0 +#define MOVE_FAILED 0x01 + /* This is global so other parts of the code can check whether the last command was a text modification command. */ int _rl_vi_last_command = 'i'; /* default `.' puts you in insert mode */ @@ -1152,7 +1156,7 @@ _rl_mvcxt_dispose (_rl_vimotion_cxt *m) static int rl_domove_motion_callback (_rl_vimotion_cxt *m) { - int c; + int c, r, opoint; _rl_vi_last_motion = c = m->motion; @@ -1162,8 +1166,15 @@ rl_domove_motion_callback (_rl_vimotion_cxt *m) rl_extend_line_buffer (rl_end + 1); rl_line_buffer[rl_end++] = ' '; rl_line_buffer[rl_end] = '\0'; + opoint = rl_point; - _rl_dispatch (c, _rl_keymap); + r = _rl_dispatch (c, _rl_keymap); + + /* Note in the context that the motion command failed. Right now we only do + this for unsuccessful searches (ones where _rl_dispatch returns non-zero + and point doesn't move). */ + if (r != 0 && rl_point == opoint && (c == 'f' || c == 'F')) + m->flags |= MOVE_FAILED; #if defined (READLINE_CALLBACKS) if (RL_ISSTATE (RL_STATE_CALLBACK)) @@ -1198,7 +1209,7 @@ _rl_vi_domove_motion_cleanup (int c, _rl_vimotion_cxt *m) { /* 'c' and 'C' enter insert mode after the delete even if the motion didn't delete anything, as long as the motion command is valid. */ - if (_rl_to_upper (m->key) == 'C' && _rl_vi_motion_command (c)) + if (_rl_to_upper (m->key) == 'C' && _rl_vi_motion_command (c) && (m->flags & MOVE_FAILED) == 0) return (vidomove_dispatch (m)); RL_UNSETSTATE (RL_STATE_VIMOTION); return (-1); @@ -1379,7 +1390,11 @@ rl_vi_delete_to (int count, int key) _rl_vimvcxt = _rl_mvcxt_alloc (VIM_DELETE, key); } else if (_rl_vimvcxt) - _rl_mvcxt_init (_rl_vimvcxt, VIM_DELETE, key); + { + /* are we being called recursively or by `y' or `c'? */ + savecxt = _rl_vimvcxt; + _rl_vimvcxt = _rl_mvcxt_alloc (VIM_DELETE, key); + } else _rl_vimvcxt = _rl_mvcxt_alloc (VIM_DELETE, key); @@ -1478,7 +1493,11 @@ rl_vi_change_to (int count, int key) _rl_vimvcxt = _rl_mvcxt_alloc (VIM_CHANGE, key); } else if (_rl_vimvcxt) - _rl_mvcxt_init (_rl_vimvcxt, VIM_CHANGE, key); + { + /* are we being called recursively or by `y' or `d'? */ + savecxt = _rl_vimvcxt; + _rl_vimvcxt = _rl_mvcxt_alloc (VIM_CHANGE, key); + } else _rl_vimvcxt = _rl_mvcxt_alloc (VIM_CHANGE, key); _rl_vimvcxt->start = rl_point; @@ -1557,7 +1576,11 @@ rl_vi_yank_to (int count, int key) _rl_vimvcxt = _rl_mvcxt_alloc (VIM_YANK, key); } else if (_rl_vimvcxt) - _rl_mvcxt_init (_rl_vimvcxt, VIM_YANK, key); + { + /* are we being called recursively or by `c' or `d'? */ + savecxt = _rl_vimvcxt; + _rl_vimvcxt = _rl_mvcxt_alloc (VIM_YANK, key); + } else _rl_vimvcxt = _rl_mvcxt_alloc (VIM_YANK, key); _rl_vimvcxt->start = rl_point; diff --git a/parse.y b/parse.y index b414265b..e3516e2d 100644 --- a/parse.y +++ b/parse.y @@ -2446,20 +2446,13 @@ shell_getc (int remove_quoted_newline) if (i == 0) shell_input_line_terminator = EOF; - if (i == 0 && bash_input.type == st_bstream) - { - BUFFERED_STREAM *bp; - bp = get_buffered_stream (default_buffered_input); - if (bp && berror (bp)) - shell_input_line_terminator = READERR; - } - else - if (i == 0 && interactive_shell == 0 && bash_input.type == st_stream && ferror (stdin)) - shell_input_line_terminator = READERR; + if (bash_input.type == st_bstream && fd_berror (default_buffered_input)) + shell_input_line_terminator = READERR; + else if (interactive_shell == 0 && bash_input.type == st_stream && ferror (stdin)) + shell_input_line_terminator = READERR; - /* If we want to make read errors cancel execution of any partial - line, take out the checks for i == 0 above and set i = 0 if - shell_input_line_terminator == READERR. */ + /* We want to make read errors cancel execution of any partial + line, so we set i = 0 if shell_input_line_terminator == READERR. */ /* XXX TAG: bash-5.3 austingroup interp 1629 */ #if defined (FATAL_READERROR) if (shell_input_line_terminator == READERR) @@ -2715,7 +2708,7 @@ pop_alias: { #if defined (FATAL_READERROR) report_error (_("script file read error: %s"), strerror (errno)); - exit_shell (2); + exit_shell (128); /* POSIX mandated error status */ #else /* Treat read errors like EOF here. */ return ((shell_input_line_index != 0) ? '\n' : EOF);