Summary
register (contracts/tholos-v2/src/lib.rs:1180-1261) computes a voter's new position and the assertion's new eligible total with plain arithmetic:
let new_amount = previous_amount + amount;
...
let new_total = resolution.eligible_total - previous_amount + new_amount;
(lib.rs:1254, lib.rs:1258). Both are checked against policy.max_position/policy.max_total_weight immediately after, but the addition itself happens before that bound check, using ordinary i128 +/- rather than checked_add/checked_sub. In a release Wasm build, integer overflow wraps silently rather than panicking, so a value that overflows i128 before the bound check runs would wrap to something the check could pass incorrectly, rather than failing loudly.
In practice amount and eligible_total are themselves bounded by realistic token supply, so this isn't demonstrated as reachable with a real SAC token, this is a defense-in-depth gap rather than a proven exploit.
Scope
- Replace the plain
+/- in register's position/total computation with checked_add/checked_sub, returning a new or existing Error variant on overflow instead of relying on the subsequent bound check to catch an already-wrapped value.
- Add a test that exercises values near
i128::MAX (or a boundary chosen to make the checked-arithmetic path observable without needing an actually overflowing real deposit) to confirm the new error path is reachable and correct.
Proposed approach
checked_add(amount).ok_or(Error::...)? in place of the two +/- expressions at lib.rs:1254 and lib.rs:1258, following the same checked-arithmetic convention already used elsewhere in tholos-v2 for settlement math.
Summary
register(contracts/tholos-v2/src/lib.rs:1180-1261) computes a voter's new position and the assertion's new eligible total with plain arithmetic:(
lib.rs:1254,lib.rs:1258). Both are checked againstpolicy.max_position/policy.max_total_weightimmediately after, but the addition itself happens before that bound check, using ordinaryi128+/-rather thanchecked_add/checked_sub. In a release Wasm build, integer overflow wraps silently rather than panicking, so a value that overflowsi128before the bound check runs would wrap to something the check could pass incorrectly, rather than failing loudly.In practice
amountandeligible_totalare themselves bounded by realistic token supply, so this isn't demonstrated as reachable with a real SAC token, this is a defense-in-depth gap rather than a proven exploit.Scope
+/-inregister's position/total computation withchecked_add/checked_sub, returning a new or existingErrorvariant on overflow instead of relying on the subsequent bound check to catch an already-wrapped value.i128::MAX(or a boundary chosen to make the checked-arithmetic path observable without needing an actually overflowing real deposit) to confirm the new error path is reachable and correct.Proposed approach
checked_add(amount).ok_or(Error::...)?in place of the two+/-expressions atlib.rs:1254andlib.rs:1258, following the same checked-arithmetic convention already used elsewhere intholos-v2for settlement math.