[15488] in bugtraq

home help back first fref pref prev next nref lref last post

Re: ftpd: the advisory version

daemon@ATHENA.MIT.EDU (Sebastian)
Tue Jun 27 15:16:41 2000

Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Message-Id:  <20000626230307.A22291@nb.in-berlin.de>
Date:         Mon, 26 Jun 2000 23:03:07 +0200
Reply-To: Sebastian <scut@NB.IN-BERLIN.DE>
From: Sebastian <scut@NB.IN-BERLIN.DE>
X-To:         Bernd Luevelsmeyer <bernd.luevelsmeyer@HEITEC.NET>
To: BUGTRAQ@SECURITYFOCUS.COM
In-Reply-To:  <3955B817.7FFD53BB@heitec.net>; from
              bernd.luevelsmeyer@HEITEC.NET on Sun, Jun 25,
              2000 at 09:43:19AM +0200

Hi.

On Sun, Jun 25, 2000 at 09:43:19AM +0200, Bernd Luevelsmeyer wrote:

> According to the C standard, <ctype.h> functions operate on values that
> are representable as a 'unsigned char' or EOF; hence, if the compiler's
> 'char' is signed then negative character values in the string 'cmd' are
> possible and might cause unpredictable results in isspace(), isupper()
> and tolower(). Because sanitizing shouldn't stop, and the test with
> isupper() is unnecessary anyway (tolower() returns the unchanged value
> if the parameter isn't an uppercase letter), I suggest:
>      for (t = cmd; *t; t++)
>              *t = tolower((unsigned char)*t);

As a general advice from my own experience in auditing programs, it is
good to always prefer the (unsigned char) type over the (char) type when
dealing with possible non-ASCII (ie >= 0x80) data, especially when user
supplied data is used.

A lot of security issues arise from code where the programmer is not
fully aware of what a type cast does, or how to properly ensure no value-
overflow does occur.

This code is insecure:

void
func (char *domain)
{
	int	len = domain[0];
	char	buff[64];


	if (len >= 64)
		return;

	strncpy (buff, &domain[1], len);
	buff[63] = '\x00';
}


len can become negative when domain[0] is >= 0x80. The len check will be
passed because of it's type, and the strncpy may result in arbitrary length
NUL-terminated data being copied into buff.

This is known, and there is a small paper about it (written by mixter I
think), and is obvious in the code above. But there are more complicated
source codes where such vulnerabilities can easily arise, and it may slip
through even the careful programmers hands (hi l0pht).

So please, use unsigned char pointers, buffers and casts. For example the
proper code for the above function would be:

void
func_proper (unsigned char *domain)
{
	int		len = domain[0];
	unsigned char	buff[64];


	if (len >= 64)
		return;

	strncpy (buff, &domain[1], len);
	buff[63] = '\x00';
}


Also, with the recent vulnerabilities being made public the security
community notices a strong trend towards more wicked vulnerabilities,
drifing away from the simple "unchecked-strcpy" vulnerabilities, to
format-string-supply, type-casting, heap-overflow and one-byte-overflows
and yet unknown vulnerabilities.

There are many common mistakes still being made especially in C code,
like misuse of sizeof constants in strncat, snprintf and other functions,
falling for a "I use str*n* functions, so I'm safe" mentality. That's
where the future exploits will arise from.


ciao,
scut / teso


--
- scut@nb.in-berlin.de - http://nb.in-berlin.de/scut/ --- you don't need a --
-- lot of people to be great, you need a few great to be the best ------------
http://3261000594/scut/pgp - 5453 AC95 1E02 FDA7 50D2 A42D 427E 6DEF 745A 8E07
-- data in VK/USA Mayfly experienced, awaiting transfer location, hi echelon -

home help back first fref pref prev next nref lref last post