Re: [patch] sed: missing bound check resulting stack overflow

2014-12-10 Thread Sébastien Marie
On Thu, Dec 11, 2014 at 04:38:50PM +1100, Jonathan Gray wrote: > > Yes, I agree. I plan to commit this version: > It is ok for me. Thanks. Sébastien Marie > Index: compile.c > === > RCS file: /cvs/src/usr.bin/sed/compile.c,v > re

Re: [patch] sed: missing bound check resulting stack overflow

2014-12-10 Thread Jonathan Gray
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. >

Re: [patch] sed: missing bound check resulting stack overflow

2014-12-10 Thread Sébastien Marie
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

Re: [patch] sed: missing bound check resulting stack overflow

2014-12-10 Thread Jonathan Gray
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

Re: [patch] sed: missing bound check resulting stack overflow

2014-12-10 Thread Sébastien Marie
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. > > $ echo | sed -e s/a//w`per

Re: [patch] sed: missing bound check resulting stack overflow

2014-12-10 Thread Sébastien Marie
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. > > $ echo | sed -e s/a//w`per

[patch] sed: missing bound check resulting stack overflow

2014-12-10 Thread Sébastien Marie
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. $ echo | sed -e s/a//w`perl -e "print '_' x 1"` Bus error (core dumped) Found also with afl-fuzz. Th