<html><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">Do we need to know which adapter to use before we can verify the params? Don't other implementation use different params?<div><br></div><div>Maybe we could require each adapter to verify that the request is valid. Would that fix this issue?</div><div><br></div><div>Mike</div><div><br></div><div><br><div><div>On Feb 25, 2009, at 1:32 PM, kevin lochner wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div>I'm not convinced we have a good solution yet. here's the relevant code:</div><div><br></div><div><div></div><blockquote type="cite"><div> def request_comes_from_facebook?</div><div> request_is_for_a_facebook_canvas? || request_is_facebook_ajax? || request_is_fb_ping?</div><div> end</div><div><br></div><div> def request_is_fb_ping?</div><div> !params['fb_sig'].blank?</div><div> end</div><div> </div><div> def request_is_for_a_facebook_canvas?</div><div> !params['fb_sig_in_canvas'].blank?</div><div> end</div><div> </div><div> def request_is_facebook_ajax?</div><div> params["fb_sig_is_mockajax"]=="1" || params["fb_sig_is_ajax"]=="1"</div><div> end</div><div><br></div></blockquote><div><br></div><div>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"]</div><div><br></div><div><blockquote type="cite"><blockquote type="cite"><br>All the malicious user has to do is send a malformed HTTP request similar to:<br><br><a href="http://my.rails.app.com/some_controller/?fb_sig_api_key=you_are_pwned" target="_blank" onclick="return top.js.OpenExtLink(window,event,this)">http://my.rails.app.com/some_controller/?fb_sig_api_key=you_are_pwned</a></blockquote></blockquote><br></div><div>I think the before filter should be:</div><div> </div><div><div><b> def set_adapter</b></div><div><b> Facebooker.load_adapter(facebook_params) if(params[:fb_sig_api_key])</b></div><div><b> end</b></div><div><br></div></div><div>since facebook_params calls verify_signature & ensures the request comes from facebook. </div><div><div><br></div><div>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).</div><div><br></div><div>Also, we throw an exception on a bad signature, so we should have a catch somewhere in the process.</div><div><br></div></div><div>- kevin</div><div><br></div><div><br></div><div><br></div><div><br><div><div>On Feb 25, 2009, at 1:10 PM, David Clements wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">Cool thanks for taking a look. Looks like I can just add a condition to that call<br><br>if request_comes_from_facebook?<br><br>I'll try to get to it later today.<br><br>Dave<br><br><div><span class="gmail_quote">On 2/25/09, <b class="gmail_sendername">vincent chu</b> <<a href="mailto:vincentchu@gmail.com">vincentchu@gmail.com</a>> wrote:</span><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"> Hi David ---<br><br>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.<span class="q"><br> <br clear="all">Cheers,<br><br>Vince<br>----<br><br></span><div><span class="e" id="q_11fae924d2807eb8_2"><div class="gmail_quote"><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><div><div><div><div><div><div><br><div><blockquote type="cite"><font class="Apple-style-span" color="#144FAE"><br></font><br><div><span class="gmail_quote">On 2/24/09, <b class="gmail_sendername">vincent chu</b> <<a href="mailto:vincentchu@gmail.com" target="_blank" onclick="return top.js.OpenExtLink(window,event,this)">vincentchu@gmail.com</a>> wrote:</span><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"> Hi all ---<br><br>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.<br> <br>All the malicious user has to do is send a malformed HTTP request similar to:<br><br><a href="http://my.rails.app.com/some_controller/?fb_sig_api_key=you_are_pwned" target="_blank" onclick="return top.js.OpenExtLink(window,event,this)">http://my.rails.app.com/some_controller/?fb_sig_api_key=you_are_pwned</a><br> <br>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. <br></blockquote></div></blockquote></div></div></div></div></div></div></div><br> <br></blockquote></div><br> </span></div></blockquote></div><br></blockquote></div><br></div></div></div>_______________________________________________<br>Facebooker-talk mailing list<br><a href="mailto:Facebooker-talk@rubyforge.org">Facebooker-talk@rubyforge.org</a><br>http://rubyforge.org/mailman/listinfo/facebooker-talk<br></blockquote></div><br><div apple-content-edited="true"> <span class="Apple-style-span" style="border-collapse: separate; border-spacing: 0px 0px; color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; text-align: auto; -khtml-text-decorations-in-effect: none; text-indent: 0px; -apple-text-size-adjust: auto; text-transform: none; orphans: 2; white-space: normal; widows: 2; word-spacing: 0px; "><div style="word-wrap: break-word; -khtml-nbsp-mode: space; -khtml-line-break: after-white-space; "><div>--</div><div>Mike Mangino</div><div><a href="http://www.elevatedrails.com">http://www.elevatedrails.com</a></div><div><br class="khtml-block-placeholder"></div></div><br class="Apple-interchange-newline"></span> </div><br></div></body></html>