Hi,

While debugging yet another overrun I came across the StrNCpy macro.
A quick grep of the source tells me that usage of the StrNCpy macro is
seemingly inconsistent.

Usage 1:
strptr = palloc(len);    // done is a diffrent context
ptr = palloc(len + 1);
StrNCpy(ptr, strptr, len + 1);

Usage 2:
NameData name;
StrNCpy(name.data, ptr2name, NAMEDATALEN);

The StrNCpy macro zero terminates the destination buffer.

Usage 1 is gives a read=buffer overrun (which I agree is not the most
serious of bugs
if you system doesn't dump core on it).
Usage 2 makes gives the name a maximum of 31 instead of 32 characters.

Is the maximun name length supposted to be 31 or 32 characters?

With regards from Maurice.


Tue, 31 Mar 1998 11:12:07 +0200
"Maurice Gittens" <mgittens@gits.nl>
StrNCpy
> 
> Hi,
> 
> While debugging yet another overrun I came across the StrNCpy macro.
> A quick grep of the source tells me that usage of the StrNCpy macro is
> seemingly inconsistent.
> 
> Usage 1:
> strptr = palloc(len);    // done is a diffrent context
> ptr = palloc(len + 1);
> StrNCpy(ptr, strptr, len + 1);
> 
> Usage 2:
> NameData name;
> StrNCpy(name.data, ptr2name, NAMEDATALEN);
> 
> The StrNCpy macro zero terminates the destination buffer.
> 
> Usage 1 is gives a read=buffer overrun (which I agree is not the most
> serious of bugs
> if you system doesn't dump core on it).
> Usage 2 makes gives the name a maximum of 31 instead of 32 characters.
> 
> Is the maximun name length supposted to be 31 or 32 characters?

Thanks for checking into these things for us Maurice.  I zero-terminate
all name-type fields, so the max is 31, not 32.

The SQL manual page says:

       Names  in  SQL  are  sequences  of  less  than NAMEDATALEN
       alphanumeric characters, starting with an alphabetic char-
       acter.   By  default, NAMEDATALEN is set to 32, but at the
       time the system is built, NAMEDATALEN can  be  changed  by
       changing  the  #ifdef  in  src/backend/include/postgres.h.
       Underscore ("_") is considered an alphabetic character.

I updated this manual page when I decided to be consistent and always
zero-terminate the NAME type.

So the second usage is OK, the first usage:
	
	> strptr = palloc(len);    // done is a diffrent context
	> ptr = palloc(len + 1);
	> StrNCpy(ptr, strptr, len + 1);

is legally a problem because strNcpy() is doing:

    (strncpy((dst),(src),(len)),(len > 0) ? *((dst)+(len)-1)='\0' :(dummyret)NULL,(void)(dst))

and the call to strncpy() is using len, when that is one too large. 
Now, I know I am going to write over that byte anyway if len >0, so it
is cleaned up, but it is wrong.  I will change the macro to do:

    (strncpy((dst),(src),(len-1)),(len > 0) ? *((dst)+(len)-1)='\0' :(dummyret)NULL,(void)(dst))

or something like that so I check for len == 0.

Here is a patch that does this.  I am applying it now.  Uses the new
macro formatting style:

---------------------------------------------------------------------------

*** ./include/c.h.orig	Tue Mar 31 10:42:36 1998
--- ./include/c.h	Tue Mar 31 10:46:23 1998
***************
*** 703,709 ****
   */
  /* we do this so if the macro is used in an if action, it will work */
  #define StrNCpy(dst,src,len)	\
! 	(strncpy((dst),(src),(len)),(len > 0) ? *((dst)+(len)-1)='\0' : (dummyret)NULL,(void)(dst))
  
  /* Get a bit mask of the bits set in non-int32 aligned addresses */
  #define INT_ALIGN_MASK (sizeof(int32) - 1)
--- 703,717 ----
   */
  /* we do this so if the macro is used in an if action, it will work */
  #define StrNCpy(dst,src,len)	\
! ( \
! 	((len) > 0) ? \
! 	( \
! 		strncpy((dst),(src),(len)-1), \
! 		*((dst)+(len)-1)='\0' \
! 	) \
! 	: \
! 		(dummyret)NULL,(void)(dst) \
! )
  
  /* Get a bit mask of the bits set in non-int32 aligned addresses */
  #define INT_ALIGN_MASK (sizeof(int32) - 1)

-- 
Bruce Momjian                          |  830 Blythe Avenue
maillist@candle.pha.pa.us              |  Drexel Hill, Pennsylvania 19026
  +  If your life is a hard drive,     |  (610) 353-9879(w)
  +  Christ can be your backup.        |  (610) 853-3000(h)

Tue, 31 Mar 1998 10:51:34 -0500 (EST)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] StrNCpy