Menu

Security Review 4 – favour for a neighbor

June 29, 2019 - Security Review
By: jale

Security Review: Review the code and find the bugs that would otherwise pass a functional code review or tests, but have serious security flaws.

Here we have some code from an Node.js Express web server. It is responsible for transferring funds from one user to another. This particular code is susceptible to a few different but related attacks. What are the vulnerabilities and how can this be made secure?

Any functions not explicitly defined in the code can assumed to be imported from elsewhere and secure.

// Uses Node.js Express

/**
 * [POST] /transfer_funds
 * transfers an amount of funds from the currently logged in user to the specified user
 * params:
 * to - the user to transfer funds to
 * amount - the amount to transfer
 */
app.route('/transfer_funds')
    .post((req, res) => {
        // make sure the user is authorized to perform this action
        if (!user_authorized(req.session.user, req.path)) {
            res.status(403).send();
            return;
        }

        // make sure correct parameters were submitted
        if (!['to', 'amount'].every(k => k in req.body)) {
            res.status(400).send();
            return;
        }

        // transfer the funds using the logged in user as the source
        if (transfer_funds(req.session.user, req.body.to, req.body.amount) === false) {
            res.send("An error occured, funds were not transferred");
            return;
        }

        res.send("funds transfered successfully");
    });

Are we guaranteed that a user intended to make the transfer?

Vulnerability

The code here is vulnerable to Cross Site Request Forgery with a kicker of not preventing multiple submissions.

CSRF is a type of malicious exploit of a website where unauthorized commands are transmitted from a user that the web application trusts. Most commonly this vulnerability renders itself as a victim clicking something on an evil site that has unintended side effects on another site.

As an example, say the above vulnerable code was running on foobarbank.com. An attacker can create the following page and host it elsehwere.

<form action="https://foobarbank.com/transfer_funds" method="POST">
    <input type="hidden" name="to" value="evil_user">
    <input type="hidden" name="amount" value="1000000">
    <input type="submit" value="Click here for free beer!">
</form>

Now the attacker directs the victim to the page, and the victim seeing the button for free beer, obviously clicks it. The victim has just unknowingly submitted a request to foobarbank.com to transfer_funds, and, if they’re logged in on that site, have just transferred funds from themselves to the attacker.

Now to address the multiple submission bug; while not strictly a security vulnerability, it is closely related. The issue is that the same action (transfer_funds) may be unintentionally or maliciously repeated without the user being aware. Say for example the user clicks a legitimate button that submits a form to /transfer_funds but it takes a little while to load. If they’re impatient they may click that button again – or hammer it – but as the server hasn’t yet sent the response back to the user, they may assume nothing has happened. Meanwhile the server has received a new request every time the user clicked the button and transferred the funds, but is getting stuck taking time sending the response to the user.

Securing The Code

There’s a several different ways to make this code more secure.

The first thing to do is implement a CSRF token. This is a unique identifier that is given to a user who wishes to perform a transaction, it is then submitted back to the server with the transaction and consumed in doing so. A common implementation for web form based transactions is to include the token in the form as a hidden field so that when the form is submitted, the token is sent along with it. An important detail is that the token is consumed once the transaction proceeds and a new one generated as necessary. Reusing the same token means an attacker may replay a token if they were able to recover one from a past transaction. Generating a new token also prevents the multiple submission bug as the token would be consumed and the transaction proceed only once.

There are important details regarding the CSRF token and implementation that should be obvious, but are worth going over anyway. One is that the token itself be unique and unpredictable; it’s very weak if an attacker can predict a token or reuse an existing one. Another subtle implementation detail is that it must be transmitted alongside the data, so that it may be used to verify the data. I say this because I had witnessed an implementation of CSRF tokens where the developer had been putting the token into the session. This may ostensibly seem more secure as session data is typically opaque and thus there’s less opportunity for the token to be leaked, but doing so completely voids it’s use. The CSRF attack is still valid as the token will automatically be pulled from the victim’s session.

Further mitigation against CSRF include implementing a referrer check. If the flow of the web application is such that the user submit a form on the site, then it would be wise to have the server-side code check the referrer sent by the user-agent to ensure the request originated from the expected form. A referrer check is not a sole solution as there still exist methods to forge the request, however, implementing a referrer check will reduce the attack surface.

As a matter of UX it is a good idea to disable the form submission button and show a loading indicator when a user submits a form, especially if replays are a concern. However, these are only front-end measures and will not alone resolve the multiple submission issue.

Lessons Learned

The key thing to understand here is that basic implementation of requirements may leave holes. If a someone asked for a feature to transfer funds from a logged in user to another, then the above vulnerable code certainly fills that requirement, but it takes mindfulness beyond the raw requirements to ensure the implementation is secure. Any good development involves some forethought into how someone may break your code, not only to prevent bugs and catch edge cases, but to ensure that whatever code is being written is done in a secure manner.

I recommend checking out the following pages for more information on CSRF, such as other mitigations and how attackers find work-arounds for current ones. CSRF is also closely related to Cross-Origin Request Sharing, so it’s a good idea to have an understanding of CORS as well.