[Security] TOCTOU race condition allows duplicate victim registration in /register

[Galaxync]

Severity: HIGH

Affected file: core/user.py (/register route, lines 72-102), core/db.py (geo table schema, line 25)

Description

The /register endpoint enforces "each vId maps to exactly one victim record set" using a non-atomic check-then-act sequence:

cant = int(db.sentences_victim('count_times', vId, 3, 0))   # line 87 - CHECK

db.sentences_victim('insert_click', [vId, trape.url_to_clone, ...], 2)   # line 89
db.sentences_victim('delete_networks', [vId], 2)                         # line 90

if cant > 0:
    # UPDATE branch
    ...
else:
    # line 96-101 - ACT: INSERT into victims, victims_data, victims_battery, geo
    db.sentences_victim('insert_victim', [victimConnect, vId, time.time()], 2)
    db.sentences_victim('insert_victim_data', [vId], 2)
    db.sentences_victim('insert_victim_battery', [vId], 2)
    db.sentences_victim('insert_victim_geo', [victimGeo, vId], 2)

vId is fully attacker-controlled (taken directly from request.form['vId']). The check (count_times) and the subsequent writes are separate SQL statements with no transaction boundary and no application-level lock. There is also no UNIQUE constraint on victims, victims_data, or victims_battery — only the geo table has a PRIMARY KEY(id) (see core/db.py:25).

Because of this, two concurrent /register requests carrying the same vId can both read cant == 0 before either has committed its insert, and both take the INSERT branch.

Proof of Concept

1. Send two simultaneous POST /register requests with the same vId (e.g. vId=test123) and arbitrary required form fields (cpu, city, country_code2, country_name, ip, latitude, longitude, isp, country_code3, state_prov, zipcode, organization).
2. Both requests call count_times('test123') before either has committed insert_click, so both observe cant == 0.
3. Both proceed to INSERT INTO victims, victims_data, victims_battery, and geo for the same id.
4. The first INSERT INTO geo succeeds; the second raises an unhandled sqlite3.IntegrityError (500 error) because geo.id is a PRIMARY KEY. Meanwhile victims, victims_data, and victims_battery (which have no uniqueness constraint) now contain duplicate rows for the same vId.

Impact

• Duplicate rows in victims/victims_data/victims_battery corrupt downstream analytics and any logic assuming one row per vId (e.g. get_data/get_preview joins, which use INNER JOIN geo and could behave inconsistently against duplicated victim rows).
• Triggers an unhandled IntegrityError, producing a 500 response and noisy failure for legitimate concurrent registrations (e.g., a fast page reload).

Suggested Remediation

• Wrap the check-then-act sequence in a single transaction, or use an atomic INSERT ... ON CONFLICT (INSERT OR IGNORE/INSERT OR REPLACE) keyed on vId.
• Add a UNIQUE/PRIMARY KEY constraint on id for victims, victims_data, and victims_battery to make duplicate inserts fail safely and consistently.
• Catch sqlite3.IntegrityError around the insert path and fall back to the update path instead of returning an unhandled 500.