Post-Judging QA

[thebrittfactor]

1. Please post feedback about specific findings on the relevant issue in this repo.
2. Post a comment in this discussion linking to all findings you've commented on, along with any general feedback for the judge.

The judge for this contest is @dmvt.

Both the Validation repo and the Findings repo will be open to wardens with the SR role, for the purposes of post-judging QA.

• All post-judging QA requests must be posted in this Github Discussion in the findings repo.
• QA and Gas reports closed by Validators (i.e. not added to the findings repo) are NOT eligible.

Reminders

• Please keep your comments as fact-based as possible.
• C4 judges have final authority in determining validity and severity.
• Please link to specific issues and don't try to start discussions here.
• Please familiarize yourselves with our Good Citizen Policy.

Thank you!

[zhaojio]

Thanks for judging,
I left a comment on #89 and code-423n4/2024-09-kakarot-validation#116

[zhaojio]

and code-423n4/2024-09-kakarot-validation#209

[dmvt]

Responded on the issues. Validator is welcome to weigh in on why they invalidated 116 and 209, but I agree with their choice in both cases. You need to prove your reports, particularly when you're saying they are high risk.

[zhaojio]

Responded on the issues. Validator is welcome to weight in on why they invalidated 116 and 209, but I agree with their choice in both cases. You need to prove your reports, particularly when you're saying they are high risk.

Thanks for replying, recommented under 116.

[Ahmed-Aghadi]

Please take a look in my comment: #4 (comment)

[dmvt]

Responded

[Ahmed-Aghadi]

Please take a look in my comment: #20 (comment)

[imsrybr0]

Hi @dmvt,

Thanks for judging this contest and having a second look at my issues, here is a recap of my comments :

• address(0) is not initially warmed up #16 : you already responded and I understand your decision.
• Actors need to maintain two Kakarot accounts for L1 to L2 messaging #25 : you already responded and I understand your decision.
• JUMP and JUMPI fail when the offset is invalid but still within the currently created contract bytecode length #23 : you already responded and marked it as full credit.
• kakarot_precompiles do not return EXCEPTIONAL_HALT for invalid / unsuccessful calls #17 : you already responded and changed it to QA, it is still marked as insufficient quality report.
• RIPEMD-160 fails when (input length & 63) is greater or equal to 55 #22 : you already responded and changed it to Medium, it is still marked as insufficient quality report and unsatisfactory.
• CALL_STIPEND is injected in gas left directly when a call has value #45 : you already responded and changed it to Medium, it is still marked as insufficient quality report.
• Loops make CairoVM run out of steps #27 : you did not respond yet.

[dmvt]

The insufficient quality label was applied by the validator. Afaik I can't remove it and it does not impact scoring.

[imsrybr0]

Got it, so that only leaves the unsatisfactory label on 22.

Thank you 👍

[dmvt]

fixed :)

[Ahmed-Aghadi]

Please take a look in my comment: #2

[koolexcrypto]

Hi @dmvt ,

Thanks for judging this contest. I've left comments on those issues:

Main

#72

#74

#2 (#96 is a dup of this)

#97

Validation

code-423n4/2024-09-kakarot-validation#252

[Ahmed-Aghadi]

Please take a look in my comment: #42

[Bauchibred]

Hey @dmvt, thanks for judging, left a quick comment here.

[3docSec]

Hi @dmvt

I've left comments in 51, 53, 57.

Because of this #48 (comment) you left, I'd also give some context to some of our findings that fall into the "RPC-level crash" that we submitted as H: while the most likely impact is griefing as you say, the worst case scenario are contracts like the LayerZero OFT, which use ExcessivelySafeCall to prevent a griefing revert from bricking a LayerZero channel, which in the LayerZero case causes permanent freezing of bridged funds because the full channel becomes bricked and can't be recovered.

Also addressing the overinflation point, I'd point out that in our QA report we reported with L severity:

[QA-05] Some invalid values for the v field in Ethereum signatures are accepted

which is identical to #13, with the only difference that instead of s, v can be altered. We don't understand on which base 13 represents a H severity, considering that the vulnerability only allows to change an already valid signature into another signature, and therefore funds are not at risk.

[dmvt]

Because of this #48 (comment) you left, I'd also give some context to some of our findings that fall into the "RPC-level crash" that we submitted as H: while the most likely impact is griefing as you say, the worst case scenario are contracts like the LayerZero OFT, which use ExcessivelySafeCall to prevent a griefing revert from bricking a LayerZero channel, which in the LayerZero case causes permanent freezing of bridged funds because the full channel becomes bricked and can't be recovered.

I don't disagree with you, but in these cases you have a clear external factor (LZ). That makes all of these cases cap out at medium risk.

[dmvt]

Regarding the QA report, I'm not in the habit of upgrading QA to high risk. From what I understand, the ability to upgrade QA at all is going away in the next version of the tool. QA reports rarely if ever have the required info to make them valid high risk issues.

[3docSec]

I don’t mind not getting dup. The question was how 13 is a high, and not, say a L as we reported, given that you still need the private key to forge a signature, so it’s not a valid attack path for funds or functionality loss