On Fri, Oct 20 2017, Sebastien Marie <sema...@online.fr> wrote:
> On Thu, Oct 19, 2017 at 08:58:12PM +0200, Jan Klemkow wrote:
>>
>> 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 19 Oct 2017 18:27:24 -0000
>> @@ -903,8 +903,17 @@ again:
>>
>> if (rwmap != NULL)
>> rewrite_map(client, filename);
>> - else
>> - tftp_open(client, filename);
>> + else {
>> + char nfilename[PATH_MAX];
>> +
>> + snprintf(nfilename, sizeof nfilename, "%s/%s",
>> + getip(&client->ss), filename);
>
> - filename has PATH_MAX length
> - getip(&client->ss) could have NI_MAXHOST length
INET6_ADDRSTRLEN since getip() calls getnameinfo(NI_NUMERICHOST), but
your point stands.
> so nfilename could be larger than PATH_MAX (sizeof nfilename).
>
> I assume the return of snprintf() need to be checked. if truncation
> occured, a warning should be issued and nfilename discarded (just
> calling tftp_open(client, filename)) ?
I think we should refuse the request if truncated.
>> +
>> + if (access(nfilename, R_OK) == 0)
>> + tftp_open(client, nfilename);
>> + else
>> + tftp_open(client, filename);
>> + }
Here we look up the same file in both the client-specific subdirectory
and the default directory. Should we instead look only in the
client-specific directory if the latter exist?
>>
>> return;
>>
>>
>
> thanks
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
