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

mid-level issues reported by slither #23

Open
huitseeker opened this issue Jul 12, 2023 · 1 comment
Open

mid-level issues reported by slither #23

huitseeker opened this issue Jul 12, 2023 · 1 comment

Comments

@huitseeker
Copy link
Contributor

huitseeker commented Jul 12, 2023 •
edited
Loading

While slither has trouble analyzing one of our tests (see crytic/slither#2046), here are the medium-severity issues found when excluding this file:

[email protected]➜~/tmp/solidity-verifier(slither✗)» slither . --exclude-dependencies                                                                                                                                 [12:08:17]
'forge clean' running (wd: /Users/huitseeker/tmp/solidity-verifier)
'forge build --build-info' running (wd: /Users/huitseeker/tmp/solidity-verifier)
INFO:Detectors:
NovaSpongePallasLib.permute(NovaSpongePallasLib.SpongeU24Pallas) (src/poseidon/Sponge.sol#429-439) ignores return value by PoseidonU24Pallas.hash(temp_state,PALLAS_MODULUS) (src/poseidon/Sponge.sol#432)
NovaSpongeVestaLib.permute(NovaSpongeVestaLib.SpongeU24Vesta) (src/poseidon/Sponge.sol#705-715) ignores return value by PoseidonU24Vesta.hash(temp_state,VESTA_MODULUS) (src/poseidon/Sponge.sol#708)
NovaVerifierStep2Lib.verifySecondary(PoseidonConstants.Pallas,NovaVerifierStep2DataLib.CompressedSnarkStep2Secondary,NovaVerifierStep2DataLib.VerifierKeyStep2,uint32,uint256[]) (src/verifier/step2/Step2Logic.sol#12-97) ignores return value by (output) = NovaSpongePallasLib.squeeze(sponge,squeezeLen) (src/verifier/step2/Step2Logic.sol#88)
NovaVerifierStep2Lib.verifyPrimary(PoseidonConstants.Vesta,NovaVerifierStep2DataLib.CompressedSnarkStep2Primary,NovaVerifierStep2DataLib.VerifierKeyStep2,uint32,uint256[]) (src/verifier/step2/Step2Logic.sol#100-185) ignores return value by (output) = NovaSpongeVestaLib.squeeze(sponge,sqeezeLen) (src/verifier/step2/Step2Logic.sol#176)
NIFSPallas.verify(NIFSPallas.NIFS,uint256,NIFSPallas.RelaxedR1CSInstance,NIFSPallas.R1CSInstance) (src/verifier/step3/Step3Logic.sol#29-137) ignores return value by (output) = NovaSpongePallasLib.squeeze(sponge,1) (src/verifier/step3/Step3Logic.sol#130)
NIFSVesta.verify(NIFSVesta.NIFS,uint256,NIFSVesta.RelaxedR1CSInstance,NIFSVesta.R1CSInstance) (src/verifier/step3/Step3Logic.sol#185-293) ignores return value by (output) = NovaSpongeVestaLib.squeeze(sponge,1) (src/verifier/step3/Step3Logic.sol#286)
KeccakTranscriptContractTest.testKeccakTranscriptPallas() (test/keccak-transcript-tests.t.sol#84-116) ignores return value by (output) = KeccakTranscriptLib.squeeze(transcript2,curve,squeezeLabel) (test/keccak-transcript-tests.t.sol#112)
KeccakTranscriptContractTest.testKeccakTranscriptVesta() (test/keccak-transcript-tests.t.sol#120-169) ignores return value by (output1) = KeccakTranscriptLib.squeeze(transcript4,curve,squeezeLabel1) (test/keccak-transcript-tests.t.sol#165)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#unused-return
INFO:Detectors:
EqPolinomialLib.evalsVesta(uint256[]).y (src/verifier/step4/EqPolynomial.sol#101) is written in both
        y = evalsResult[j + size] (src/verifier/step4/EqPolynomial.sol#107)
        y = mulmod(uint256,uint256,uint256)(x,r_i,modulusVesta) (src/verifier/step4/EqPolynomial.sol#109)
EqPolinomialLib.evalsPallas(uint256[]).y (src/verifier/step4/EqPolynomial.sol#135) is written in both
        y = evalsResult[j + size] (src/verifier/step4/EqPolynomial.sol#141)
        y = mulmod(uint256,uint256,uint256)(x,r_i,modulusPallas) (src/verifier/step4/EqPolynomial.sol#143)
ScalarFromUniformLib.montgomeryReducePallas(uint64,uint64,uint64,uint64,uint64,uint64,uint64,uint64).carry2_montgomeryReducePallas_asm_0 (src/verifier/step4/KeccakTranscript.sol#165) is written in both
        carry2_montgomeryReducePallas_asm_0 = 0 (src/verifier/step4/KeccakTranscript.sol#165)
        (carry2_montgomeryReducePallas_asm_0,carry_montgomeryReducePallas_asm_0) = mac(r0,k_montgomeryReducePallas_asm_0,PALLAS_MODULUS_0,0) (src/verifier/step4/KeccakTranscript.sol#167)
ScalarFromUniformLib.montgomeryReducePallas(uint64,uint64,uint64,uint64,uint64,uint64,uint64,uint64).carry_montgomeryReducePallas_asm_0 (src/verifier/step4/KeccakTranscript.sol#164) is written in both
        carry_montgomeryReducePallas_asm_0 = 0 (src/verifier/step4/KeccakTranscript.sol#164)
        (carry2_montgomeryReducePallas_asm_0,carry_montgomeryReducePallas_asm_0) = mac(r0,k_montgomeryReducePallas_asm_0,PALLAS_MODULUS_0,0) (src/verifier/step4/KeccakTranscript.sol#167)
ScalarFromUniformLib.montgomeryReducePallas(uint64,uint64,uint64,uint64,uint64,uint64,uint64,uint64).carry2_montgomeryReducePallas_asm_0 (src/verifier/step4/KeccakTranscript.sol#165) is written in both
        (carry2_montgomeryReducePallas_asm_0,carry_montgomeryReducePallas_asm_0) = mac(r0,k_montgomeryReducePallas_asm_0,PALLAS_MODULUS_0,0) (src/verifier/step4/KeccakTranscript.sol#167)
        (r4,carry2_montgomeryReducePallas_asm_0) = adc(r4,0,carry_montgomeryReducePallas_asm_0) (src/verifier/step4/KeccakTranscript.sol#171)
ScalarFromUniformLib.montgomeryReducePallas(uint64,uint64,uint64,uint64,uint64,uint64,uint64,uint64).r0 (src/verifier/step4/KeccakTranscript.sol#129) is written in both
        (r0,carry_montgomeryReducePallas_asm_0) = mac(r1,k_montgomeryReducePallas_asm_0,PALLAS_MODULUS_0,0) (src/verifier/step4/KeccakTranscript.sol#174)
        (r0,carry_montgomeryReducePallas_asm_0) = mac(r2,k_montgomeryReducePallas_asm_0,PALLAS_MODULUS_0,0) (src/verifier/step4/KeccakTranscript.sol#181)
ScalarFromUniformLib.montgomeryReducePallas(uint64,uint64,uint64,uint64,uint64,uint64,uint64,uint64).r0 (src/verifier/step4/KeccakTranscript.sol#129) is written in both
        (r0,carry_montgomeryReducePallas_asm_0) = mac(r2,k_montgomeryReducePallas_asm_0,PALLAS_MODULUS_0,0) (src/verifier/step4/KeccakTranscript.sol#181)
        (r0,carry_montgomeryReducePallas_asm_0) = mac(r3,k_montgomeryReducePallas_asm_0,PALLAS_MODULUS_0,0) (src/verifier/step4/KeccakTranscript.sol#188)
ScalarFromUniformLib.montgomeryReducePallas(uint64,uint64,uint64,uint64,uint64,uint64,uint64,uint64).r0 (src/verifier/step4/KeccakTranscript.sol#129) is written in both
        (r0,carry_montgomeryReducePallas_asm_0) = mac(r3,k_montgomeryReducePallas_asm_0,PALLAS_MODULUS_0,0) (src/verifier/step4/KeccakTranscript.sol#188)
        (r7,r0) = adc(r7,carry2_montgomeryReducePallas_asm_0,carry_montgomeryReducePallas_asm_0) (src/verifier/step4/KeccakTranscript.sol#192)
ScalarFromUniformLib.montgomeryReducePallas(uint64,uint64,uint64,uint64,uint64,uint64,uint64,uint64).r0 (src/verifier/step4/KeccakTranscript.sol#129) is written in both
        (r7,r0) = adc(r7,carry2_montgomeryReducePallas_asm_0,carry_montgomeryReducePallas_asm_0) (src/verifier/step4/KeccakTranscript.sol#192)
        (r0,carry2_montgomeryReducePallas_asm_0) = sbb(r4,PALLAS_MODULUS_0,carry2_montgomeryReducePallas_asm_0) (src/verifier/step4/KeccakTranscript.sol#208)
ScalarFromUniformLib.montgomeryReduceVesta(uint64,uint64,uint64,uint64,uint64,uint64,uint64,uint64).carry2_montgomeryReduceVesta_asm_0 (src/verifier/step4/KeccakTranscript.sol#262) is written in both
        carry2_montgomeryReduceVesta_asm_0 = 0 (src/verifier/step4/KeccakTranscript.sol#262)
        (carry2_montgomeryReduceVesta_asm_0,carry_montgomeryReduceVesta_asm_0) = mac(r0,k_montgomeryReduceVesta_asm_0,VESTA_MODULUS_0,0) (src/verifier/step4/KeccakTranscript.sol#264)
ScalarFromUniformLib.montgomeryReduceVesta(uint64,uint64,uint64,uint64,uint64,uint64,uint64,uint64).carry_montgomeryReduceVesta_asm_0 (src/verifier/step4/KeccakTranscript.sol#261) is written in both
        carry_montgomeryReduceVesta_asm_0 = 0 (src/verifier/step4/KeccakTranscript.sol#261)
        (carry2_montgomeryReduceVesta_asm_0,carry_montgomeryReduceVesta_asm_0) = mac(r0,k_montgomeryReduceVesta_asm_0,VESTA_MODULUS_0,0) (src/verifier/step4/KeccakTranscript.sol#264)
ScalarFromUniformLib.montgomeryReduceVesta(uint64,uint64,uint64,uint64,uint64,uint64,uint64,uint64).carry2_montgomeryReduceVesta_asm_0 (src/verifier/step4/KeccakTranscript.sol#262) is written in both
        (carry2_montgomeryReduceVesta_asm_0,carry_montgomeryReduceVesta_asm_0) = mac(r0,k_montgomeryReduceVesta_asm_0,VESTA_MODULUS_0,0) (src/verifier/step4/KeccakTranscript.sol#264)
        (r4,carry2_montgomeryReduceVesta_asm_0) = adc(r4,0,carry_montgomeryReduceVesta_asm_0) (src/verifier/step4/KeccakTranscript.sol#268)
ScalarFromUniformLib.montgomeryReduceVesta(uint64,uint64,uint64,uint64,uint64,uint64,uint64,uint64).r0 (src/verifier/step4/KeccakTranscript.sol#226) is written in both
        (r0,carry_montgomeryReduceVesta_asm_0) = mac(r1,k_montgomeryReduceVesta_asm_0,VESTA_MODULUS_0,0) (src/verifier/step4/KeccakTranscript.sol#271)
        (r0,carry_montgomeryReduceVesta_asm_0) = mac(r2,k_montgomeryReduceVesta_asm_0,VESTA_MODULUS_0,0) (src/verifier/step4/KeccakTranscript.sol#278)
ScalarFromUniformLib.montgomeryReduceVesta(uint64,uint64,uint64,uint64,uint64,uint64,uint64,uint64).r0 (src/verifier/step4/KeccakTranscript.sol#226) is written in both
        (r0,carry_montgomeryReduceVesta_asm_0) = mac(r2,k_montgomeryReduceVesta_asm_0,VESTA_MODULUS_0,0) (src/verifier/step4/KeccakTranscript.sol#278)
        (r0,carry_montgomeryReduceVesta_asm_0) = mac(r3,k_montgomeryReduceVesta_asm_0,VESTA_MODULUS_0,0) (src/verifier/step4/KeccakTranscript.sol#285)
ScalarFromUniformLib.montgomeryReduceVesta(uint64,uint64,uint64,uint64,uint64,uint64,uint64,uint64).r0 (src/verifier/step4/KeccakTranscript.sol#226) is written in both
        (r0,carry_montgomeryReduceVesta_asm_0) = mac(r3,k_montgomeryReduceVesta_asm_0,VESTA_MODULUS_0,0) (src/verifier/step4/KeccakTranscript.sol#285)
        (r7,r0) = adc(r7,carry2_montgomeryReduceVesta_asm_0,carry_montgomeryReduceVesta_asm_0) (src/verifier/step4/KeccakTranscript.sol#289)
ScalarFromUniformLib.montgomeryReduceVesta(uint64,uint64,uint64,uint64,uint64,uint64,uint64,uint64).r0 (src/verifier/step4/KeccakTranscript.sol#226) is written in both
        (r7,r0) = adc(r7,carry2_montgomeryReduceVesta_asm_0,carry_montgomeryReduceVesta_asm_0) (src/verifier/step4/KeccakTranscript.sol#289)
        (r0,carry2_montgomeryReduceVesta_asm_0) = sbb(r4,VESTA_MODULUS_0,carry2_montgomeryReduceVesta_asm_0) (src/verifier/step4/KeccakTranscript.sol#305)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#write-after-write
INFO:Slither:. analyzed (58 contracts with 52 detectors), 24 result(s) found
@storojs72
Copy link
Collaborator

Thanks for reporting. I will take a look at it later. I think, we can include enforcement of clear static analyser output into our CI - that will improve our verifier's code quality

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@huitseeker @storojs72