Multiple correct submissions cause SQL exception

[janderse]

If there are multiple correct submissions to the same challenge by the same user, a persistent misleading SQL exception occurs. This does not happen for multiple incorrect submissions. The exception occurs while rendering the Challenges page, while attempting to render the challenge that has multiple correct submissions. The exception causes the user to be unable to view challenges in the same category that happen after the one with multiple correct submissions.

Exception text:

SQLSTATE[HY000]: General error: 2014 Cannot execute queries while other unbuffered queries are active. Consider using PDOStatement::fetchAll(). Alternatively, if your code is only ever going to run against mysql, you may enable query buffering by settin

Exception Trace:

    0 /var/www/mellivora/include/db.inc.php(179): PDOStatement->fetchAll(2) #1 /var/www/mellivora/include/db.inc.php(158): db_query('? SELECT? ...', Array, true) #2 /var/www/mellivora/htdocs/challenges.php(131): db_query_fetch_all('? SELECT? ...', Array) #3 {main}

Repro: There are multiple ways that this can occur.

1. Setting a problematic question to manual grading. Then I marked as correct multiple correct answers from the same user. This is how I think it first showed up in our CTF.
2. Timing / race condition. Because we had 170 people participate in the CTF, sometimes it took a second for the server to respond. I don't know for sure, but I think we ran into this when they clicked multiple times on submit.
3. Merging accounts. Because of various problems some accounts needed to be merged. To merge them I did manual SQL to update the user_id in the submissions table so that the submissions came from the merged user, and deleted the old user. Again this caused multiple correct submissions for the same challenge and user.
4. Manual Repro. Just go into the database and insert multiple correct answers for the same user and challenge.

Fix: Not sure yet. It should be easy to reproduce but I haven't looked at the code yet.

[janderse]

Easier repro steps: Submit the same wrong answer twice to a challenge. Go to the Submissions page and mark both submissions as correct. Go back to the Challenges page. You'll see an SQL exception.

[janderse]

Tracked down the bug. It is in this code: https://github.com/Nakiami/mellivora/blob/master/htdocs/challenges.php

$challenges = db_query_fetch_all('
    SELECT
       c.id,
       c.title,
       c.description,
       c.available_from,
       c.available_until,
       c.points,
       c.num_attempts_allowed,
       c.min_seconds_between_submissions,
       c.automark,
       c.relies_on,
       IF(c.automark = 1, 0, (SELECT ss.id FROM submissions AS ss WHERE ss.challenge = c.id AND ss.user_id = :user_id_1 AND ss.marked = 0)) AS unmarked, -- a submission is waiting to be marked
       (SELECT ss.added FROM submissions AS ss WHERE ss.challenge = c.id AND ss.user_id = :user_id_2 AND ss.correct = 1) AS correct_submission_added, -- a correct submission has been made
       (SELECT COUNT(*) FROM submissions AS ss WHERE ss.challenge = c.id AND ss.user_id = :user_id_3) AS num_submissions, -- number of submissions made
       (SELECT max(ss.added) FROM submissions AS ss WHERE ss.challenge = c.id AND ss.user_id = :user_id_4) AS latest_submission_added
    FROM challenges AS c
    WHERE c.category = :category
    ORDER BY c.points ASC, c.id ASC',
    array(
        'user_id_1'=>$_SESSION['id'],
        'user_id_2'=>$_SESSION['id'],
        'user_id_3'=>$_SESSION['id'],
        'user_id_4'=>$_SESSION['id'],
        'category'=>$current_category['id']
    )

The problem is that if there are multiple correct submissions, a subquery returns more than one row where MySQL requires just one return row, and so it causes an error. This cascades into an unrelated error message which is a really annoying "feature" of MySQL.

Here is the problem subquery:

(SELECT ss.added FROM submissions AS ss WHERE ss.challenge = c.id AND ss.user_id = :user_id_2 AND ss.correct = 1)

Here is the fix:

(SELECT ss.added FROM submissions AS ss WHERE ss.challenge = c.id AND ss.user_id = :user_id_2 AND ss.correct = 1 LIMIT 1)

I'll submit a pull request.

[Nakiami]

Hi! Thanks for your work. I think this hides the underlying problem. There needs to be a more reliable way to stop duplicate correct entries. I'd suggest implementing:

1. Race condition protection using sessions for the challenges page. We could also disable the button after being pressed once for a better user experience.
2. A check when marking submissions in the admin panel to make sure the challenge doesn't already have a correct solution for that user.

[Nakiami]

Fixed in https://github.com/Nakiami/mellivora/commit/6b9003b84777546007910f1e3ce0b7d23eefff7f + https://github.com/Nakiami/mellivora/commit/5d3932086e4643eb272c04352d79647e447a66bf