Re: [PATCH 1/3] client: sanitize FIO_NET_CMD_SENDFILE requests

[Jens Axboe <axboe@kernel.dk>]

On 9/17/26 2:46 PM, Eric Sandeen wrote:
> On 9/17/26 3:00 PM, Jens Axboe wrote:
>> On 9/17/26 1:41 PM, Eric Sandeen wrote:
>>> A legitimate fio server should only be requesting files from the
>>> client's cwd; reject absolute paths or any paths above the cwd.
>>>
>>> Since this adds yet another failure mode, move fail: to the end of
>>> the function as is a bit more typical for a goto fail;
>>>
>>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>>> ---
>>>  client.c | 23 ++++++++++++++++++-----
>>>  1 file changed, 18 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/client.c b/client.c
>>> index 374a744a..b32c2852 100644
>>> --- a/client.c
>>> +++ b/client.c
>>> @@ -1809,12 +1809,18 @@ static int fio_send_file(struct fio_client *client, struct cmd_sendfile *pdu,
>>>  	size = sizeof(*rep);
>>>  	rep = malloc(size);
>>>  
>>> +	/* Sanitize requested path to reject files not under cwd */
>>> +	if (pdu->path[0] == '/' ||
>>> +	    strstr((const char *)pdu->path, "..") != NULL) {
>>> +		log_err("fio: rejecting sendfile request for unsafe path: %s\n",
>>> +			(const char *)pdu->path);
>>> +		rep->error = EPERM;
>>> +		goto fail;
>>> +	}
>>
>> Won't this potentially break existing use cases?
> TBH I did this a while ago and never sent it, so let me re-check. I thought
> I had considered that at the time...
> 
> OK yeah, I guess it is too broad, I suppose if a job name has ".." in it,
> this would  cause improper rejections. I don't think anything legitimately
> tries to grab files on relative paths, though.
> 
> I'm guessing you think this isn't worth it either, and if so again I'm
> happy to just drop it. Or I could tighten it up to only catch things that
> really try to grab arbitrary paths outside of cwd.
> 
> I'm also leaning towards not worth it. :)

Not worth it.

-- 
Jens Axboe