fix: added missing range checks in ShaBytesDynamic #579
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* make contract sdk simpler * reduce root inputs * delete convert function * summarize our library * update npm package * update package version * update attestation id * add util function to get revealed data
This reverts commit 847b88d.
Collaborator
|
hey @ArmanKolozyan, nice catch! |
Contributor
Author
|
Thanks a lot for the quick response, and I just DMed you on Telegram! |
Contributor
Author
|
@remicolin Just wanted to let you know that I added the range check for |
remicolin
approved these changes
Jun 3, 2025
remicolin
added a commit
that referenced
this pull request
Jun 12, 2025
This reverts commit 0c8c873.
remicolin
added a commit
that referenced
this pull request
Jun 12, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request adds a missing range check in the
ShaBytesDynamictemplate to address the vulnerability described in the following issue:The
Sha256Bytestemplate assumes that the input length (in_len_padded_bytes * 8) is constrained to fit withinceil(log2(8 * max_num_bytes))bits. However, this assumption is not enforced at the call sites in this project. In particular,in_len_padded_bytes * 8is passed to subtemplates likeSha256General, where it is used in comparison and selector logic without first constraining it to the required bitwidth.To fix this, we add an explicit
Num2Bitsconstraint onin_len_padded_bytes * 8insideShaBytesDynamic.This prevents potential overflow-based exploits that could bypass internal constraints and produce incorrect outputs while still satisfying the circuit.
Closes #578.