-
Notifications
You must be signed in to change notification settings - Fork 286
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Any controller will accept an uploaded file even if there's no POST method and put it in scratch/ #656
Comments
I think this is mainly a SimpleBridge issue, but there may be away to control SimpleBridge so it doesn't do this. The quickest workaround I found was to add a 'POST' handler that deletes the file. |
Yeah this one also surprised me. The same behaviour does also apply to simple_bridge_handler_sample.erl, which could be easily reproduced with the > make run_inets
...
> curl -X GET http://127.0.0.1:8000/peer_ip -F 'file1=@<pathh_to_file>'
...
> ls -l scratch/
total 3520
-rw-r--r-- 1 david staff 1.7M Nov 5 15:44 78-200-77-73-91-82-21-199-130-108-240-188-19-104-115-117 Not sure if it's some kind of default case misbehaviour or maybe even intentional. Maybe @choptastic could be so kind and give us some details on how it's intended? @chatterbeak we came up with another workaround which seems IMHO to be a little bit more complete and prevents from forgetting some controller endpoints. We're using a -module(some_boss_filter).
-export([
after_filter/3
]).
after_filter({StatusCode, Payload, Headers}, _FilterConfig, RequestContext) ->
case lists:keyfind(request, 1, RequestContext) of
{request, Request} ->
handle_request_cleanup(Request);
false ->
lager:error("Couldn't find request in context for final data cleanup", [RequestContext])
end,
{StatusCode, Payload, Headers};
after_filter(Other, _FilterConfig, _RequestContext) ->
%% Will match for 500er errors
Other.
%%--------------------------------------------------------------------
%%% Internal functions
%%--------------------------------------------------------------------
handle_request_cleanup(Request) ->
lists:foreach(fun(Attachment) ->
{LogLevel, DeleteResult} =
case file:delete(sb_uploaded_file:temp_file(Attachment)) of
{error, enoent} ->
%% The file was already deleted from business logic so only a debug log is needed
{debug, file_already_deleted};
OtherReturn ->
{notice, OtherReturn}
end,
lager:log(LogLevel, self(), "Deleting remaining temporary file ~p with name ~p, size ~p byte and "
"field_name ~p from scratch folder; result: ~p", [
sb_uploaded_file:temp_file(Attachment),
sb_uploaded_file:original_name(Attachment),
sb_uploaded_file:size(Attachment),
sb_uploaded_file:field_name(Attachment),
DeleteResult
])
end, Request:post_files()). and in in the [{boss, [
...
{controller_filter_modules, [
some_boss_filter
]},
...
]}] |
Thanks for the ideas. I noticed this because various "malicious" php scripts were appearing in the scratch directory on some of my ChicagoBoss sites. Fortunately, the couldn't really do harm. But if you look at the constant barrage of PHP exploit attempts that any public website sees, it's common for these exploit suites to try uploading files to "/" and seeing if they can get them to run. |
I tried uploading a file to the default / handler in my application with the following script. (cat.jpg is just a file sitting in the same directory as this script)
There's no POST handler at route /
A scratch directory is created:
and the file is there:
It would probably be a good idea to at least check to see if there's a Post Method before allowing this.
Also, the user wasn't logged in, so the before_ method actually redirected to my login controller. And the file still made it up!
I think I can handle this in the before_, but I didn't expect this. I noticed it when some systems we have in production were collecting all sorts of "malware" in this scratch directory. (Fortunately, none of this PhP malware has any power in this directory and in the Erlang Universe.)
The text was updated successfully, but these errors were encountered: