On Wed, Dec 10, 2014 at 02:09:10PM +0100, Sébastien Marie wrote:
> Hi Jonathan,
>
> I think there is a mistake in pointer comparaison (q + 1 >= eq): it
> results we keep two chars at end (whereas only one is necessary for
> '\0').
>
> - eq points to the last cell in array before out-of-bound.
> eq = wfile + sizeof(wfile) - 1;
>
> - q points to the cell that would receive a new character.
>
> The err terminaison should be: when q points to eq (because if we do it,
> there is no more place for '\0').
>
> Actually it is "if (q + 1 >= eq)", so we stop when q+1 points to the
> last cell, or when q points to the cell before the last cell. As it would
> never write to it (we stop): we keep two cells at end.
>
>
> As simple test, defining wfile to "char wfile[2]", don't permit to save
> to any filename, whereas one-char filename should be ok.
>
> $ echo | sed -e s/a//w_
> sed: 1: "s/a//w_": wfile too long
>
> I think the test should be "if (q >= eq)".
Yes, I agree. I plan to commit this version:
Index: compile.c
===================================================================
RCS file: /cvs/src/usr.bin/sed/compile.c,v
retrieving revision 1.36
diff -u -p -r1.36 compile.c
--- compile.c 8 Oct 2014 04:19:08 -0000 1.36
+++ compile.c 11 Dec 2014 05:32:42 -0000
@@ -538,7 +538,7 @@ compile_flags(char *p, struct s_subst *s
{
int gn; /* True if we have seen g or n */
long l;
- char wfile[PATH_MAX], *q;
+ char wfile[PATH_MAX], *q, *eq;
s->n = 1; /* Default */
s->p = 0;
@@ -584,9 +584,12 @@ compile_flags(char *p, struct s_subst *s
#endif
EATSPACE();
q = wfile;
+ eq = wfile + sizeof(wfile) - 1;
while (*p) {
if (*p == '\n')
break;
+ if (q >= eq)
+ err(COMPILE, "wfile too long");
*q++ = *p++;
}
*q = '\0';
