Security flaw #37

Closed
si14 opened this Issue Oct 17, 2011 · 8 comments

Projects

None yet

5 participants

@si14

When user opens page with content like

body() ->
    wf:comet(fun() -> counter(1) end),
    #panel { id=p_container, body=[
        #label { text="Messages so far:" },
        #panel { id=p_messages }
    ]}.

counter/1 is spawned, but never killed if user didn't load .js files for some reason.

@anha0825

Yes, I've seen this as well. In the end of action_comet:accumulator_loop/4 is code that should handle this but is doesn't trigger if the comet process pushes anything to the page (that send a {add_actions, NewActions} to the accumulator.

@si14

In fact I can see this behaviour even with enabled JS.

It is a serious problem for my usage of Nitrogen.

@si14

Ok, I've been wrong, processes are killed after timeout. The question is — why not to kill them after http client disconnect?

@anha0825

I have a accumulator process (from action_comet.erl) that aren't killed.
Add http://nitrogenproject.com/demos/viewsource?module=demos_comet3 to your project and call it from i erlang shell: httpc:request("http://localhost:8000/demos_comet3"). That should reproduce the error.

@AllYouCanAlex

I've spent 6 hours today hunting this bug. No luck. What happens is that on initial JavaScript execution, eventContext is POSTed back to the server, and event(start_async) kicks off in lib/nitrogen_core/src/actions/action_comet.erl

What happens is that when the page gets rendered with JavaScript disabled, the action_comet:event/1 never gets called, and action_comet:flush/0 just keeps on pushing messages into accumulator_loop. I hacked accumulator_loop/4 to detect when there was no javascript executed, because TimerRef is undefined when the process receives {add_actions, NewActions} but erlang:exit({accumulator_loop, undefied}); at that point does nothing.

The bug might not be in the action_comet.erl but in the way the whole Comet process gets handled. I wish somebody could fix this bug.

@choptastic
The Nitrogen Web Framework for Erlang member

Here's my fix for this: For a detailed explaination, see the "Note" section I added at the top of the "INTERFACE" section

choptastic/nitrogen_core@d634b8e

Also, I recommend cherry-picking this for now, rather than pull from my repo, as I have a lot of things in here that are a little "alpha", if you will that need to be finished up before merging into mainline.

@choptastic
The Nitrogen Web Framework for Erlang member

Also, please let me know if you catch something I may have overlooked. Note: my fix here will catch and kill the process after 30 seconds if the javascript still isn't loaded for that Pid.

@choptastic choptastic closed this Nov 8, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment