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.

Sign up for GitHub

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

Jump to bottom

fix: check email length <= max #107

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: check email length <= max #107

wants to merge 1 commit into from
+9 −3

Conversation

lonerapier
Copy link
Collaborator

resolves #70

Divide-By-0
Divide-By-0 reviewed Sep 22, 2023
View reviewed changes
packages/twitter-verifier-circuits/twitter.circom Outdated
@@ -54,6 +54,7 @@ template TwitterVerifier(max_header_bytes, max_body_bytes, n, k, pack_size, expo
signal (from_regex_out, from_regex_reveal[max_header_bytes]) <== FromRegex(max_header_bytes)(in_padded);
log(from_regex_out);
from_regex_out === 1;
in_padded[email_from_idx+max_email_from_len] === 0; // adversary can't use email len > max allowed
Copy link
Member

Choose a reason for hiding this comment

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

What? Does this array out of bounds indexing work in circom lol

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

oops, yes you are right. Have added correct solution now.

@lonerapier
Copy link
Collaborator Author

@Divide-By-0 ideally we should pass email length as input to check specific length, but I think for now this solution works.

@Divide-By-0
Copy link
Member

I dont know how it passed tests with your previous compilation -- can you add a test to the twitter verifier so we can be confident it is correct?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Divide-By-0 Divide-By-0 Awaiting requested review from Divide-By-0

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Max len email addesses can be spoofed
2 participants
@lonerapier @Divide-By-0