jdabtieu / CTFOJ

Lightweight CTF judge platform for capture-the-flag (CTF) clubs
GNU Affero General Public License v3.0
10 stars 5 forks source link

[BUG] Race conditions #239

Closed jdabtieu closed 11 months ago

jdabtieu commented 11 months ago

Multiple race conditions are possible with SQL checks being done before inserts, in separate transactions. This presents the opportunity for race conditions (e.g. two duplicate users registered at the same time). These should be fixed by enforcing uniqueness in SQL schema, reordering certain queries, and using explicit transactions

Perhaps reorder code from e.g. [select check, select check, insert] to [insert, if fail, select check, select check] This should also reduce the number of SQL queries for the vast number of legitimate requests

These fixes are to be rolled out starting in v4.2.0 and continuing to v4.3.0

List of important routes to be fixed:

There are other routes that can suffer race conditions, but aren't important enough to fix. Examples include: For paginated pages, it's no big deal if the length is short by 1 or so due to a race, but the data itself must be consistent. Other minor data races that don't result in any inconsistencies/special handling (e.g. if a user's password is reset by two admins at the same time) are also no big deal. Permission checks are also not important (e.g. problem gets published halfway through a user's attempt to access it, or problem gets unpublished halfway through a user's attempt to access it).

jdabtieu commented 11 months ago

cs50 raises ValueError if a unique constraint fails

jdabtieu commented 11 months ago