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: }