>Number:         31077
>Category:       bin
>Synopsis:       /usr/bin/make can read off of end of buffer
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    bin-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Aug 26 22:20:00 +0000 2005
>Originator:     Wim L
>Release:        cvs checkout of netbsd-3 tag
>Organization:
HHHH Assoc
>Environment:
OpenBSD underhill.hhhh.org 3.7 UNDERHILL#0 i386
(yes, this is a cross build)
>Description:
The nbmake tool built by the build script has a problem when doing variable expansion on strings with a trailing $: it will skip the NUL and look at the byte following the NUL. Most of the time this byte is also zero so nothing bad happens, but occasionally not, in which case a bunch of garbage gets appended to the expanded value.

I've attached a diff (cvs diff -u) which fixes the problem. Some notes on the patch:

Line 434: Buf_AddBytes() can't actually handle a NULL value. I don't know if val==NULL can happen in practice, but since there was safeguard code there already, I think it's good if it actually works.

Line 1888: This is the main offender. Uh, I guess whoever applies this should remove that printf.

Line 3265: This is an equivalent problem. I haven't been able to exercise this part of the code, and the code surrounding it is more complex than around line 1888, so I'm not as confident that this is correct. Looks good to me, though.

>How-To-Repeat:
Building netbsd for an ARM target on an OpenBSD-3.7-Athlon host. My /bin/sh is pdksh-5.2.14, which has variable expansion which disagrees with build.sh, and the result is that MAKEOBJDIR="$" . Builds will fail at apparently-random points in the process, usually with a perplexing error from the shell.

The simplest workaround is to use bash, which does the variable expansion correctly and so avoids tickling the bug in make.

Looking at the source for netbsd make, it's clear that expansion of a string ending in $ will end up reading past the end of the valid data.

>Fix:
Index: var.c
===================================================================
RCS file: /cvsroot/src/usr.bin/make/var.c,v
retrieving revision 1.92
diff -u -r1.92 var.c
--- var.c       16 Feb 2005 15:11:53 -0000      1.92
+++ var.c       26 Aug 2005 21:57:59 -0000
@@ -434,7 +434,9 @@

     v = emalloc(sizeof(Var));

-    len = val ? strlen(val) : 0;
+    if (val == NULL)
+       val = "";
+    len = strlen(val);
     v->val = Buf_Init(len+1);
     Buf_AddBytes(v->val, len, (const Byte *)val);

@@ -1886,7 +1888,14 @@
     parsestate.oneBigWord = FALSE;
     parsestate.varSpace = ' '; /* word separator */

-    if (str[1] != '(' && str[1] != '{') {
+    if (str[1] == '\0') {
+       /*
+        * Error: A $ at the end of the line can't be a valid var name.
+        */
+       fprintf(stderr, "FLAMING DEATH AVERTED!!!!!!!!!!!!!!!!!111!!!11one [%s]\n", str);
+       *lengthPtr = 1;
+       return (err ? var_Error : varNoError);
+    } if (str[1] != '(' && str[1] != '{') {
        /*
         * If it's not bounded by braces of some sort, life is much simpler.
         * We just need to check for the first character and return the
@@ -3256,7 +3265,13 @@
            if (var != NULL) {
                int expand;
                for (;;) {
-                   if (str[1] != '(' && str[1] != '{') {
+                   if (str[1] == (Byte)0) {
+                       /* A trailing $ is kind of a special case */
+                       Buf_AddByte(buf, str[0]);
+                       str ++;
+                       expand = FALSE;
+                   }
+                   else if (str[1] != '(' && str[1] != '{') {
                        if (str[1] != *var || strlen(var) > 1) {
                            Buf_AddBytes(buf, 2, (const Byte *)str);
                            str += 2;
@@ -3357,8 +3372,8 @@
                    free ((Address)val);
                }
            }
-       }
-    }
+       } /* else() (if variable expansion)  */
+    } /* while(*str) */

     Buf_AddByte (buf, '\0');
     val = (char *)Buf_GetAll (buf, (int *)NULL);