[Facebooker-talk] Possible Security Hole in Facebooker -- Please Update!

[Mike Mangino]

Do we need to know which adapter to use before we can verify the  
params? Don't other implementation use different params?

Maybe we could require each adapter to verify that the request is  
valid. Would that fix this issue?

Mike


On Feb 25, 2009, at 1:32 PM, kevin lochner wrote:

> I'm not convinced we have a good solution yet.  here's the relevant  
> code:
>
>>       def request_comes_from_facebook?
>>         request_is_for_a_facebook_canvas? ||  
>> request_is_facebook_ajax? || request_is_fb_ping?
>>       end
>>
>>       def request_is_fb_ping?
>>         !params['fb_sig'].blank?
>>       end
>>
>>       def request_is_for_a_facebook_canvas?
>>         !params['fb_sig_in_canvas'].blank?
>>       end
>>
>>       def request_is_facebook_ajax?
>>         params["fb_sig_is_mockajax"]=="1" ||  
>> params["fb_sig_is_ajax"]=="1"
>>       end
>>
>
> So calling request_comes_from_facebook isn't really a security  
> feature, and doesn't change the behavior w.r.t. the test case that  
> vince outlined, since they could just as easily fake params["fb_sig"]
>
>>>
>>> All the malicious user has to do is send a malformed HTTP request  
>>> similar to:
>>>
>>> http://my.rails.app.com/some_controller/? 
>>> fb_sig_api_key=you_are_pwned
>
> I think the before filter should be:
>
>       def set_adapter
>         Facebooker.load_adapter(facebook_params)  
> if(params[:fb_sig_api_key])
>       end
>
> since facebook_params calls verify_signature & ensures the request  
> comes from facebook.
>
> Alternatively, we could be verify the params signature in  
> request_comes_from_facebook.  That may be a better solution since  
> the request_comes_from_facebook method is a little misleading (as  
> david has just demonstrated).
>
> Also, we throw an exception on a bad signature, so we should have a  
> catch somewhere in the process.
>
> - kevin
>
>
>
>
> On Feb 25, 2009, at 1:10 PM, David Clements wrote:
>
>> Cool thanks for taking a look.  Looks like I can just add a  
>> condition to that call
>>
>> if request_comes_from_facebook?
>>
>> I'll try to get to it later today.
>>
>> Dave
>>
>> On 2/25/09, vincent chu <vincentchu at gmail.com> wrote:
>> Hi David ---
>>
>> I took a look at your fix. Though I'm somewhat unfamiliar with  
>> exactly what you want to do, I think it would be prudent to  
>> validate that the incoming params hash actually originates from  
>> facebook before using the parameters to reset the adapter. This  
>> way, you never touch the adapter until you're sure that it's  
>> Facebook sending the request, and not some malicious actor.
>>
>> Cheers,
>>
>> Vince
>> ----
>>
>>
>>>
>>>
>>> On 2/24/09, vincent chu <vincentchu at gmail.com> wrote:
>>> Hi all ---
>>>
>>> In the course of developing our Facebook connect app, we realized  
>>> that there was a  security hole in Facebooker that allows any  
>>> malicious user to change the state of the Facebooker module and  
>>> crash any controller/view that uses Facebooker to capture a  
>>> Facebook session. For Facebook connect apps, this could  
>>> potentially be in any view that uses the "set_facebook_session"  
>>> before_filter.
>>>
>>> All the malicious user has to do is send a malformed HTTP request  
>>> similar to:
>>>
>>> http://my.rails.app.com/some_controller/? 
>>> fb_sig_api_key=you_are_pwned
>>>
>>> The problem comes in the 'set_adapter' method of 'facebooker/lib/ 
>>> facebooker/rails/controller.rb' where Facebooker will attempt to  
>>> load an adapter from the params hash if fb_sig_api_key is in the  
>>> request (ignoring the configuration found in the facebooker.yml  
>>> file). In this case, Facebooker would dutifully set the api_key to  
>>> "you_are_pwned" and any subsequent call to Facebooker would try  
>>> and use "you_are_pwned" as the api_key, causing it to crash the  
>>> site.
>>
>>
>>
>>
>
> _______________________________________________
> Facebooker-talk mailing list
> Facebooker-talk at rubyforge.org
> http://rubyforge.org/mailman/listinfo/facebooker-talk

--
Mike Mangino
http://www.elevatedrails.com



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://rubyforge.org/pipermail/facebooker-talk/attachments/20090225/f472b09c/attachment.html>