From 6aec4b85e6ced4b991cd4a6563ac47327ea9658b Mon Sep 17 00:00:00 2001 From: David Lawrence Ramsey Date: Mon, 15 Mar 2004 20:26:30 +0000 Subject: [PATCH] fix potential memory corruption problems caused by passing answer in as the value of def in statusq(), etc. git-svn-id: svn://svn.savannah.gnu.org/nano/trunk/nano@1695 35c25a1d-7b9e-4130-9fde-d3aeb78583b8 --- ChangeLog | 15 ++++++ src/files.c | 132 ++++++++++++++++++++++++++++----------------------- src/nano.c | 8 ++-- src/proto.h | 4 +- src/search.c | 27 ++++++----- src/utils.c | 9 ++-- src/winio.c | 10 ++-- 7 files changed, 117 insertions(+), 88 deletions(-) diff --git a/ChangeLog b/ChangeLog index 760d693a..f1e8ec01 100644 --- a/ChangeLog +++ b/ChangeLog @@ -31,6 +31,13 @@ CVS code - and use tail() to get the filename if get_full_path() fails. - Port to the Tandem NonStop Kernel (nsr-tandem-nsk). (Tom Bates; minor tweaks by DLR) + - Change some instances of boolean 0 and 1 to TRUE and FALSE. + (David Benbennick) + - Fix memory corruption problem occurring when answer is used as + the value of def; if the realloc() of answer leads to its + pointing to a different memory block, there will be a segfault + when the value of def is copied into it via strcpy(). (bort, + Christian Weisgarber, David Benbennick, and DLR) - files.c: do_insertfile() - Wrap one reference to NANO_EXTCMD_KEY in a NANO_SMALL #ifdef. @@ -45,6 +52,9 @@ CVS code - write_marked() - New function used to write the current marked selection to a file, split out from do_writeout(). (DLR) + do_writeout() + - Tweak for efficiency. (David Benbennick) DLR: Modify to have + the current answer preserved between toggles again. filestat() - Removed, as it is only called in one place and is redundant. (DLR) @@ -175,6 +185,9 @@ CVS code - strstrwrapper() - Refactor for increased efficiency, and eliminate the need for the line_pos parameter. (David Benbennick) + mallocstrcpy() + - Tweak so that when src is "", "" is allocated and returned + instead of NULL. (David Benbennick) - winio.c: get_verbatim_kbinput() - Set keypad() to FALSE and switch to raw mode while reading @@ -212,6 +225,8 @@ CVS code - - Tweak for efficiency. (David Benbennick) edit_refresh() - Tweak for efficiency. (David Benbennick) + statusq() + - Rename "tabs" to "allowtabs". (David Benbennick) do_credits() - Use nanosleep() instead of usleep(). The latter is only standard under BSD, whereas the former is POSIX compliant. diff --git a/src/files.c b/src/files.c index 35610dc3..f346f499 100644 --- a/src/files.c +++ b/src/files.c @@ -406,7 +406,7 @@ char *get_next_filename(const char *name) buf = charalloc(strlen(name) + num_of_digits(INT_MAX) + 2); strcpy(buf, name); - while (1) { + while (TRUE) { if (stat(buf, &fs) == -1) return buf; @@ -451,17 +451,17 @@ int do_insertfile(int loading_file) if (operating_dir != NULL && strcmp(operating_dir, ".")) #ifdef ENABLE_MULTIBUFFER if (ISSET(MULTIBUFFER)) - i = statusq(1, insertfile_list, inspath, + i = statusq(TRUE, insertfile_list, inspath, #ifndef NANO_SMALL - 0, + NULL, #endif _("File to insert into new buffer [from %s] "), operating_dir); else #endif - i = statusq(1, insertfile_list, inspath, + i = statusq(TRUE, insertfile_list, inspath, #ifndef NANO_SMALL - 0, + NULL, #endif _("File to insert [from %s] "), operating_dir); @@ -470,16 +470,16 @@ int do_insertfile(int loading_file) #endif #ifdef ENABLE_MULTIBUFFER if (ISSET(MULTIBUFFER)) - i = statusq(1, insertfile_list, inspath, + i = statusq(TRUE, insertfile_list, inspath, #ifndef NANO_SMALL - 0, + NULL, #endif _("File to insert into new buffer [from ./] ")); else #endif /* ENABLE_MULTIBUFFER */ - i = statusq(1, insertfile_list, inspath, + i = statusq(TRUE, insertfile_list, inspath, #ifndef NANO_SMALL - 0, + NULL, #endif _("File to insert [from ./] ")); @@ -501,8 +501,12 @@ int do_insertfile(int loading_file) #endif /* ENABLE_MULTIBUFFER */ if (i == NANO_EXTCMD_KEY) { - int ts = statusq(TRUE, extcmd_list, answer, NULL, + char *ans = mallocstrcpy(NULL, answer); + int ts = statusq(TRUE, extcmd_list, ans, NULL, _("Command to execute")); + + free(ans); + if (ts == -1 || answer == NULL || answer[0] == '\0') { statusbar(_("Cancelled")); display_main_list(); @@ -1802,35 +1806,50 @@ int write_marked(const char *name, int tmp, int append, int } #endif /* !NANO_SMALL */ -int do_writeout(const char *path, int exiting, int append) +int do_writeout(int exiting) { - int i = 0; + int i; + int append = 0; #ifdef NANO_EXTRA - static int did_cred = 0; + static int did_cred = FALSE; #endif #if !defined(DISABLE_BROWSER) || !defined(DISABLE_MOUSE) currshortcut = writefile_list; #endif - answer = mallocstrcpy(answer, path); - if (exiting && ISSET(TEMP_OPT)) { + i = -1; if (filename[0] != '\0') { - i = write_file(answer, 0, 0, 0); - display_main_list(); - return i; - } else { + i = write_file(filename, FALSE, 0, FALSE); + if (i == 1) { + /* Write succeeded. */ + display_main_list(); + return 1; + } + } + + /* No filename or the write above failed. */ + if (i == -1) { UNSET(TEMP_OPT); do_exit(); - /* They cancelled, abort quit */ + /* They cancelled; abort quit. */ return -1; } } - while (1) { #ifndef NANO_SMALL + if (ISSET(MARK_ISSET) && !exiting) + answer = mallocstrcpy(answer, ""); + else +#endif + answer = mallocstrcpy(answer, filename); + + while (TRUE) { + const char *msg; +#ifndef NANO_SMALL + char *ans = mallocstrcpy(NULL, answer); const char *formatstr, *backupstr; if (ISSET(MAC_FILE)) @@ -1845,44 +1864,39 @@ int do_writeout(const char *path, int exiting, int append) else backupstr = ""; - /* Be nice to the translation folks */ + /* Be nice to the translation folks. */ if (ISSET(MARK_ISSET) && !exiting) { if (append == 2) - i = statusq(1, writefile_list, "", 0, - "%s%s%s", _("Prepend Selection to File"), formatstr, backupstr); + msg = _("Prepend Selection to File"); else if (append == 1) - i = statusq(1, writefile_list, "", 0, - "%s%s%s", _("Append Selection to File"), formatstr, backupstr); + msg = _("Append Selection to File"); else - i = statusq(1, writefile_list, "", 0, - "%s%s%s", _("Write Selection to File"), formatstr, backupstr); - } else { - if (append == 2) - i = statusq(1, writefile_list, answer, 0, - "%s%s%s", _("File Name to Prepend to"), formatstr, backupstr); - else if (append == 1) - i = statusq(1, writefile_list, answer, 0, - "%s%s%s", _("File Name to Append to"), formatstr, backupstr); - else - i = statusq(1, writefile_list, answer, 0, - "%s%s%s", _("File Name to Write"), formatstr, backupstr); - } -#else + msg = _("Write Selection to File"); + } else +#endif /* !NANO_SMALL */ if (append == 2) - i = statusq(1, writefile_list, answer, - "%s", _("File Name to Prepend to")); + msg = _("File Name to Prepend to"); else if (append == 1) - i = statusq(1, writefile_list, answer, - "%s", _("File Name to Append to")); + msg = _("File Name to Append to"); else - i = statusq(1, writefile_list, answer, - "%s", _("File Name to Write")); -#endif /* !NANO_SMALL */ + msg = _("File Name to Write"); + + i = statusq(TRUE, writefile_list, +#ifndef NANO_SMALL + ans, NULL, "%s%s%s", msg, formatstr, backupstr +#else + filename, "%s", msg +#endif + ); + +#ifndef NANO_SMALL + free(ans); +#endif if (i == -1) { statusbar(_("Cancelled")); display_main_list(); - return 0; + return -1; } #ifndef DISABLE_BROWSER @@ -1926,7 +1940,7 @@ int do_writeout(const char *path, int exiting, int append) if (exiting && !ISSET(TEMP_OPT) && !strcasecmp(answer, "zzy") && !did_cred) { do_credits(); - did_cred = 1; + did_cred = TRUE; return -1; } #endif @@ -1934,7 +1948,7 @@ int do_writeout(const char *path, int exiting, int append) struct stat st; if (!stat(answer, &st)) { - i = do_yesno(0, _("File exists, OVERWRITE ?")); + i = do_yesno(FALSE, _("File exists, OVERWRITE ?")); if (i == 0 || i == -1) continue; } else if (filename[0] != '\0' @@ -1942,7 +1956,7 @@ int do_writeout(const char *path, int exiting, int append) && (!ISSET(MARK_ISSET) || exiting) #endif ) { - i = do_yesno(0, _("Save file under DIFFERENT NAME ?")); + i = do_yesno(FALSE, _("Save file under DIFFERENT NAME ?")); if (i == 0 || i == -1) continue; } @@ -1952,25 +1966,25 @@ int do_writeout(const char *path, int exiting, int append) /* Here's where we allow the selected text to be written to * a separate file. */ if (ISSET(MARK_ISSET) && !exiting) - i = write_marked(answer, 0, append, 1); + i = write_marked(answer, FALSE, append, FALSE); else #endif /* !NANO_SMALL */ - i = write_file(answer, 0, append, 0); + i = write_file(answer, FALSE, append, FALSE); #ifdef ENABLE_MULTIBUFFER /* If we're not about to exit, update the current entry in - the open_files structure. */ + * the open_files structure. */ if (!exiting) add_open_file(1); #endif display_main_list(); return i; - } /* while (1) */ + } /* while (TRUE) */ } int do_writeout_void(void) { - return do_writeout(filename, 0, 0); + return do_writeout(FALSE); } /* Return a malloc()ed string containing the actual directory, used @@ -2338,7 +2352,7 @@ char *input_tab(char *buf, int place, int *lastwastab, int *newplace, int *list) pos <= strlen(matches[0]); pos++) tmp++; - while (1) { + while (TRUE) { match_matches = 0; for (i = 0; i < num_matches; i++) { @@ -2752,9 +2766,9 @@ char *do_browser(const char *inpath) case 'G': /* Pico compatibility */ case 'g': curs_set(1); - j = statusq(0, gotodir_list, "", + j = statusq(FALSE, gotodir_list, "", #ifndef NANO_SMALL - 0, + NULL, #endif _("Goto Directory")); bottombars(browser_list); diff --git a/src/nano.c b/src/nano.c index 01a69bf2..268d473e 100644 --- a/src/nano.c +++ b/src/nano.c @@ -1511,7 +1511,7 @@ int do_int_spell_fix(const char *word) do_replace_highlight(TRUE, word); /* Allow the replace word to be corrected. */ - i = statusq(0, spell_list, word, + i = statusq(FALSE, spell_list, word, #ifndef NANO_SMALL NULL, #endif @@ -2739,14 +2739,14 @@ int do_exit(void) if (ISSET(TEMP_OPT)) i = 1; else - i = do_yesno(0, _("Save modified buffer (ANSWERING \"No\" WILL DESTROY CHANGES) ? ")); + i = do_yesno(FALSE, _("Save modified buffer (ANSWERING \"No\" WILL DESTROY CHANGES) ? ")); #ifdef DEBUG dump_buffer(fileage); #endif if (i == 1) { - if (do_writeout(filename, 1, 0) > 0) { + if (do_writeout(TRUE) > 0) { #ifdef ENABLE_MULTIBUFFER if (!close_open_file()) { @@ -3528,7 +3528,7 @@ int main(int argc, char *argv[]) edit_refresh(); reset_cursor(); - while (1) { + while (TRUE) { keyhandled = 0; if (ISSET(CONSTUPDATE)) diff --git a/src/proto.h b/src/proto.h index 83697ffc..d0cb35a3 100644 --- a/src/proto.h +++ b/src/proto.h @@ -190,7 +190,7 @@ int write_file(const char *name, int tmp, int append, int nonamechange); int write_marked(const char *name, int tmp, int append, int nonamechange); #endif -int do_writeout(const char *path, int exiting, int append); +int do_writeout(int exiting); int do_writeout_void(void); char *real_dir_from_tilde(const char *buf); #ifndef DISABLE_TABCOMP @@ -503,7 +503,7 @@ void center_cursor(void); void edit_refresh(void); void edit_refresh_clearok(void); void edit_update(filestruct *fileptr, topmidnone location); -int statusq(int tabs, const shortcut *s, const char *def, +int statusq(int allowtabs, const shortcut *s, const char *def, #ifndef NANO_SMALL historyheadtype *history_list, #endif diff --git a/src/search.c b/src/search.c index 020eb4db..7575d584 100644 --- a/src/search.c +++ b/src/search.c @@ -136,12 +136,12 @@ int search_init(int replacing) } /* This is now one simple call. It just does a lot. */ - i = statusq(0, replacing ? replace_list : whereis_list, backupstring, + i = statusq(FALSE, replacing ? replace_list : whereis_list, + backupstring, #ifndef NANO_SMALL &search_history, #endif - "%s%s%s%s%s%s", - _("Search"), + "%s%s%s%s%s%s", _("Search"), #ifndef NANO_SMALL /* This string is just a modifier for the search prompt; no @@ -275,7 +275,7 @@ int findnextstr(int can_display_wrap, int wholeword, const filestruct rev_start = fileptr->data + (current_x + 1); /* Look for needle in searchstr. */ - while (1) { + while (TRUE) { found = strstrwrapper(fileptr->data, needle, rev_start); if (found != NULL && (!wholeword || is_whole_word(found - @@ -645,7 +645,7 @@ int do_replace_loop(const char *needle, const filestruct *real_current, curs_set(0); do_replace_highlight(TRUE, exp_word); - i = do_yesno(1, _("Replace this instance?")); + i = do_yesno(TRUE, _("Replace this instance?")); do_replace_highlight(FALSE, exp_word); free(exp_word); @@ -763,11 +763,11 @@ int do_replace(void) last_replace = mallocstrcpy(last_replace, ""); #endif - i = statusq(0, replace_list_2, last_replace, + i = statusq(FALSE, replace_list_2, last_replace, #ifndef NANO_SMALL - &replace_history, + &replace_history, #endif - _("Replace with")); + _("Replace with")); #ifndef NANO_SMALL /* Add this replace string to the replace history list. i == 0 @@ -813,11 +813,14 @@ int do_replace(void) int do_gotoline(int line, int save_pos) { if (line <= 0) { /* Ask for it */ - int st = statusq(FALSE, goto_list, line != 0 ? answer : "", + char *ans = mallocstrcpy(NULL, answer); + int st = statusq(FALSE, goto_list, line != 0 ? ans : "", #ifndef NANO_SMALL - NULL, + NULL, #endif - _("Enter line number")); + _("Enter line number")); + + free(ans); /* Cancel, or Enter with blank string. */ if (st == -1 || st == -2) @@ -918,7 +921,7 @@ int do_find_bracket(void) assert(ISSET(REGEXP_COMPILED)); search_last_line = 0; - while (1) { + while (TRUE) { if (findnextstr(FALSE, FALSE, current, current_x, regexp_pat, FALSE) != 0) { /* Found identical bracket. */ if (current->data[current_x] == ch_under_cursor) diff --git a/src/utils.c b/src/utils.c index e18a7802..5110fdb1 100644 --- a/src/utils.c +++ b/src/utils.c @@ -293,15 +293,12 @@ void *nrealloc(void *ptr, size_t howmuch) * dest = mallocstrcpy(dest, src); */ char *mallocstrcpy(char *dest, const char *src) { - if (src == dest) - return dest; + if (src == NULL) + src = ""; - if (dest != NULL) + if (src != dest) free(dest); - if (src == NULL) - return NULL; - dest = charalloc(strlen(src) + 1); strcpy(dest, src); diff --git a/src/winio.c b/src/winio.c index d2614e94..01484ef5 100644 --- a/src/winio.c +++ b/src/winio.c @@ -127,7 +127,7 @@ int get_ignored_kbinput(WINDOW *win) { int kbinput; - while (1) { + while (TRUE) { kbinput = wgetch(win); switch (kbinput) { case ERR: @@ -1065,7 +1065,7 @@ void nanoget_repaint(const char *buf, const char *inputbuf, size_t x) wattroff(bottomwin, A_REVERSE); } -/* Get the input from the kb; this should only be called from +/* Get the input from the keyboard; this should only be called from * statusq(). */ int nanogetstr(int allowtabs, const char *buf, const char *def, #ifndef NANO_SMALL @@ -1671,7 +1671,7 @@ void edit_add(const filestruct *fileptr, const char *converted, * line not followed by an end on this line? */ start_col = 0; - while (1) { + while (TRUE) { start_col += startmatch.rm_so; startmatch.rm_eo -= startmatch.rm_so; if (regexec(tmpcolor->end, @@ -2011,7 +2011,7 @@ void edit_update(filestruct *fileptr, topmidnone location) * want to put up by default. * * New arg tabs tells whether or not to allow tab completion. */ -int statusq(int tabs, const shortcut *s, const char *def, +int statusq(int allowtabs, const shortcut *s, const char *def, #ifndef NANO_SMALL historyheadtype *which_history, #endif @@ -2031,7 +2031,7 @@ int statusq(int tabs, const shortcut *s, const char *def, va_end(ap); foo[COLS - 4] = '\0'; - ret = nanogetstr(tabs, foo, def, + ret = nanogetstr(allowtabs, foo, def, #ifndef NANO_SMALL which_history, #endif -- 2.39.5