Potential double spend vulnerability

[kendricktan]

Similar issue to https://github.com/kobigurk/semaphore/issues/16

Background

keyImages used to prevent double spending

Attack Vector

Currently doesn't check if the points keyImage[0] and keyImage[1] are within field order 21888242871839275222246405745257275088696311157297823662689037894645226208583. A user could theoretically withdraw using keyImage[0], keyImage[1] and keyImage[0] + 21888242871839275222246405745257275088696311157297823662689037894645226208583, keyImage[1] + 21888242871839275222246405745257275088696311157297823662689037894645226208583, as long as they're both under uint256.

Recommended fix

Add the function below to AltBn128.sol

function modp(uint256 x) public pure
    returns (uint256)
{
    return x % P;
}

Change https://github.com/kendricktan/heiswap-dapp/blob/master/contracts/Heiswap.sol#L174 to

for (i = 0; i < ring.wParticipantsNo; i++) {
    if (ring.keyImages[uint8(i)][0] == AltBn128.modp(keyImage[0]) &&
        ring.keyImages[uint8(i)][1] == AltBn128.modp(keyImage[1])) {
        revert("Signature has been used!");
    }
}

[kendricktan]

Fix on new branch https://github.com/kendricktan/heiswap-dapp/commit/de022ffc9ffdfa4e6d9a7b51dc555728e25e9ca5