netbsd-bugs@netbsd.org
[Top] [All Lists]

bin/31077: /usr/bin/make can read off of end of buffer

Subject: bin/31077: /usr/bin/make can read off of end of buffer
From:
Date: Fri, 26 Aug 2005 22:20:00 +0000 UTC
>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);

<Prev in Thread] Current Thread [Next in Thread>