Re: tftpd(8): diff for ip path rewrite

[Sebastien Marie]

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