Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(circuits): enforce use of stateIndex from message #1170

Merged
merged 2 commits into from Feb 19, 2024

Conversation

ctrlc03
Copy link
Collaborator

@ctrlc03 ctrlc03 commented Feb 8, 2024

Description

Prevent passing unmatched state leaves to censor votes by the coordinator. Instead, ensure that the state leaf sent for each message, is the state leaf at position message.stateIndex within the stateTree. Also, force decryption on the TS message processing, to ensure that a malicious user might not send an invalid message which when decrypted by the circuit (using decryptWithoutCheck and thus possibily failing decryption) would result in a valid stateIndex, causing a DoS on the coordinator which would not be able to generate a proof.

Also, the padding messages are not anymore the last message sent (this.messages[this.messages.length - 1]), but actually an empty command with a state index 0 so that when processed in the circuit, it will have no effect.

cc @kcharbo3

Related issue(s)

re #1133

Confirmation

Copy link

netlify bot commented Feb 8, 2024

Deploy Preview for maci-typedoc ready!

Name Link
🔨 Latest commit 22e091d
🔍 Latest deploy log https://app.netlify.com/sites/maci-typedoc/deploys/65d3203802d88d000864ec43
😎 Deploy Preview https://deploy-preview-1170--maci-typedoc.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@ctrlc03 ctrlc03 force-pushed the fix/circuit-unmatched-state-leaf branch from efc1626 to 8973c84 Compare February 8, 2024 16:03
@samajammin
Copy link
Collaborator

Thanks @ctrlc03!

Also, the padding messages are not the last message sent, but actually the noting up my sleeve message with a state index 0 so that we can process it as null in the circuit.

Not sure what you mean hear, could you please clarify?

@ctrlc03
Copy link
Collaborator Author

ctrlc03 commented Feb 8, 2024

Thanks @ctrlc03!

Also, the padding messages are not the last message sent, but actually the noting up my sleeve message with a state index 0 so that we can process it as null in the circuit.

Not sure what you mean hear, could you please clarify?

Have a look at core/ts/Poll.ts in genProcessMessagesPartialInputs, for some reason we were padding with the last message in the array (basically there might be one batch which needs to be padded to be full, which is the first one unless messages % batch size === 0). Now instead of sending the last message in the message array, which could have been a top up or a vote message (random based on whatever is sent last in a voting round), we just send a command with a state index of 0 so when processed by the circuit it'll just do nothing.

@ctrlc03 ctrlc03 marked this pull request as ready for review February 12, 2024 16:30
Copy link
Collaborator

@0xmad 0xmad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ctrlc03 thanks, just one minor comment.

circuits/circom/processMessages.circom Outdated Show resolved Hide resolved
@ctrlc03 ctrlc03 self-assigned this Feb 13, 2024
@ctrlc03 ctrlc03 force-pushed the fix/circuit-unmatched-state-leaf branch from 8973c84 to 37d7961 Compare February 13, 2024 10:58
@ctrlc03 ctrlc03 requested a review from 0xmad February 13, 2024 10:58
@ctrlc03 ctrlc03 force-pushed the fix/circuit-unmatched-state-leaf branch 3 times, most recently from 077b7ae to 1185b97 Compare February 13, 2024 15:42
@ctrlc03 ctrlc03 force-pushed the fix/circuit-unmatched-state-leaf branch from 1185b97 to 94f5c78 Compare February 13, 2024 15:46
@ctrlc03 ctrlc03 requested a review from 0xmad February 13, 2024 15:49
Copy link
Collaborator

@0xmad 0xmad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ctrlc03 thanks!

@ctrlc03 ctrlc03 force-pushed the fix/circuit-unmatched-state-leaf branch from 94f5c78 to 7271b95 Compare February 13, 2024 16:53
@kcharbo3
Copy link

LGTM!

@ctrlc03 ctrlc03 force-pushed the fix/circuit-unmatched-state-leaf branch from 7271b95 to 8cab606 Compare February 19, 2024 09:28
… = 0

Prevent coordinator censoring a valid second message by passing the currentVoteWeight equal to a
number which would result in not enough voice credits in the circuit
@ctrlc03 ctrlc03 force-pushed the fix/circuit-unmatched-state-leaf branch from 8cab606 to 22e091d Compare February 19, 2024 09:32
@ctrlc03 ctrlc03 merged commit 4fc84a6 into dev Feb 19, 2024
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants