Re: bin/38201: Potential lack of null termination when copying a string

[David Holland <dholland-bugs%netbsd.org@localhost>]

The following reply was made to PR bin/38201; it has been noted by GNATS.

From: David Holland <dholland-bugs%netbsd.org@localhost>
To: gnats-bugs%NetBSD.org@localhost
Cc: gnats-admin%netbsd.org@localhost, netbsd-bugs%netbsd.org@localhost, 
netbsd%happyjack.org@localhost
Subject: Re: bin/38201: Potential lack of null termination when copying a string
Date: Wed, 12 Mar 2008 13:26:00 +0000

 On Tue, Mar 11, 2008 at 06:45:02PM +0000, David Laight wrote:
  >  On Sat, Mar 08, 2008 at 10:25:46PM +0000, David Holland wrote:
  >  > On Sat, Mar 08, 2008 at 07:55:01PM +0000, netbsd%happyjack.org@localhost 
wrote:
  >  >  > [who/utmpentry.c]
  >  >  >
  >  >  > In two places, the wrong object is null-terminated after a
  >  >  > strncpy(), resulting in a potential lack of null termination.
  >  > 
  >  > Good catch... but it turns out that those null terminations are
  >  > neither necessary nor useful. I will clean up the mess...
  >  
  >  Actually they were also wrong!
  >  
  >     (void)strncpy(e->host, up->ut_host, sizeof(up->ut_host));
  >  -  e->name[sizeof(e->name) - 1] = '\0';
  >  +  e->host[sizeof(e->host) - 1] = '\0';
  >  
  >  Since e->host is guaranteed to be longer than up->ut_host, and strncpy
  >  is used because up->ut_host might not be null terminated, the correct
  >  line would be:
  >     e->host[sizeof up->ut_host - 1] = 0;
  >
  >  Of course, the code worked because the buffer had just been calloc'ed.
 
 As mrg noted, e->host[sizeof up->ut_host] = 0; would be better.
 
 This is why I changed it around the way I did...
 
 -- 
 David A. Holland
 dholland%netbsd.org@localhost