0036 2001/04/16  bugs in BIND 8.2.3-REL, ProFTPd, ...

==== TESO Informational =======================================================
This piece of information is to be kept confidential.
===============================================================================

Description ..........: bugs in BIND 8.2.3-REL, proftpd, ...
Date .................: 2001/04/16 18:00
Author ...............: scut
Publicity level ......: unknown
Affected .............: BIND 8.2.3-REL and below, ProFTPd
Type of entity .......: bugs
Type of discovery ....: vulnerability
Severity/Importance ..: medium
Found by .............: scut

===============================================================================

BIND 8.2.3-REL and below contains two buffer overflows in the handling of
UPDATE packets.

### bug 1

BIND-8.2.3-REL/src/bin/named/ns_update.c:1636

	case T_SIG:
		if (dlen < SIG_HDR_SIZE || size < dlen)
			return (0);
		memcpy(cp1, cp, SIG_HDR_SIZE);
		size -= SIG_HDR_SIZE;
		cp += SIG_HDR_SIZE;
		cp1 += SIG_HDR_SIZE;
		n = dn_expand(msg, eom, cp, (char *)cp1, size);
		if (n < 0 || n + SIG_HDR_SIZE > dlen)
			return (0);
		cp += n;
		n1 = dlen - n - SIG_HDR_SIZE;
		n = strlen((char *)cp1) + 1;
		cp1 += n;
/* #1 */
		if (size < n1)
			return (0);
		memcpy(cp1, cp, n1);
		cp1 += n1;
		return (cp1 - cp1init);

This function processes T_SIG resource records within UPDATE packets. This
piece of code is difficult to reach remotely, as discussed below.

The buffer overflow happens because `cp1' and `size' get desynchronized.
Normally `cp1' is a pointer into the data buffer that is written to when
processing the packet. `size' is the number of remaining bytes within this
buffer that are unused. However in this case the author of the file forgot
to subtract the number of bytes that are written to the buffer by the
dn_expand function from the `size' variable. So `size' still denotes the
number of bytes remaining in the buffer, before dn_expand was called,
while `cp1' has been moved. This allows one to overflow the buffer by as
much as MAXDNAME (1025) bytes, minus a few in the following memcpy call.

At position #1 this line has to be inserted to fix the problem:

		size -= n;

Sometimes, the problem lies in the invisible code, forgotten to be written.

### bug 2

BIND-8.2.3-REL/src/bin/named/ns_update.c:1655

	case T_NXT:
		n = dn_expand(msg, eom, cp, (char *)cp1, size);
		if (n < 0 || (u_int)n >= dlen)
			return (0);
		size -= n;
/* #1 */
		cp += n;
		n1 = dlen - n;
		n = strlen((char *)cp1) + 1;
		cp1 += n;
		/*
		 * The first bit of the first octet determines the format
		 * of the NXT record.  A format for types >= 128 has not
		 * yet been defined, so if bit zero is set, we just copy
		 * what's there because we don't understand it.
		 */
		if ((*cp & 0x80) == 0) {
			/*
			 * Bit zero is not set; this is an ordinary NXT
			 * record.  The bitmap must be at least 4 octets
			 * because the NXT bit should be set.  It should be
			 * less than or equal to 16 octets because this NXT
			 * format is only defined for types < 128.
			 */
			if (n1 < 4 || n1 > 16)
				return (0);
		}
		if (n1 > size)
			return (0);
		memcpy(cp1, cp, n1);
		cp1 += n1;
		return (cp1 - cp1init);

Here the authors of BIND fall for a mistake by not knowing the meaning of
their own API. The `dn_expand' function returns the number of bytes the
source processing pointer was advanced. But it may write as much as `size'
bytes to the output buffer, which is given by pointer `cp1'.

Hence, `size' is getting decreased by the wrong value, the number of bytes
the source pointer was moved, while the destination pointer `cp1' is moved
correctly. This can lead to a buffer overflow condition in which up to
MAXDNAME (minus a few) bytes can be overwritten outside of the buffer.

The UPDATE code is difficult to reach and requires though ACL checks to be
passed. Even then, behind the buffer you can overflow there are no important
informations which can be used to increase your current access level. To
conclude: this buffer overflow may be interesting, but is most likely not
exploitable except in very special cases (architectures with different stack
layouts, priviledge to issue UPDATE packets, ...).



ProFTPd messed code

proftpd-1.2.0rc2/modules/mod_ls.c:333

          char *p = nameline + strlen(nameline);
	  ...
          snprintf(p, sizeof(nameline) - strlen(nameline) - 4, " -> %s", l);

Where nameline is a local stack buffer:

	char nameline[MAXPATHLEN + MAXPATHLEN + 128] = {'\0'};

The problem manifests itself if strlen (nameline) > (sizeof (nameline) - 3),
since then the length parameter to snprintf turns negative, behaving like a
normal sprintf function. The nameline buffer is printed to before, with
filemodes and name information, but it seems not possible to reach a length
of more then 124 bytes. However, this bug shows that nasty sign conversion
bugs are still within popular code in use today.

===============================================================================