firoorg / firo

The privacy-focused cryptocurrency
https://firo.org
MIT License
720 stars 354 forks source link

Review fixes #1218

Closed AaronFeickert closed 1 year ago

AaronFeickert commented 1 year ago

Addresses issues FLS-01, FLS-02, FLS-03, FLS-07 from the HashCloak review.

onurinanc commented 1 year ago

For the FLS-02, you're only making the checks inside the BPPlus::verify(). You should also make the checks inside BPPlus::proof()

AaronFeickert commented 1 year ago

For the FLS-02, you're only making the checks inside the BPPlus::verify(). You should also make the checks inside BPPlus::proof()

Yep, only in the verifier. It was unclear how adding those failure modes to the prover would have other effects elsewhere in the codebase. It's not a security issue for the prover to generate such a proof anyway, since the verifier will reject it (and a malicious or malfunctioning prover can always generate a bad proof anyway). And the failure mode can't be triggered in practice anyway, since it's effectively a hash function preimage attack.