Black Hat with Amazon.com–2011 Code Challenges II

The second day of Black Hat, and we’re showing three coding challenges. [Sorry this post took a while to make – but I hope you’ll agree that the new code formatting makes it easier to read]

As usual, the challenge is to figure out where the flaws lie, and for bonus points, to decide how you would prevent such flaws from happening throughout your own enterprises.

If you want to skip the code challenges, and get straight to the rewards, email security-ninjas@amazon.com or http://security.amazon-jobs.com/jobs.html to get details on the job opportunities we have for all manner of security professionals in Seattle, Dublin, Virginia and Bangalore.

Challenge III – GetSignInResult

Here’s a short stretch of code from an authentication function – how long does it take you to spot the security flaw?

   1: SignInResult *getSignInResult(


   2:         const UserViewImpl *in,


   3:         const char *password )


   4: {


   5:     LdapAuthenticator *ldap = LdapAuthenticator::getInstance();


   6:     SignInResult *out = new SignInResult();


   7:     if( in == NULL || in->getUserId() == NULL ) { 


   8:         LogDebug( kFuncName.c_str(), "UserNotFound" );


   9:         out->setStatus( SignInStatus::UserNotFound );


  10:         return out;


  11:     }


  12:     if( !in->getAccountId() ) {


  13:         LogDebug( kFuncName.c_str(), "AccountNotFound" );


  14:         out->setStatus( SignInStatus::AccountNotFound );


  15:         return out;


  16:     }


  17:     out->setUserId( in->getUserId()->getId());


  18:     out->setAccountId( in->getAccountId()->getId());


  19:     AuthCodec *authCodec = AuthCodec::getInstance();


  20:     AuthToken authToken ( authCodec, in->getAccountId()->getId() );


  21:     out->setAuthToken( authToken.getEncodedAuthToken() );


  22:     CSRFToken csrfToken( authCodec );


  23:     out->setCsrfToken( csrfToken.getCSRFToken() );


  24:     if( !ldap->authenticateLdapUser(in->getUserName(), password)) {


  25:         LogDebug( kFuncName.c_str(), "LDAPBadPassword" );


  26:         out->setStatus( SignInStatus::BadPassword );


  27:         return out;


  28:     }        


  29:     out->setStatus( SignInStatus::Success );


  30:     LogDebug( kFuncName.c_str(), "Success" );


  31:     return out;


  32: }

Challenge IV – IsDifferent

This is a slightly modified version of last year’s “IsDifferent” challenge, mostly it’s the same thing, but if it’s new to you, it’s still new.

I’m embarrassed to say that at the conference, we briefly had a bug in our own code. That bug has been corrected below.

   1: bool isDifferent(


   2:     SomeClass const * newObject,


   3:     SomeClass const * oldObject) const


   4: {


   5:     // Returns true if newObject is different from oldObject.


   6:  


   7:     // Shortcut if same pointer.


   8:     return newObject != oldObject &&


   9:  


  10:     // Protect equals from being called on NULL.


  11:         newObject != 0 &&


  12:  


  13:     // Return the result of "equals"


  14:         ! newObject->equals(oldObject);


  15: }

Challenge V – MultithreadRun

This was perhaps the one with the most extraneous flaws in it.

Yes, the cmd could be altered between the “is_user_admin” check and the cmd_copy line. That might happen, but the WRITE and DELETE commands still would be disallowed if the user is not an admin. Maybe this code operates in an environment where it’s safe for this to happen – perhaps admins only call WRITE or DELETE, and maybe WRITE and DELETE only execute queued operations that have already been approved and are merely waiting for an admin to choose the right time. OK, that’s a stretch, but the hint here is that this is NOT the bug we are looking for.

So, what is the bug we are looking for? Ask more questions, and I’ll answer them here – but of course, I won’t post the answer to any of these code challenges for a few weeks.

   1: #define READ      1


   2: #define WRITE     2


   3: #define DELETE    3


   4:  


   5: int cmd;


   6:  


   7: void multithread_update_cmd(int new_cmd)


   8: {


   9:     cmd = new_cmd;


  10: }


  11:  


  12: int multithread_run(USERCONTEXT *pContext, CMDCONTEXT *pCmd)


  13: {


  14:     bool admin = is_user_admin(pContext);


  15:     // Copy the command so as to prevent it being altered


  16:     // by another thread.


  17:     int cmd_copy = cmd;


  18:  


  19:     // Only admins can perform writes and deletes


  20:     if(!admin && (cmd_copy == WRITE || cmd_copy == DELETE))


  21:         return -1;


  22:  


  23:     switch(cmd_copy)


  24:     {


  25:     case READ:


  26:         perform_read(pCmd);


  27:         break;


  28:     case WRITE:


  29:         perform_write(pCmd);


  30:         break;


  31:     case DELETE:


  32:         perform_delete(pCmd);


  33:         break;


  34:     default:


  35:         return -1;


  36:  


  37:     }


  38:  


  39:     return 0;


  40: }

One Response to Black Hat with Amazon.com–2011 Code Challenges II

  • carlos says:

    Only a year late, but I can’t resist a challenge!  Did you ever post the answers anywhere?  Here’s my take anyway.

    II: Broken with negative numbers, e.g. “-1.2″ gives -0.8.

    III: Sets auth tokens before validating the user.  If the caller doesn’t check the status correctly the user could be logged in with a bogus password.

    IV: The boolean expression should be disjunctive.

    V: Assuming there isn’t a load of external synchronization between the two functions, there’s no connection between the user and the command they want to execute.  Global variables combined with multithreading have a very bad smell.

    Oh yeah, and I: It’s Perl :)

Leave a Reply

Your email address will not be published. Required fields are marked *

You may use these HTML tags and attributes: <a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <strike> <strong>