PATCH: 3.1.6-pws-3: bslashquote() is slightly messed up.

PATCH: 3.1.6-pws-3: bslashquote() is slightly messed up.

[Bart Schaefer]

Sven added a call to VARARR() in bslashquote() to avoid a buffer overflow.
However, that buffer later gets unconditionally dupstring()d, so there's no
reason not to simply allocate it with ncalloc() in the first place.

Then there's this bit of insanity:

    *v = '\0';
    tt = dupstring(buf);
    v += tt - buf;			<-- tt and buf don't point into
    if (e && (sf & 1))			    the same string any more in
	*e += tt - buf;			<-- either of these places!

    if (e && *e == u)
	*e = v;

Possibly the "v += tt - buf" is supposed to relocate v into the same spot
in tt that it previously pointed into buf -- but that's not guaranteed to
work, as ANSI C compilers are not required to do arithmetic on pointers
unless they point into the same allocated block (segmented architectures
and all that sort of rot).  The right thing would be

    v = tt + (v - buf);

But that isn't necessary if dupstring() is avoided in the first place.

I also changed "sf |= 1" to just "sf = 1" as the bit values in sf have not
been significant for some while now (q.v. the chunk of comment I removed at
the top of the function).

Index: utils.c
===================================================================
@@ -2950,9 +2950,7 @@
 
 /* Quote the string s and return the result.  If e is non-zero, the         *
  * pointer it points to may point to a position in s and in e the position  *
- * of the corresponding character in the quoted string is returned.  Like   *
- * e, te may point to a position in the string and pl is used to return     *
- * the position of the character pointed to by te in the quoted string.     *
+ * of the corresponding character in the quoted string is returned.         *
  * The last argument should be zero if this is to be used outside a string, *
  * one if it is to be quoted for the inside of a single quoted string, and  *
  * two if it is for the inside of  double quoted string.                    *
@@ -2964,14 +2962,14 @@
 {
     const char *u, *tt;
     char *v;
-    VARARR(char, buf, 2 * strlen(s) + 1);
+    char *buf = ncalloc(2 * strlen(s) + 1);
     int sf = 0;
 
     tt = v = buf;
     u = s;
     for (; *u; u++) {
 	if (e && *e == u)
-	    *e = v, sf |= 1;
+	    *e = v, sf = 1;
 	if (ispecial(*u) &&
 	    (!instring || (isset(BANGHIST) &&
 			   *u == (char)bangchar) ||
@@ -2998,15 +2996,12 @@
 	*v++ = *u;
     }
     *v = '\0';
-    tt = dupstring(buf);
-    v += tt - buf;
-    if (e && (sf & 1))
-	*e += tt - buf;
 
     if (e && *e == u)
-	*e = v;
+	*e = v, sf = 1;
+    DPUTS(!e || sf, "BUG: Wild pointer *e in bslashquote()");
 
-    return (char *) tt;
+    return buf;
 }
 
 /* Unmetafy and output a string, quoted if it contains special characters. */

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Re: PATCH: 3.1.6-pws-3: bslashquote() is slightly messed up.

[Tanaka Akira]

In article <990912165419.ZM23254@candle.brasslantern.com>,
  "Bart Schaefer" <schaefer@candle.brasslantern.com> writes:

> +    DPUTS(!e || sf, "BUG: Wild pointer *e in bslashquote()");

Hm. Is this really a bug? I think e == NULL is allowed.

Z(2):akr@is27e1u11% Src/zsh -f
is27e1u11% a=abc
is27e1u11% echo $a:q
BUG: Wild pointer *e in bslashquote()
abc
is27e1u11% 
-- 
Tanaka Akira

Re: PATCH: 3.1.6-pws-3: bslashquote() is slightly messed up.

[Bart Schaefer]

On Sep 13,  4:00am, Tanaka Akira wrote:
} Subject: Re: PATCH: 3.1.6-pws-3: bslashquote() is slightly messed up.
}
} In article <990912165419.ZM23254@candle.brasslantern.com>,
}   "Bart Schaefer" <schaefer@candle.brasslantern.com> writes:
} 
} > +    DPUTS(!e || sf, "BUG: Wild pointer *e in bslashquote()");
} 
} Hm. Is this really a bug? I think e == NULL is allowed.

Hm.  It's really tricky to write the conditions for DPUTS(), partly because
I keep thinking in terms of assert(), of which DPUTS() is the reverse.

Index: utils.c
===================================================================
@@ -2999,7 +2999,7 @@
 
     if (e && *e == u)
 	*e = v, sf = 1;
-    DPUTS(!e || sf, "BUG: Wild pointer *e in bslashquote()");
+    DPUTS(e && !sf, "BUG: Wild pointer *e in bslashquote()");
 
     return buf;
 }

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com