FacebookSignatureUtil.generateSignature() has incorrect sort

[GoogleCodeExporter]

What steps will reproduce the problem?
1. Add a preload fql called friends2
2. Try to verify signature for a request (with new profile)

What is the expected output? What do you see instead?
Signature verification fails

The basic problem is that FacebookSignatureUtil.generateSignature() does
not generate the buffer to encode correctly.  It generates all the
"key=value" pairs and then sorts and concatenates them.

This is not how facebook generates their signature however.  What they do
is sort just the keys, and the concatenate the key=value pairs in the key
sort order.  

This means that if you have two keys, say friends and friends2 (via preload
fql), then this library will sort "friends=xxx" and "friends2=yyy" to
create "friends2=yyyfriends=xxx" (because friends2 comes before friends=),
but facebook would have created "friends=xxxfriends2=yyy" (because friends
comes before friends2)

This is a pretty subtle bug an took me a long time to track down with the
help of the FB security team.

I fixed the issue locally by modifying the method like so:

  public static String generateSignature(Map<String, CharSequence> params,
String secret) {
    StringBuffer buffer = new StringBuffer();

    // Sort by keys, not key=value pairs to get correct sorting of "key"
and "key1"
    List<String> keys = new ArrayList<String>(params.keySet());
    Collections.sort(keys);

    for (String key: keys) {

buffer.append(FacebookParam.stripSignaturePrefix(key)).append('=').append(params
.get(key));
    }

    buffer.append(secret);

    //etc
}

I'm not going to have a chance to push this to svn right now, perhaps
someone else might be able to pick it up?  It is going to change some
signatures that folks might be using unfortunately...  I suppose that
technically we could maintain the old api for generateSignature by using a
special sort comparator that only compares up to the first = rather than
the whole string.  Now that I think about it, that wouldn't be too bad...

Original issue reported on code.google.com by jasper.r...@gmail.com on 28 Aug 2008 at 2:56

[GoogleCodeExporter]

do you have a url pointing to any of their documentation that might back this 
up? 
Just want to add it as a comment if possible.  I'll apply it soon then
(2.0.0-SNAPSHOT), do you need it on the 1.8 branch as well?  Because I can 
check it
in, but I haven't dealt with a release of the older branch yet..

Original comment by fern...@gmail.com on 28 Aug 2008 at 5:09

[GoogleCodeExporter]

I found this doc which seems pretty clear (see: 
"alphabetically_sort_array_by_keys"):
http://wiki.developers.facebook.com/index.php/Authentication_guide#Requesting_Au
thentication

Or at least as clear as the FB docs ever get :)

We are running a custom build locally until 2.0 is ready, so don't push it to 
1.8 on
my account.  Thanks!

Original comment by jasper.r...@gmail.com on 28 Aug 2008 at 6:15

[GoogleCodeExporter]

It looks like there might have been some changes to try to address this, but
generateSignature() still is calling sort on the key=value pairs.  I am just 
going to
update the sort to just sort up to the first "=".  That will maintain backwards
compatibility, but also fix the underlying issue.

Original comment by jasper.r...@gmail.com on 3 Nov 2008 at 4:24

• Changed state: Started

[GoogleCodeExporter]

Original comment by jasper.r...@gmail.com on 3 Nov 2008 at 4:26

• Changed state: Fixed