prevent cal segfault

prevent cal segfault

[Petr Gajdos]

[-- Attachment #1.1: Type: text/plain, Size: 273 bytes --]

Hello,

cal utility segfaults in putp() when setupterm() fails as cal
insufficiently checks for setupterm() failure.

$ TERM= cal
Segmentation fault

Please find details in 

https://bugzilla.suse.com/show_bug.cgi?id=903440

especially in comment 4.

Petr


[-- Attachment #1.2: util-linux-cal-no-terminal.patch --]
[-- Type: text/x-patch, Size: 1979 bytes --]

Index: misc-utils/cal.c
===================================================================
--- misc-utils/cal.c.orig	2014-08-26 16:48:45.132654654 +0200
+++ misc-utils/cal.c	2015-03-11 14:16:06.075064939 +0100
@@ -81,14 +81,22 @@
 # endif
 # include <term.h>
 
-static void my_setupterm(const char *term, int fildes, int *errret)
+#if defined(HAVE_LIBNCURSES) || defined(HAVE_LIBNCURSESW) || defined(HAVE_LIBTERMCAP)
+static const char	*term="";
+#endif
+
+static int my_setupterm(const char *term, int fildes)
 {
-	setupterm((char *)term, fildes, errret);
+        int errret; /* unused */
+	return setupterm((char *)term, fildes, &errret) == OK;
 }
 
 static void my_putstring(char *s)
 {
-	putp(s);
+	if (term)
+		putp(s);
+	else
+		fputs(s, stdout);
 }
 
 static const char *my_tgetstr(char *s __attribute__((__unused__)), char *ss)
@@ -107,9 +115,9 @@
 static char tcbuffer[4096];
 static char *strbuf = termbuffer;
 
-static void my_setupterm(const char *term, int fildes __attribute__((__unused__)), int *errret)
+static int my_setupterm(const char *term, int fildes __attribute__((__unused__)))
 {
-	*errret = tgetent(tcbuffer, term);
+	return tgetent(tcbuffer, term) == 1;
 }
 
 static void my_putstring(char *s)
@@ -135,10 +143,6 @@
 
 #endif	/* end of LIBTERMCAP / NCURSES */
 
-#if defined(HAVE_LIBNCURSES) || defined(HAVE_LIBNCURSESW) || defined(HAVE_LIBTERMCAP)
-static const char	*term="";
-#endif
-
 static const char	*Senter="", *Sexit="";/* enter and exit standout mode */
 
 #include "widechar.h"
@@ -293,12 +297,14 @@
 
 #if defined(HAVE_LIBNCURSES) || defined(HAVE_LIBNCURSESW) || defined(HAVE_LIBTERMCAP)
 	if ((term = getenv("TERM"))) {
-		int ret;
-		my_setupterm(term, STDOUT_FILENO, &ret);
-		if (ret > 0) {
+		if (my_setupterm(term, STDOUT_FILENO)) {
 			Senter = my_tgetstr("so","smso");
 			Sexit = my_tgetstr("se","rmso");
 		}
+		else
+		{
+			term = NULL;
+		}
 	}
 #endif
 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: prevent cal segfault

[Ruediger Meier]

On Thursday 12 March 2015, Petr Gajdos wrote:
> Hello,
>
> cal utility segfaults in putp() when setupterm() fails as cal
> insufficiently checks for setupterm() failure.
>
> $ TERM= cal
> Segmentation fault

Regarding TERM there is another issue with "more".

$ TERM= make check TS_OPTS="--parallel=1 more" 
  more: regexp                   ... FAILED (more/regexp)
  more: squeeze                  ... OK


Without TERM it prints a '\r' because of text-utils/more.c:1083:
void kill_line(void)
{
        erasep(0);
        if (!eraseln || dumb)
                putchar('\r');
}


cu,
Rudi

Re: prevent cal segfault

[Karel Zak]

On Thu, Mar 12, 2015 at 09:46:39AM +0100, Petr Gajdos wrote:
> cal utility segfaults in putp() when setupterm() fails as cal
> insufficiently checks for setupterm() failure.
> 
> $ TERM= cal
> Segmentation fault

I'm not able to reproduce this problem. Anyway you're right that the
current code is pretty fragile and ignore setupterm() return code is
bad idea.

I have applied a little bit different patch, please test it in your
environment. The patch is also in stable/v2.26 (for v2.26.1).

https://github.com/karelzak/util-linux/commit/2a4b073e8b2d34dfadd8306f7d71957687923bdd


Note that I have doubts we really need so many fallbacks in cal(1)
code. IMHO it would be enough to support two ways: ncurses or just
without any extra terminal support (so stdio.h only). I'll probably 
cleanup this for v2.27 to make the code more maintainable. 

> --- misc-utils/cal.c.orig	2014-08-26 16:48:45.132654654 +0200
> +++ misc-utils/cal.c	2015-03-11 14:16:06.075064939 +0100
> @@ -81,14 +81,22 @@
>  # endif
>  # include <term.h>
>  
> -static void my_setupterm(const char *term, int fildes, int *errret)
> +#if defined(HAVE_LIBNCURSES) || defined(HAVE_LIBNCURSESW) || defined(HAVE_LIBTERMCAP)
> +static const char	*term="";
> +#endif

This seems incorrect, you have moved "#if defined" into another "#if".

    Karel


-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: prevent cal segfault

[Petr Gajdos]

[-- Attachment #1: Type: text/plain, Size: 1387 bytes --]

On Thu, Mar 12, 2015 at 12:05:28PM +0100, Karel Zak wrote:
> On Thu, Mar 12, 2015 at 09:46:39AM +0100, Petr Gajdos wrote:
> > cal utility segfaults in putp() when setupterm() fails as cal
> > insufficiently checks for setupterm() failure.
> > 
> > $ TERM= cal
> > Segmentation fault
> 
> I'm not able to reproduce this problem. Anyway you're right that the
> current code is pretty fragile and ignore setupterm() return code is
> bad idea.
> 
> I have applied a little bit different patch, please test it in your
> environment. The patch is also in stable/v2.26 (for v2.26.1).
> 
> https://github.com/karelzak/util-linux/commit/2a4b073e8b2d34dfadd8306f7d71957687923bdd

Yes, that works with my original testcase. has_term is better then
*term misuse.

> > --- misc-utils/cal.c.orig	2014-08-26 16:48:45.132654654 +0200
> > +++ misc-utils/cal.c	2015-03-11 14:16:06.075064939 +0100
> > @@ -81,14 +81,22 @@
> >  # endif
> >  # include <term.h>
> >  
> > -static void my_setupterm(const char *term, int fildes, int *errret)
> > +#if defined(HAVE_LIBNCURSES) || defined(HAVE_LIBNCURSESW) || defined(HAVE_LIBTERMCAP)
> > +static const char	*term="";
> > +#endif
> 
> This seems incorrect, you have moved "#if defined" into another "#if".

Ugh, sure. I have sent you older version of the patch, not the one
from comment 6 of referenced bug.

Thanks!
Petr


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]