search

code-423n4 / org

Code4rena Governance and Discussion
70 stars 17 forks source link

Inconsistent severity for two-step change in governance #1

Open danbinnun opened 2 years ago

danbinnun commented 2 years ago

https://github.com/code-423n4/2021-12-pooltogether-findings/issues/106 The following issue was marked as high severity. As mentioned by judge leastwood, the issue two-step change of governance will be observed as non-critical. These issues are a missing layer of protection from human mistakes that would result in dire consequences. there seems to be a disparity in the severity.

another similar issue that was marked high: https://github.com/code-423n4/2021-12-pooltogether-findings/issues/8

conversation in the chat channel: danb — 02/05/2022 https://github.com/code-423n4/2021-12-pooltogether-findings/issues/106 it seems very weird to me that it was marked as high risk

hickuphh3 — 02/05/2022 Could u explain why u think it's weird? There doesn't seem to be a way to recover funds if epochDuration was set to 0. Definitely high / critical impact. One can argue that the likelihood of such an error is low, thus reducing the overall severity to medium perhaps.

danb — 02/05/2022 Correct, it's just a missing sanity check danb — 02/05/2022 in my opinion it should have the same severity as two step governance change

hickuphh3 — 02/05/2022 Ok, I see where you’re coming from. Both have similar mechanisms where an assumed incorrect input would lead to the loss of functionality / funds lockup.



What makes this case more severe is accessibility, where anyone can call the createPromotion() function as opposed to a governance setter that’s restricted.

Also, I’d argue that the loss of functionality from a wrong address set is less severe than fund lockup (although it could encompass it if governance is in control of funds) because there is the possibility of migrating to a new set of contracts, but there is no remedy for the latter.

Nevertheless, there does seem to be a disparity in severity if 2 step governance is to be considered non-critical..

MrToph commented 2 years ago

Another example: User calls the function in a wrong way and loses funds => medium

Users calling functions in unexpected ways and losing their funds seems to be the common theme among these issues but the assigned severities differ depending on the judges.