bounds check comparing polynomials #56
Merged
Conversation
This file contains 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
|
In principle, two invalid polynomials could be equal if they don't have enough commits (though it would panic). I don't think this is a valid use case, hence just checking the threshold against the commit length |
AnomalRoil
requested changes
Feb 15, 2024
share/poly.go
Outdated
| @@ -387,6 +387,11 @@ func (p *PubPoly) Equal(q *PubPoly) bool { | |||
| return false | |||
| } | |||
| b := 1 | |||
|
|
|||
| if len(p.commits) < p.Threshold() || len(q.commits) < p.Threshold() { | |||
There was a problem hiding this comment.
What if p.Threshold() != q.Threshold() ?
There was a problem hiding this comment.
I assumed they just wouldn't be equal, but I guess this might not be the case if q has strictly more commits than p
AnomalRoil
approved these changes
Feb 15, 2024
Robingoumaz
pushed a commit
to Robingoumaz/kyber-drand
that referenced
this pull request
Apr 4, 2024
* bounds check when comparing polynomials * added extra check of threshold * fixing comment on PubPoly Equal being constant time --------- Co-authored-by: Yolan Romailler <anomalroil@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
I came across a panic here in my travels if somebody constructs an invalid
PubPolyfor comparison.Tbh this is not exhaustive, there are probably a bunch of other places where this could blow up, I only fixed this one