On Sat, Oct 21, 2017 at 10:10:39PM +0200, Jan Klemkow wrote: > > Common files should be found in the default directory. But, host > specific files could be overwritten if they exist in the subdirectory. > > The diff below should address all comments. > > Index: tftpd.c > =================================================================== > RCS file: /mount/openbsd/cvs/src/usr.sbin/tftpd/tftpd.c,v > retrieving revision 1.39 > diff -u -p -r1.39 tftpd.c > --- tftpd.c 26 May 2017 17:38:46 -0000 1.39 > +++ tftpd.c 21 Oct 2017 19:49:04 -0000 > @@ -903,7 +911,20 @@ again: > > if (rwmap != NULL) > rewrite_map(client, filename); > - else > + else if (iflag) { > + char nfilename[PATH_MAX]; > + > + if (snprintf(nfilename, sizeof(nfilename), "%s/%s", > + getip(&client->ss), filename) > PATH_MAX - 1) { > + ecode = EBADOP; > + goto error; > + }
snprintf(3) could return -1 on error. the snprintf(3) man page document the proper secure idiom as: int ret = snprintf(nfilename, sizeof(nfilename), "%s/%s", getip(&client->ss), filename); if (ret == -1 || ret >= sizeof(nfilename)) { ecode = EBADOP; goto error; } regarding the error EBADOP (Illegal TFTP operation), I am unsure if it is the proper error code for this case. ENOTFOUND (File not found) or EACCESS (Access violation) seems more indicated to me. but I am not familiar with tftpd protocol and expectations regarding these error codes. thanks. -- Sebastien Marie