>Number:         2190
>Category:       lib
>Synopsis:       catopen returns -1, strerror doesn't check, catgets barfs.
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    lib-bug-people (Library Bug People)
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Mar  8 06:05:01 1996
>Last-Modified:
>Originator:     Ronald Khoo
>Organization:
Demon Internet Limited
>Release:        <NetBSD-current source date> Today
>Environment:
Any NetBSD-1.1 or NetBSD-1.1A current should exhibit this behavior
System: NetBSD netbsd.eng.demon.net 1.1A NetBSD 1.1A (NETBSD-ENG) #0: Fri Jan 19 08:27:23 GMT 1996 ronald@netbsd.eng.demon.net:/sys/arch/i386/compile/NETBSD-ENG i386


>Description:
The return value from catopen, if it fails, is -1.  strerror doesn't
check for this, and passes it to catgets, which casts it to a pointer
and dereferences it, thus killing the calling program with SIGSEGV.

This will happen if, for example, the calling program fails to
allocate memory and tries to use strerror to report the problem.

Arguably, strerror is at fault for not checking the return value
of catopen, but the underlying problem is that the error return
value of -1 is treated as a pointer, and error checks are against
NULL in msgcat.c.
>How-To-Repeat:
This program should print "Cannot allocate memory".  In fact it core dumps.

#include <errno.h>
#include <sys/types.h>
#include <sys/time.h>
#include <sys/resource.h>

main()
{
	extern char etext;	/* XXX */
	struct rlimit rlp;

	getrlimit(RLIMIT_DATA, &rlp);
	if (brk(&etext + rlp.rlim_cur) < 0) {
		perror("brk");
		exit(1);
	}
	printf("%s\n", strerror(ENOMEM));
}
>Fix:
This fix is probably incomplete.  It simply checks for -1 in the
case documented above (you have to do catclose too :-)  With this
fix, the test program above works.

*** /usr/src/lib/libc/nls/msgcat.c	Sat Oct 14 01:49:44 1995
--- ./msgcat.c	Fri Mar  8 10:41:00 1996
***************
*** 248,253 ****
--- 248,256 ----
      MCCatT	*cat = (MCCatT *) catd;
      char	*cptr;
  
+     if (catd == -1)
+ 	return dflt;
+ 
      msg = MCGetMsg(MCGetSet(cat, setId), msgId);
      if (msg) cptr = msg->msg.str;
      else cptr = dflt;
***************
*** 263,268 ****
--- 266,272 ----
      MCMsgT	*msg;
      int		i, j;
  
+     if (catd == -1) return -1;
      if (!cat) return -1;
      
      if (cat->loadType != MCLoadAll) close(cat->fd);
>Audit-Trail:
>Unformatted: