# MACI

## Medium

### 1. DDos to prevent poll owner from generating `mergedStateRoot`

After voting is over, poll owner is supposed to call `mergeMaciStateAqSubRoots(...)` repeatedly, until all subtrees are merged, then owner will then call `mergeMaciStateAq(...)` to create `mergedStateRoot`.

However, the last call may always be front-run by a signup call by an unknowing or malicious user, such that `subTreesMerged` is no longer True.

https://github.com/privacy-scaling-explorations/maci/blob/776f9ece4191c765e59168b3214418e461af909c/contracts/contracts/trees/AccQueue.sol#L149

If the signup gate keeper maintains only limited registration quota, then this is a gas-grief attack, as `mergeMaciStateAqSubRoots(...)` is more expensive then a signup. However, if there is no limit on registration, the poll may be DDosed to prevent tallying.

### 2. Adding a new password variable to vote message to disincentivize even private key bribery

MACI is aimed to minimize the incentive of bribery, by introduing reverse order message processing. However, if the briber demands private key from the bribed, the best scenario will be a race to see who submitted the last vote.

By introducing a new password parameter in the message, MACI has a chance to minimize the incentive for even private key bribery.

To vote, a user need to include a password in the message. Only the password that was sent with the first message is the valid one. To make valid votes afterwards, the user would need to continue using the same password. User could send messages with other passwords, but they will be invalid.

Now if user is sending the first message from a different ETH address than the one used for signup, no one other than the coordinator will be able to associate the message with the user's MACI keys.

When a briber asks for private key and password, the user could happily provide wrong password along with correct MACI and ephemral private keys that are associated with the wrong passwords in transactions.

Equipped with private keys, if the briber were to fetch all messages sent to the poll, decrypt all of them to find out the one which decrypts using the correct private key, they would see the MACI public key of the user being included in the clear text message. However, at that point, they still could not be sure that the first message is the actual first message because even if it had nonce set to 1, it could be the second message and not be valid, so the password might not be the correct one.

# Low
### 1.
https://github.com/privacy-scaling-explorations/maci/blob/4658f3628df173e25aac200e2629db1080a7745e/contracts/contracts/Poll.sol#L94
This is missing a safeguard check:

if (_coordinatorPubKey.x >= SNARK_SCALAR_FIELD || _coordinatorPubKey.y >= SNARK_SCALAR_FIELD) {
revert MaciPubKeyLargerThanSnarkFieldSize();
}

### 2.
https://github.com/privacy-scaling-explorations/maci/blob/4658f3628df173e25aac200e2629db1080a7745e/contracts/contracts/Poll.sol#L167
while it is not an issue, it is a good practiceto use `>=` in place of `==`

## Optimization
### 1.
https://github.com/privacy-scaling-explorations/maci/blob/4658f3628df173e25aac200e2629db1080a7745e/contracts/contracts/MessageProcessor.sol#L97

uint256 r = numMessages % messageBatchSize;

if (r == 0) {
currentMessageBatchIndex = (numMessages / messageBatchSize) * messageBatchSize;
} else {
currentMessageBatchIndex = numMessages;
}

if `numMessages % messageBatchSize==0`, `(numMessages / messageBatchSize) * messageBatchSize` just be `numMessages`, thus it can be simplied to

uint256 r = numMessages % messageBatchSize;
currentMessageBatchIndex = numMessages;

### 2.
https://github.com/privacy-scaling-explorations/maci/blob/776f9ece4191c765e59168b3214418e461af909c/contracts/contracts/utilities/Utilities.sol#L42-L47
dat[2] to dat[10] default to 0, so no need to assign them explicitly.