Nmap Development mailing list archives

Re: Nmap uses ctype macros improperly


From: Solar Designer <solar () openwall com>
Date: Tue, 28 Jul 2009 17:23:05 +0400

On Mon, Jul 27, 2009 at 04:43:32PM -0600, David Fifield wrote:
I understand your reason for suggesting (int) (unsigned char), but I
asked Josh to use just (unsigned char). Not being able to index an array
with an unsigned char seems like a compiler or C library bug, and we
have to decide how far we're willing to work around things.

It's not just about "indexing an array" (that's just one possible
implementation), and thus not just about working around bugs.  These
macros are defined to accept an int.  They are free to misbehave when
you pass something other than an int.  Is that not sufficient reason to
make sure that we only pass ints?  Well, if you need a specific example
of how a hypothetical standards-compliant implementation may break with
unsigned char, without any bug in the implementation, here it is:

Suppose the implementation uses per-character bitmasks, with each bit
representing the type of a character.  Suppose the bitmasks are of a
size other than 1 byte - say, they're 2 bytes each.  Finally, suppose
that a single-dimensional array is used.  A ctype macro may then be
implemented like:

unsigned char _bitmasks[0x200];
[...]
#define _LOWER 11
[...]
#define islower(c) \
        ((c) == EOF ? 0 : ((int)_bitmasks[((c) << 1) + (_LOWER >> 3)] & (1 << (_LOWER & 7))))

If "c" turns out to be an unsigned char instead of an int, this will
fail for values of "c" with the 8th bit set.  If the bitmasks are, say,
3 bytes per character, this will fail for some 7-bit values as well.

Indeed, a defensive implementation would cast "c" to an int on its own,
yet strictly speaking in the above scenario the bug would be in the
application, not in the ctype implementation.

Does this make sense?

Alexander

_______________________________________________
Sent through the nmap-dev mailing list
http://cgi.insecure.org/mailman/listinfo/nmap-dev
Archived at http://SecLists.Org


Current thread: