NetBSD Problem Report #42636

From mouse@Sparkle.Rodents-Montreal.ORG  Mon Jan 18 07:26:44 2010
Return-Path: <mouse@Sparkle.Rodents-Montreal.ORG>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by www.NetBSD.org (Postfix) with ESMTP id 4E60A63B86D
	for <gnats-bugs@gnats.NetBSD.org>; Mon, 18 Jan 2010 07:26:44 +0000 (UTC)
Message-Id: <201001180726.CAA28381@Sparkle.Rodents-Montreal.ORG>
Date: Mon, 18 Jan 2010 02:19:50 -0500 (EST)
From: der Mouse <mouse@Rodents-Montreal.ORG>
Reply-To: mouse@Rodents-Montreal.ORG
To: gnats-bugs@gnats.NetBSD.org
Subject: [dM] t_getstr potential memory overrun
X-Send-Pr-Version: 3.95

>Number:         42636
>Category:       lib
>Synopsis:       [dM] t_getstr potential memory overrun
>Confidential:   no
>Severity:       serious
>Priority:       low
>Responsible:    lib-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Jan 18 07:30:00 +0000 2010
>Originator:     der Mouse
>Release:        NetBSD 4.0.1
>Organization:
	Dis-
>Environment:
System: NetBSD iMac-backup.Rodents-Montreal.ORG 4.0.1 NetBSD 4.0.1 (IMAC-BACKUP) #2: Thu Dec 31 09:47:34 EST 2009 mouse@iMac-backup.Rodents-Montreal.ORG:/home/mouse/kbuild/IMAC-BACKUP i386
Architecture: i386
Machine: i386
>Description:
	The manpage description of t_getstr describes the area and
	limit parameters saying that the "limit parameter that gives
	the number of characters that can be inserted in to the array
	pointed to by area", adding that it "is updated [...] to give
	the number of characters that remain available in area", and
	that if area is nil then "the size required to hold the
	capability string will be returned in limit".

	However, this is not quite the reality.  The area pointer is
	advanced by one more than the limit parameter is decreased by,
	leading to a potential buffer overrun by as many bytes as there
	are string capabilities involved - the limit advance does not
	take the \0 into account, while the area pointer advance does.
	Also, the returned limit value for a nil-pointer area call is
	the amount the limit value would have been decreased by, not
	the number of bytes that would have been copied if the area
	pointer had been non-nil, thus inviting the caller to allocate
	one byte too few.  (Fortunately, the buffer overrun is more
	difficult to exploit than most buffer overruns, since the
	overrun data is not very controllable by the attacker in most
	scenarios.)

	It's possible there's actually a standard somewhere that
	specifies this bizarre bug-inviting behaviour.  This is serious
	enough that I would actually recommend ignoring any such
	standard; if NetBSD does not want to do that, this should then
	be reclassified as a doc-bug and warnings splashed all over the
	manpage that the behaviour is severely counterintuitive and the
	pointer and limit variables must be treated very carefully to
	avoid buffer overrun bugs.
>How-To-Repeat:
	% cat z.c
	#include <stdio.h>
	#include <errno.h>
	#include <stdlib.h>
	#include <termcap.h>

	extern const char *__progname;

	static void fail(const char *why)
	{
	 fprintf(stderr,"%s: %s\n",__progname,why);
	 exit(1);
	}

	int main(void);
	int main(void)
	{
	 struct tinfo *inf;
	 char area[64];
	 char *ap;
	 size_t lim;
	 char *cap;

	 if (t_getent(&inf,"vt100") != 1) fail("can't look up vt100");
	 ap = &area[0];
	 lim = sizeof(area);
	 cap = t_getstr(inf,"bl",&ap,&lim);
	 if (cap == 0) fail("can't look up bl capability");
	 printf("area pointer advanced by %d\n",(int)(ap-&area[0]));
	 printf("limit reduced by %d\n",(int)(sizeof(area)-lim));
	 errno = 0;
	 lim = 0;
	 t_getstr(inf,"bl",0,&lim);
	 if (errno) fail("can't nil-pointer check bl capability");
	 printf("nil-pointer length is %d\n",(int)lim);
	 return(0);
	}
	% cc -o z z.c
	% ./z
	area pointer advanced by 2
	limit reduced by 1
	nil-pointer length is 1
	% 
>Fix:
	Relative to termcap.c,v 1.49:

	--- termcap.c.orig	2010-01-17 11:56:50.000000000 -0500
	+++ termcap.c	2010-01-17 11:58:22.000000000 -0500
	@@ -384,6 +384,7 @@
	 		return NULL;
	 	}

	+	i ++; /* account for the NUL */
	 	if (area != NULL) {
	 		/*
	 		 * check if there is room for the new entry to be put into
	@@ -398,7 +399,7 @@
	 		(void)strcpy(*area, s);
	 		free(s);
	 		s = *area;
	-		*area += i + 1;
	+		*area += i;
	 		if (limit != NULL) *limit -= i;

	 		return (s);

/~\ The ASCII				  Mouse
\ / Ribbon Campaign
 X  Against HTML		mouse@rodents-montreal.org
/ \ Email!	     7D C8 61 52 5D E7 2D 39  4E F1 31 3E E8 B3 27 4B

>Unformatted:
 	Also believed present as far back as 1.4T - probably anything
 	with the t_* termcap routines.  May be present in more recent
 	versions too; I don't have anything newer at ready hand.

NetBSD Home
NetBSD PR Database Search

(Contact us) $NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2007 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.