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

[kevin lochner]

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.
>
>
>
>

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