WSF_REQUEST.save_uploaded_file: unsafe

[Berend de Boer]

Hi All,

It seems that save_uploaded_file allows attackers to overwrite files
uploaded by others. I.e. there is a delay between exists file and
creating the file. Code like this is extremely unsafe. I'm guessing
this is perhaps an attempt at portability, but I strongly suggest we
use the OS supplied tmpfile facilities.

Obviously it's easy to replace, which I will do in my code. But I use
eposix (as that gives me access to OS facilities), so not a patch I
can submit, but at least this function should come with a very big
warning IMO.

--
All the best,

Berend de Boer

[Jocelyn Fiat]

Hi Berend,

Indeed this code has to be improved.
I am wondering if tmpfile() is available for all platform, and is it safe in concurrent application?
I am sure it is safer than current code in EWF.

-- Jocelyn

[Berend de Boer]

>>>>> "Jocelyn" == Jocelyn Fiat <jf...@eiffel.com> writes:

Jocelyn> Indeed this code has to be improved. I am wondering if
Jocelyn> tmpfile() is available for all platform.

For a draft implementation, see EPX_WSF_REQUEST:

https://github.com/berenddeboer/eposix/blob/f7f59c70151056bbbdbae05d24d4df9f53cfc8b3/src/ewf/epx_wsf_request.e

I'm using a SUS_ class (SUS_TEMPORARY_FILE), meaning it's based on the
Standard Unix Specification. The routine it uses is mkstemp().

I have an STDC_TEMPORARY_FILE class as well, which uses tmpfile(), an
STDC_ class is available on all platforms with a C compiler, so the
answer is yes, available on all platforms.


Jocelyn> and is it safe in concurrent application?

It is.

[Jocelyn Fiat]

See related issue: https://github.com/EiffelWebFramework/EWF/issues/132

And a patch to address this issue: https://github.com/EiffelWebFramework/EWF/pull/133

This is waiting for comment, and eventually approval.

-- Jocelyn

[Jocelyn Fiat]

This issue had been addressed.

Don't hesitate to comment on the solution committed.

Regards,

-- Jocelyn