[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Critical bug in ncurses 6.0

From: Johannes Schindelin
Subject: Critical bug in ncurses 6.0
Date: Fri, 26 Jun 2015 15:47:44 +0200
User-agent: Roundcube Webmail/1.1.0

Hi ncurses maintainers,

I believe I found a really awful bug in ncurses 6.0-20150613: when TERM is set to a value that does not correspond to a subdirectory in /usr/share/terminfo/??/.

The reason is that the `drv_CanHandle()` function in `ncurses/tinfo/tinfo_driver.c` wants to return a `bool`, but reuses the `ret_error*()` functions from `ncurses/curses.priv.h` that returns `ERR` which is (-1) and is mapped to `TRUE`!!!

As a consequence, the driver is used by mistake in `_nc_get_driver()` in `ncurses/base/lib_driver.c` and uses `free()`d data (by continuing to use the `termp` variable that no longer points at allocated memory!).

This causes a crash in Git for Windows 2.x (which I am maintaining) when calling Bash with an unknown `TERM` value.

Please consider the following patch, which is also available on GitHub (because my mail program will most likely corrupt the patch): https://raw.githubusercontent.com/git-for-windows/MSYS2-packages/fix-ncurses-crash/ncurses/0001-CRITICAL-fix-return-value-of-drv_CanHandle-on-unknow.patch

-- snipsnap --
From 41f152b6e8bea7e92be960158ef4bd9df34dd8a0 Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <address@hidden>
Date: Fri, 26 Jun 2015 13:19:49 +0000
Subject: [PATCH] CRITICAL: fix return value of drv_CanHandle on unknown

By mistake, the 5.9 -> 6.0 patches tried to reuse the `ret_error*()`
functions in both lib_driver.c and tinfo_driver.c. However, in the
latter file, the return value must be FALSE (because the type is bool),
not ERR (as it must be in lib_driver.c). As a consequence,
`_nc_get_driver()` assumed *success* when we actually failed, and worse:
we already free()d data that is still happily used by the code
afterwards because it assumed that we were successful.

Work around this by reinstating the `ret_error*()` family that returns
FALSE in tinfo_driver.c.

Signed-off-by: Johannes Schindelin <address@hidden>
 ncurses/tinfo/tinfo_driver.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/ncurses/tinfo/tinfo_driver.c b/ncurses/tinfo/tinfo_driver.c
index a17accd..9597466 100644
--- a/ncurses/tinfo/tinfo_driver.c
+++ b/ncurses/tinfo/tinfo_driver.c
@@ -113,6 +113,28 @@ drv_Name(TERMINAL_CONTROL_BLOCK * TCB)
     return "tinfo";

+/* We need to return FALSE here, not ERR. */
+#undef ret_error
+#define ret_error(code, fmt, arg)      if (errret) {\
+                                           *errret = code;\
+                                           returnCode(FALSE);\
+                                       } else {\
+                                           fprintf(stderr, fmt, arg);\
+                                           exit(EXIT_FAILURE);\
+                                       }
+#undef ret_error1
+#define ret_error1(code, fmt, arg)     ret_error(code, "'%s': " fmt, arg)
+#undef ret_error0
+#define ret_error0(code, msg)          if (errret) {\
+                                           *errret = code;\
+                                           returnCode(FALSE);\
+                                       } else {\
+                                           fprintf(stderr, msg);\
+                                           exit(EXIT_FAILURE);\
+                                       }
 static bool
drv_CanHandle(TERMINAL_CONTROL_BLOCK * TCB, const char *tname, int *errret)

reply via email to

[Prev in Thread] Current Thread [Next in Thread]