Description of Problem: Hampton beta2: Running mc as user - in console - <ctrl-o> is not working properly. Version-Release number of selected component (if applicable): mc-4.5.51-36 How Reproducible: every time Steps to Reproduce: 1. Running "mc" from console as a user. 2. <ctrl-o> to hide mc - works so far. 3. <ctrl-o> to get mc again Actual Results: Can't get "mc" back again, and it is not possible to type anything. Expected Results: Switch "mc" back with <ctrl-o>, and be able to type letters in console. Additional Information: It seems to work as "root", and with "mc" in terminal in X (KDE).
Patch from Jakub: create group vcs make /dev/vcs* root.vcs 660 make /usr/lib/mc/bin/cons.saver root.vcs 2111 Before doing that, I think mc/src/cons.saver.c should be trimmed down a little bit, particularly by removing compatibility with pre-/dev/vcs* kernels (console_flag={1,2}) and before doing send_console or restore_console and after save_console, check again if the corresponding /dev/ttyN is still owned by the user who started cons.saver. Plus at least 2-3 people should read the code once edited to look at possible security bugs. Here is a rought patch (untested) which trims down cons.saver and adds the tty owner checks: --- mc-4.5.51/src/cons.saver.c.jj Mon Jul 3 10:32:11 2000 +++ mc-4.5.51/src/cons.saver.c Tue Oct 9 05:00:41 2001 @@ -63,46 +63,31 @@ static signed char console_flag = -1; */ static int console_fd = -1; static char *tty_name; -static int len; +static size_t len;Looking at mc cons.saver, I think the best would be: create group vcs make /dev/vcs* root.vcs 660 make /usr/lib/mc/bin/cons.saver root.vcs 2111 Before doing that, I think mc/src/cons.saver.c should be trimmed down a little bit, particularly by removing compatibility with pre-/dev/vcs* kernels (console_flag={1,2}) and before doing send_console or restore_console and after save_console, check again if the corresponding /dev/ttyN is still owned by the user who started cons.saver. Plus at least 2-3 people should read the code once edited to look at possible security bugs. Here is a rought patch (untested) which trims down cons.saver and adds the tty owner checks: --- mc-4.5.51/src/cons.saver.c.jj Mon Jul 3 10:32:11 2000 +++ mc-4.5.51/src/cons.saver.c Tue Oct 9 05:00:41 2001 @@ -63,46 +63,31 @@ static signed char console_flag = -1; */ static int console_fd = -1; static char *tty_name; -static int len; +static size_t len; static char *buffer = NULL; static int buffer_size = 0; static int columns, rows; static char vcs_name [128]; static int vcs_fd; -static void dwrite (int fd, char *buffer) -{ - write (fd, buffer, strlen (buffer)); -} - static void tty_getsize () { struct winsize winsz; - + winsz.ws_col = winsz.ws_row = 0; - ioctl (console_fd, TIOCGWINSZ, &winsz); - if (winsz.ws_col && winsz.ws_row){ + if (ioctl (console_fd, TIOCGWINSZ, &winsz) >= 0 + && winsz.ws_col && winsz.ws_row){ columns = winsz.ws_col; rows = winsz.ws_row; } else { /* Never happens (I think) */ - dwrite (2, "TIOCGWINSZ failed\n"); columns = 80; rows = 25; console_flag = 0; } } -inline void tty_cursormove(int y, int x) -{ - char buffer [128]; - - /* Standard ANSI escape sequence for cursor positioning */ - snprintf (buffer, sizeof (buffer), "\33[%d;%dH", y + 1, x + 1); - dwrite (console_fd, buffer); -} - -int check_file (char *filename, int check_console, char **msg) +int check_file (char *filename, int check_console) { int fd; struct stat stat_buf; @@ -110,39 +95,37 @@ int check_file (char *filename, int chec /* Avoiding race conditions: use of fstat makes sure that both 'open' and 'stat' operate on the same file */ - *msg = 0; - fd = open (filename, O_RDWR); - if (fd == -1) + if (fd == -1){ return -1; + } - if (fstat (fd, &stat_buf) == -1) + if (fstat (fd, &stat_buf) == -1){ + close (fd); return -1; + } /* Must be character device */ if (!S_ISCHR (stat_buf.st_mode)){ - *msg = "Not a character device"; + close (fd); return -1; } -#ifdef DEBUG - fprintf (stderr, "Device: %x\n", stat_buf.st_rdev); -#endif if (check_console){ /* Second time: must be console */ if ((stat_buf.st_rdev & 0xff00) != 0x0400){ - *msg = "Not a console"; + close (fd); return -1; } - + if ((stat_buf.st_rdev & 0x00ff) > 63){ - *msg = "Minor device number too big"; + close (fd); return -1; } /* Must be owned by the user */ if (stat_buf.st_uid != getuid ()){ - *msg = "Not a owner"; + close (fd); return -1; } } @@ -155,10 +138,9 @@ int check_file (char *filename, int chec /* Because the name of the tty is supplied by the user and this can be a setuid program a lot of checks has to done to avoid creating a security hole */ -char *detect_console (void) +void detect_console (void) { - char *msg; - int xlen; + size_t xlen; /* Must be console */ /* Handle the case for /dev/tty?? and /dev/vc/?? */ @@ -173,24 +155,16 @@ char *detect_console (void) strncmp(tty_name+xlen-4,"vc/",3) == 0) || !isdigit(tty_name[xlen - 1]) || !isdigit(tty_name[len - 1])) - return "Doesn't look like console"; + return; snprintf (vcs_name, sizeof (vcs_name), "/dev/vcsa%s", tty_name + xlen - 1); - vcs_fd = check_file (vcs_name, 0, &msg); - console_fd = check_file (tty_name, 1, &msg); + vcs_fd = check_file (vcs_name, 0); + console_fd = check_file (tty_name, 1); -#ifdef DEBUG - fprintf (stderr, "vcs_fd = %d console_fd = %d\n", vcs_fd, console_fd); -#endif - - if (vcs_fd != -1){ + if (vcs_fd != -1 && console_fd != -1){ console_flag = 3; - } - - if (console_fd == -1) - return msg; - - return NULL; + } else if (vcs_fd != -1) + close (vcs_fd); } void save_console (void) @@ -200,73 +174,31 @@ void save_console (void) if (!console_flag) return; buffer [1] = tty_name [len-1] - '0'; - if (console_flag >= 2){ - /* Linux >= 1.1.67 */ - /* Get screen contents and cursor position */ - buffer [0] = 8; - if (console_flag == 2){ - if ((i = ioctl (console_fd, TIOCLINUX, buffer)) == -1){ - /* Oops, this is not Linux 1.1.67 */ - console_flag = 1; - } - } else { - lseek (vcs_fd, 0, 0); - read (vcs_fd, buffer, buffer_size); - } - } - if (console_flag == 1){ - int index, x, y; - - /* Linux < 1.1.67 */ - /* Get screen contents */ - buffer [0] = 0; - if (ioctl(console_fd, TIOCLINUX, buffer) == -1){ - buffer[0] = buffer[1] = 0; - - /* Linux bug: bad ioctl on console 8 */ - if (ioctl(console_fd, TIOCLINUX, buffer) == -1){ - /* Oops, this is not a console after all */ - console_flag = 0; - return; - } - } - /* Select the beginning of the bottommost empty line - to be the cursor position */ - index = 2 + rows * columns; - for (y = rows - 1; y >= 0; y--) - for (x = columns - 1; x >= 0; x--) - if (buffer[--index] != ' ') - goto non_space_found; - non_space_found: - buffer[0] = y + 1; - buffer[1] = 0; - /*tty_cursormove(y + 1, 0);*/ + lseek (vcs_fd, 0, 0); + read (vcs_fd, buffer, buffer_size); + console_fd = check_file (tty_name, 1); + if (console_fd == -1){ + /* Ouch, we might have read something we have no businness reading. */ + memset (buffer, 0, buffer_size); + console_flag = 0; + return; } + close (console_fd); } void restore_console (void) { if (!console_flag) return; - if (console_flag == 2){ - /* Linux >= 1.1.67 */ - /* Restore screen contents and cursor position */ - buffer [0] = 9; - buffer [1] = tty_name [len-1] - '0'; - ioctl (console_fd, TIOCLINUX, buffer); - } - if (console_flag == 3){ - lseek (vcs_fd, 0, 0); - write (vcs_fd, buffer, buffer_size); - } - if (console_flag == 1){ - /* Clear screen */ - write(console_fd, "\033[H\033[2J", 7); - /* Output saved screen contents */ - write(console_fd, buffer + 2, rows * columns); - /* Move the cursor to the previously selected position */ - tty_cursormove(buffer[0], buffer[1]); + console_fd = check_file (tty_name, 1); + if (console_fd == -1){ + /* The user lost permissions to his tty. */ + console_flag = 0; + return; } + lseek (vcs_fd, 0, 0); + write (vcs_fd, buffer, buffer_size); + close (console_fd); } void send_contents () @@ -276,24 +208,28 @@ void send_contents () int lastline; unsigned char message; unsigned short bytes; - int bytes_per_char; - - bytes_per_char = console_flag == 1 ? 1 : 2; - + /* Calculate the number of used lines */ - if (console_flag == 2 || console_flag == 1 || console_flag == 3){ - index = (2 + rows * columns) * bytes_per_char; - for (y = rows - 1; y >= 0; y--) - for (x = columns - 1; x >= 0; x--){ - index -= bytes_per_char; - if (buffer[index] != ' ') - goto non_space_found; - } - non_space_found: - lastline = y + 1; - } else + if (console_flag == 0) return; + index = (2 + rows * columns) * 2; + for (y = rows - 1; y >= 0; y--) + for (x = columns - 1; x >= 0; x--){ + index -= 2; + if (buffer[index] != ' ') + goto non_space_found; + } +non_space_found: + lastline = y + 1; + + console_fd = check_file (tty_name, 1); + if (console_fd == -1){ + console_flags = 0; + return; + } + close(console_fd); + /* Inform the invoker that we can handle this command */ message = CONSOLE_CONTENTS; write (cmd_output, &message, 1); @@ -305,48 +241,31 @@ void send_contents () begin_line = lastline; if (end_line > lastline) end_line = lastline; + if (begin_line > end_line) + begin_line = end_line; /* Tell the invoker how many bytes it will be */ bytes = (end_line - begin_line) * columns; write (cmd_output, &bytes, 2); /* Send the contents */ - for (index = (2 + begin_line * columns) * bytes_per_char; - index < (2 + end_line * columns) * bytes_per_char; - index += bytes_per_char) - write (cmd_output, buffer + index, 1); + for (index = (2 + begin_line * columns) * 2; + index < (2 + end_line * columns) * 2; + index += 2) + write (cmd_output, buffer + index, 1); /* All done */ } int main (int argc, char **argv) { - char *error; unsigned char action = 0; - int stderr_fd; - /* - * Make sure Stderr points to a valid place - */ close (2); - stderr_fd = open ("/dev/tty", O_RDWR); - if (stderr_fd == -1) /* This may well happen if program is running non-root */ - stderr_fd = open ("/dev/null", O_RDWR); - - if (stderr_fd == -1) - exit (1); - - if (stderr_fd != 2) - while (dup2 (stderr_fd, 2) == -1 && errno == EINTR) - ; - if (argc != 2){ - /* Wrong number of arguments */ - - dwrite (2, "Usage: cons.saver <ttyname>\n"); console_flag = 0; write (cmd_output, &console_flag, 1); - return 3; + exit(3); } /* Lose the control terminal */ @@ -354,28 +273,26 @@ int main (int argc, char **argv) /* Check that the argument is a legal console */ tty_name = argv [1]; - len = strlen(tty_name); - error = detect_console (); - - if (error){ - /* Not a console -> no need for privileges */ - setuid (getuid ()); -/* dwrite (2, error); */ + len = strnlen(tty_name, 64); + if (len == 64) { + console_flag = 0; + write (cmd_output, &console_flag, 1); + exit(3); + } + detect_console (); + /* Console was detected */ + if (console_flag != 3) console_flag = 0; - if (console_fd >= 0) - close (console_fd); - } else { - /* Console was detected */ - if (console_flag != 3) - console_flag = 2; /* Default to Linux >= 1.1.67 */ + else { /* Allocate buffer for screen image */ tty_getsize (); buffer_size = 4 + 2 * columns * rows; - buffer = (char*) malloc (buffer_size); + buffer = (char*) calloc (buffer_size, 1); + if (buffer == NULL) + console_flag = 0; } - /* If using /dev/vcs*, we don't need anymore the console fd */ - if (console_flag == 3) + if (console_fd >= 0) close (console_fd); /* Inform the invoker about the result of the tests */ static char *buffer = NULL; static int buffer_size = 0; static int columns, rows; static char vcs_name [128]; static int vcs_fd; -static void dwrite (int fd, char *buffer) -{ - write (fd, buffer, strlen (buffer)); -} - static void tty_getsize () { struct winsize winsz; - + winsz.ws_col = winsz.ws_row = 0; - ioctl (console_fd, TIOCGWINSZ, &winsz); - if (winsz.ws_col && winsz.ws_row){ + if (ioctl (console_fd, TIOCGWINSZ, &winsz) >= 0 + && winsz.ws_col && winsz.ws_row){ columns = winsz.ws_col; rows = winsz.ws_row; } else { /* Never happens (I think) */ - dwrite (2, "TIOCGWINSZ failed\n"); columns = 80; rows = 25; console_flag = 0; } } -inline void tty_cursormove(int y, int x) -{ - char buffer [128]; - - /* Standard ANSI escape sequence for cursor positioning */ - snprintf (buffer, sizeof (buffer), "\33[%d;%dH", y + 1, x + 1); - dwrite (console_fd, buffer); -} - -int check_file (char *filename, int check_console, char **msg) +int check_file (char *filename, int check_console) { int fd; struct stat stat_buf; @@ -110,39 +95,37 @@ int check_file (char *filename, int chec /* Avoiding race conditions: use of fstat makes sure that both 'open' and 'stat' operate on the same file */ - *msg = 0; - fd = open (filename, O_RDWR); - if (fd == -1) + if (fd == -1){ return -1; + } - if (fstat (fd, &stat_buf) == -1) + if (fstat (fd, &stat_buf) == -1){ + close (fd); return -1; + } /* Must be character device */ if (!S_ISCHR (stat_buf.st_mode)){ - *msg = "Not a character device"; + close (fd); return -1; } -#ifdef DEBUG - fprintf (stderr, "Device: %x\n", stat_buf.st_rdev); -#endif if (check_console){ /* Second time: must be console */ if ((stat_buf.st_rdev & 0xff00) != 0x0400){ - *msg = "Not a console"; + close (fd); return -1; } - + if ((stat_buf.st_rdev & 0x00ff) > 63){ - *msg = "Minor device number too big"; + close (fd); return -1; } /* Must be owned by the user */ if (stat_buf.st_uid != getuid ()){ - *msg = "Not a owner"; + close (fd); return -1; } } @@ -155,10 +138,9 @@ int check_file (char *filename, int chec /* Because the name of the tty is supplied by the user and this can be a setuid program a lot of checks has to done to avoid creating a security hole */ -char *detect_console (void) +void detect_console (void) { - char *msg; - int xlen; + size_t xlen; /* Must be console */ /* Handle the case for /dev/tty?? and /dev/vc/?? */ @@ -173,24 +155,16 @@ char *detect_console (void) strncmp(tty_name+xlen-4,"vc/",3) == 0) || !isdigit(tty_name[xlen - 1]) || !isdigit(tty_name[len - 1])) - return "Doesn't look like console"; + return; snprintf (vcs_name, sizeof (vcs_name), "/dev/vcsa%s", tty_name + xlen - 1); - vcs_fd = check_file (vcs_name, 0, &msg); - console_fd = check_file (tty_name, 1, &msg); + vcs_fd = check_file (vcs_name, 0); + console_fd = check_file (tty_name, 1); -#ifdef DEBUG - fprintf (stderr, "vcs_fd = %d console_fd = %d\n", vcs_fd, console_fd); -#endif - - if (vcs_fd != -1){ + if (vcs_fd != -1 && console_fd != -1){ console_flag = 3; - } - - if (console_fd == -1) - return msg; - - return NULL; + } else if (vcs_fd != -1) + close (vcs_fd); } void save_console (void) @@ -200,73 +174,31 @@ void save_console (void) if (!console_flag) return; buffer [1] = tty_name [len-1] - '0'; - if (console_flag >= 2){ - /* Linux >= 1.1.67 */ - /* Get screen contents and cursor position */ - buffer [0] = 8; - if (console_flag == 2){ - if ((i = ioctl (console_fd, TIOCLINUX, buffer)) == -1){ - /* Oops, this is not Linux 1.1.67 */ - console_flag = 1; - } - } else { - lseek (vcs_fd, 0, 0); - read (vcs_fd, buffer, buffer_size); - } - } - if (console_flag == 1){ - int index, x, y; - - /* Linux < 1.1.67 */ - /* Get screen contents */ - buffer [0] = 0; - if (ioctl(console_fd, TIOCLINUX, buffer) == -1){ - buffer[0] = buffer[1] = 0; - - /* Linux bug: bad ioctl on console 8 */ - if (ioctl(console_fd, TIOCLINUX, buffer) == -1){ - /* Oops, this is not a console after all */ - console_flag = 0; - return; - } - } - /* Select the beginning of the bottommost empty line - to be the cursor position */ - index = 2 + rows * columns; - for (y = rows - 1; y >= 0; y--) - for (x = columns - 1; x >= 0; x--) - if (buffer[--index] != ' ') - goto non_space_found; - non_space_found: - buffer[0] = y + 1; - buffer[1] = 0; - /*tty_cursormove(y + 1, 0);*/ + lseek (vcs_fd, 0, 0); + read (vcs_fd, buffer, buffer_size); + console_fd = check_file (tty_name, 1); + if (console_fd == -1){ + /* Ouch, we might have read something we have no businness reading. */ + memset (buffer, 0, buffer_size); + console_flag = 0; + return; } + close (console_fd); } void restore_console (void) { if (!console_flag) return; - if (console_flag == 2){ - /* Linux >= 1.1.67 */ - /* Restore screen contents and cursor position */ - buffer [0] = 9; - buffer [1] = tty_name [len-1] - '0'; - ioctl (console_fd, TIOCLINUX, buffer); - } - if (console_flag == 3){ - lseek (vcs_fd, 0, 0); - write (vcs_fd, buffer, buffer_size); - } - if (console_flag == 1){ - /* Clear screen */ - write(console_fd, "\033[H\033[2J", 7); - /* Output saved screen contents */ - write(console_fd, buffer + 2, rows * columns); - /* Move the cursor to the previously selected position */ - tty_cursormove(buffer[0], buffer[1]); + console_fd = check_file (tty_name, 1); + if (console_fd == -1){ + /* The user lost permissions to his tty. */ + console_flag = 0; + return; } + lseek (vcs_fd, 0, 0); + write (vcs_fd, buffer, buffer_size); + close (console_fd); } void send_contents () @@ -276,24 +208,28 @@ void send_contents () int lastline; unsigned char message; unsigned short bytes; - int bytes_per_char; - - bytes_per_char = console_flag == 1 ? 1 : 2; - + /* Calculate the number of used lines */ - if (console_flag == 2 || console_flag == 1 || console_flag == 3){ - index = (2 + rows * columns) * bytes_per_char; - for (y = rows - 1; y >= 0; y--) - for (x = columns - 1; x >= 0; x--){ - index -= bytes_per_char; - if (buffer[index] != ' ') - goto non_space_found; - } - non_space_found: - lastline = y + 1; - } else + if (console_flag == 0) return; + index = (2 + rows * columns) * 2; + for (y = rows - 1; y >= 0; y--) + for (x = columns - 1; x >= 0; x--){ + index -= 2; + if (buffer[index] != ' ') + goto non_space_found; + } +non_space_found: + lastline = y + 1; + + console_fd = check_file (tty_name, 1); + if (console_fd == -1){ + console_flags = 0; + return; + } + close(console_fd); + /* Inform the invoker that we can handle this command */ message = CONSOLE_CONTENTS; write (cmd_output, &message, 1); @@ -305,48 +241,31 @@ void send_contents () begin_line = lastline; if (end_line > lastline) end_line = lastline; + if (begin_line > end_line) + begin_line = end_line; /* Tell the invoker how many bytes it will be */ bytes = (end_line - begin_line) * columns; write (cmd_output, &bytes, 2); /* Send the contents */ - for (index = (2 + begin_line * columns) * bytes_per_char; - index < (2 + end_line * columns) * bytes_per_char; - index += bytes_per_char) - write (cmd_output, buffer + index, 1); + for (index = (2 + begin_line * columns) * 2; + index < (2 + end_line * columns) * 2; + index += 2) + write (cmd_output, buffer + index, 1); /* All done */ } int main (int argc, char **argv) { - char *error; unsigned char action = 0; - int stderr_fd; - /* - * Make sure Stderr points to a valid place - */ close (2); - stderr_fd = open ("/dev/tty", O_RDWR); - if (stderr_fd == -1) /* This may well happen if program is running non-root */ - stderr_fd = open ("/dev/null", O_RDWR); - - if (stderr_fd == -1) - exit (1); - - if (stderr_fd != 2) - while (dup2 (stderr_fd, 2) == -1 && errno == EINTR) - ; - if (argc != 2){ - /* Wrong number of arguments */ - - dwrite (2, "Usage: cons.saver <ttyname>\n"); console_flag = 0; write (cmd_output, &console_flag, 1); - return 3; + exit(3); } /* Lose the control terminal */ @@ -354,28 +273,26 @@ int main (int argc, char **argv) /* Check that the argument is a legal console */ tty_name = argv [1]; - len = strlen(tty_name); - error = detect_console (); - - if (error){ - /* Not a console -> no need for privileges */ - setuid (getuid ()); -/* dwrite (2, error); */ + len = strnlen(tty_name, 64); + if (len == 64) { + console_flag = 0; + write (cmd_output, &console_flag, 1); + exit(3); + } + detect_console (); + /* Console was detected */ + if (console_flag != 3) console_flag = 0; - if (console_fd >= 0) - close (console_fd); - } else { - /* Console was detected */ - if (console_flag != 3) - console_flag = 2; /* Default to Linux >= 1.1.67 */ + else { /* Allocate buffer for screen image */ tty_getsize (); buffer_size = 4 + 2 * columns * rows; - buffer = (char*) malloc (buffer_size); + buffer = (char*) calloc (buffer_size, 1); + if (buffer == NULL) + console_flag = 0; } - /* If using /dev/vcs*, we don't need anymore the console fd */ - if (console_flag == 3) + if (console_fd >= 0) close (console_fd); /* Inform the invoker about the result of the tests */
Sorry that was longer than I thought ;-)
Jakub is your patch something that can just be applied or does it need work/testing? Have you been using mc with this patch?
It needs testing, furthermore /dev/vcs* group needs to be changed in dev package. I hope to get to it today or tomorrow.
mc-4.5.55-2 contains this fix, please test.
Patch seems to work for me, reopen if not.