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

BBS+: add some checks on disclosed indexes array #949

Merged
merged 3 commits into from
Oct 24, 2024
Merged

Conversation

FilippoTrotter
Copy link
Collaborator

No description provided.

@FilippoTrotter FilippoTrotter linked an issue Oct 14, 2024 that may be closed by this pull request
3 tasks
@jaromil
Copy link
Member

jaromil commented Oct 14, 2024

ahah!! so you read my email and blog !!!! <3 thanks a lot for taking the initiative !!! 👍🏽

local disclosed_messages = {}
local undisclosed_messages = {}
for i, v in ipairs(disclosed_indexes) do
if i > 1 and disclosed_indexes[i] == disclosed_indexes[i - 1] then
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure that checking only adjacent values suffices here. There can be scattered duplicates.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Arrays are always sorted in proof_gen (line 684) before being passed as input to core_proof_gen

Copy link
Member

Choose a reason for hiding this comment

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

can you add this sentence as comment in that line of code?

@@ -679,6 +682,7 @@ function bbs.proof_gen(ciphersuite, pk, signature, header, ph, messages, disclos
header = header or O.empty()
ph = ph or O.empty()
table.sort(disclosed_indexes) -- make sure indexes are sorted
if (disclosed_indexes[1] <= 0 or disclosed_indexes[#disclosed_indexes] > #messages) then error('disclosed indexes contains not valid integers', 2) end
Copy link
Member

Choose a reason for hiding this comment

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

It is best to separate this check in two and give a more accurate error message describing the cause.

@jaromil
Copy link
Member

jaromil commented Oct 24, 2024

many thanks!

@jaromil jaromil merged commit 1c9da4e into master Oct 24, 2024
27 checks passed
@jaromil jaromil deleted the hardening-bbs branch October 24, 2024 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BBS+ hardening
2 participants