Welcome back to “Code of the Week,” where we delve into the subtle yet impactful vulnerabilities lurking in our codebases. This week, we’re exploring a common yet often overlooked issue in web applications.

The code

The challenge is to spot the bug in the following Flask application code. Assume this snippet is part of a larger application where users submit codes (more largely part of a gift) to receive points:

@app.route('/<gift_id>')
@jsonify_response
def submit(gift_id):
    body = request.get_json()
    gift = api.find_gift(gift_id)
    
    if "code" not in body:
        return False, "missing code"
    if gift is None:
        return False, "unable to find gift"
    if user.has_claimed(gift.id):
        return False, "already claimed"

    if gift.check_code(body['code']):
        database.run("UPDATE users SET points = points + ? WHERE id=?", gift.points, user.id)
        return True, ""
    
    return False, "wrong code"

The bug

The race condition in this snippet arises when multiple requests to submit a flag for the same challenge are processed simultaneously. If two requests check whether the user has already solved the challenge at the same time, they may both pass the user.has_claimed(gift.id) check, leading to double counting of points.

Here’s a detailed breakdown of the issue: 1) User submits the same gift twice in quick succession. 2) Both requests pass the user.has_claimed(challenge.id) check before the database update occurs. 3) Both requests proceed to update the user’s points, resulting in double points for a single gift. This race condition allows a user to gain more points than intended by submitting the flag multiple times in quick succession.

The fix

To resolve this race condition, we need to ensure that the check and update operations are atomic, meaning they are executed as a single, indivisible operation. One approach to achieve this is by using database transactions and row-level locks to prevent concurrent modifications.

Here’s a revised version of the function using a transaction:

@app.route('/<gift_id>')
@jsonify_response
def submit(gift_id):
    body = request.get_json()
    challenge = api.find_challenge(gift_id)

    if "code" not in body:
        return False, "missing code"
    if challenge is None:
        return False, "unable to find gift"
    
    # Begin a transaction
    with database.transaction():
        if user.has_claimed(gift.id):
            return False, "already claimed"

        if challenge.check_code(body['code']):
            # Lock the row and update points atomically
            database.run("UPDATE users SET points = points + ? WHERE id=? AND NOT EXISTS (SELECT 1 FROM claimed_gift WHERE user_id=? AND gift_id=?)",
                         challenge.points, user.id, user.id, challenge.id)
            database.run("INSERT INTO claimed_gift (user_id, gift_id) VALUES (?, ?)", user.id, gift.id)
            return True, ""

    return False, "wrong code"

In this updated version, we: 1) Begin a database transaction to ensure atomicity. 2) Use a conditional update query to update the user’s points only if the gift has not already been claimed. 3) Insert a record into the claimed_gift table to keep track of claimed gifts, ensuring that future requests will recognize the gift as already been claimed.

The conclusion

In this fifth installment of “Code of the Week,” we’ve examined a subtle yet impactful race condition in a Flask web application. Race conditions can lead to inconsistent application states and unexpected behavior, making it crucial to identify and address them effectively.

By understanding the nature of race conditions and employing techniques like database transactions and atomic operations, we can build more robust and secure web applications. Stay tuned for the next installment, where we’ll continue our mission to uncover and mitigate hidden vulnerabilities in our code.

Remember, the essence of this series is to ensure that “a bug found once should never be found again.” Let’s continue to build a culture of security-minded development, where every piece of code is scrutinized not just for functionality but for its potential to compromise security.