Re: Possible unsafe use of strncat in sbin/sysctl/sysctl.c

[Jimmy Johansson <jimmy+current%Update.UU.SE@localhost>]

On Tue, Aug 23, 2011 at 11:29:32PM +0200, Marc Balmer wrote:
> Am 23.08.11 23:23, schrieb Jimmy Johansson:
> > Hi,
> > 
> > 

[snip, patch that makes eyes bleed]

> 
> I don't like how functions, which return values are ignored, are casted
> to (void).
> 
> Who invented that idiom?  What is that good for, other than making your
> eyes bleed?
> 
> I think it is no more than quieting cc or lint.

I don't care either way, but it seems to be the suggested way to write
code for NetBSD according to KFN. I even think that it mentioned casting
return values from functions where you didn't handle the return value
specifically at some point, but it doesn't do so now.

I don't want the eyes of anymore NetBSD developer to bleed though, so
new patch. :-)

Index: sbin/sysctl/sysctl.c
===================================================================
RCS file: /cvsroot/src/sbin/sysctl/sysctl.c,v
retrieving revision 1.136
diff -u -w -p -r1.136 sysctl.c
--- sbin/sysctl/sysctl.c        3 Aug 2011 01:58:30 -0000       1.136
+++ sbin/sysctl/sysctl.c        23 Aug 2011 21:41:15 -0000
@@ -579,8 +579,8 @@ print_tree(int *name, u_int namelen, str
                for (ni = 0; ni < namelen; ni++) {
                        (void)snprintf(n, sizeof(n), "%d", name[ni]);
                        if (ni > 0)
-                               strncat(gdname, ".", sizeof(gdname));
-                       strncat(gdname, n, sizeof(gdname));
+                               strlcat(gdname, ".", sizeof(gdname));
+                       strlcat(gdname, n, sizeof(gdname));
                }
        }
 
@@ -589,11 +589,11 @@ print_tree(int *name, u_int namelen, str
        else if (add) {
                snprintf(n, sizeof(n), "%d", pnode->sysctl_num);
                if (namelen > 1) {
-                       strncat(gsname, sep, sizeof(gsname));
-                       strncat(gdname, ".", sizeof(gdname));
+                       strlcat(gsname, sep, sizeof(gsname));
+                       strlcat(gdname, ".", sizeof(gdname));
                }
-               strncat(gsname, pnode->sysctl_name, sizeof(gsname));
-               strncat(gdname, n, sizeof(gdname));
+               strlcat(gsname, pnode->sysctl_name, sizeof(gsname));
+               strlcat(gdname, n, sizeof(gdname));
        }
 
        if (Mflag && pnode != &my_root) {

This is uniform with the other strlcat in the file too, so it might be
for the better.

Regards,

Jimmy
-- 
If you don't shoot the bearers of bad news, people will keep bringing it to you.