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)".
--
Sébastien Marie
On Wed, Dec 10, 2014 at 10:25:11PM +1100, Jonathan Gray wrote:
> On Wed, Dec 10, 2014 at 11:46:57AM +0100, Sébastien Marie wrote:
> > On Wed, Dec 10, 2014 at 11:16:21AM +0100, Sébastien Marie wrote:
> > > Hi,
> > >
> > > In compile_flags, the variable holding the filename ('w' flag of 's'
> > > command) is an array with PATH_MAX length.
> > >
> > > We should check the size of wanted filename, before copying it in wfile.
> > >
>
> 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 10 Dec 2014 11:18:14 -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 + 1 >= eq)
> + err(COMPILE, "wfile too long");
> *q++ = *p++;
> }
> *q = '\0';
