Monthly Archives: May 2012

Multiplexing Git Hooks

Git hooks are great, especially in larger companies – they allow you to do everything from pre-commit syntax checking to post-push IRC reporting and more. The way Git implements its hooks (as scripts with specific names), however, has a drawback: it’s nontrivial to have a given hook event call multiple different scripts.

For small-time operations, this often isn’t much of a problem – everything can just get put into the same hook script. As my employer’s usage of Git has expanded along with its engineering team, however, we find ourselves wanting to be able to do more mix-and-matching of various hook functionality across different repositories. The ideal situation would be to have one script per functional item, and simply tell Git which ones to call for a given repository + event combination.

I created for this purpose. It’s not all that complex (less than 20 lines of bash when you omit the comments), but it provides very handy functionality. Here’s how it works:

First, symlink the proxy script into the repository’s hooks directory, named like the hook event you want to proxy…

cd /path/to/repo/.git/hooks
ln -s /path/to/ pre-commit

Second, in the same directory symlink in each of the actual hook scripts you want to run, named like the hook event plus a suffix:

ln -s ~/ pre-commit-01-check-syntax
ln -s ~/ pre-commit-02-fix-style

And you’re done! The proxy script will automatically find the suffixed hooks and run them in sorted order (hence the usage of -01-… and -02-…) when that hook event occurs. You can repeat the process for each other hook event you want to multiplex. For hooks where the exit code matters, the proxy still runs all the matching scripts, but aggregates the exit codes – if one or more scripts exited nonzero, the proxy will also exit nonzero.

Yet Another Way You Shouldn’t Implement Password Recovery

A couple of weeks ago I had the dubious pleasure of calling a company to tell them that they had a security vulnerability in their site that you could drive a truck through. I’m not going to name names, but given that the issue is now patched, I think it’s a good lesson on things to avoid when designing your own application.

I’ll show you the relevant source code first and let you see if you can figure it out.

// forgot.php ("forgot password" functionality)

if(!empty($_POST['username'])) {
    // send the request
    CURLHandler::Post(LOGIN_APP_URL . 'resettoken', array('username' => $_POST['username']));
    $result = ob_get_contents();

    $result = json_decode($result);
    if ($result->success == true) {
        $resetUrl = SECURE_SERVER_URL . 'resetpass.php?un=' . base64_encode($_POST['username']) . '&token=' . $result->token;
        $resetUrl = '<a title="Password Recovery" href="' . $resetUrl . '">' . $resetUrl . '</a>';
        sendTemplateEmail($_POST['username'], 'recovery', array('url' => $resetUrl));
        $msg= '<p>Login information will be sent if the email address ' . $_POST['username'] . ' is registered.</p>';
    } else {
        $msg = '<p>Sorry, unable to send password reset information. Try again or contact an administrator.</p>';

There’s actually a couple of things wrong here.

The most problematic flaw (and the one that actually caused the security vulnerability) is that LOGIN_APP_URL happens to point to a server that is accessible to the public internet. This means that anyone who wanted to could bypass this script and make a request directly to its resettoken endpoint, supplying any username they want, and get a valid password reset token for that username! Then all they had to do was manually construct the URL for the reset password page with the username and their acquired token to be able to acquire access to that account.

There’s another more subtle issue here, though: why is an HTTP request being made that returns a password reset token in the first place? What should be happening here is that the same code which generates the reset token should also send the email, thus avoiding the need to expose the reset token generator via an external interface in the first place.

(There’s also the slight “wtf” of using PHP’s output buffer to get the results of a cURL call, rather than using CURLOPT_RETURNTRANSFER, but that’s not a security issue.)